Skip to content

feat: Implement Tracking in .NET #309#327

Merged
toddbaert merged 4 commits into
open-feature:mainfrom
chrfwow:logging
Dec 11, 2024
Merged

feat: Implement Tracking in .NET #309#327
toddbaert merged 4 commits into
open-feature:mainfrom
chrfwow:logging

Conversation

@chrfwow

@chrfwow chrfwow commented Dec 4, 2024

Copy link
Copy Markdown
Contributor

This PR

Adds support for tracking

Related Issues

Closes #309

Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
@chrfwow chrfwow requested a review from a team as a code owner December 4, 2024 16:03
@chrfwow chrfwow changed the title [FEATURE] Implement Tracking in .NET #309 feat Implement Tracking in .NET #309 Dec 4, 2024
@chrfwow chrfwow changed the title feat Implement Tracking in .NET #309 feat: Implement Tracking in .NET #309 Dec 4, 2024
@codecov

codecov Bot commented Dec 4, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 65.51724% with 30 lines in your changes missing coverage. Please review.

Project coverage is 85.54%. Comparing base (70f847b) to head (23f4a06).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...c/OpenFeature/Model/TrackingEventDetailsBuilder.cs 59.18% 20 Missing ⚠️
src/OpenFeature/Model/TrackingEventDetails.cs 66.66% 8 Missing ⚠️
src/OpenFeature/FeatureProvider.cs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #327      +/-   ##
==========================================
- Coverage   86.66%   85.54%   -1.12%     
==========================================
  Files          34       36       +2     
  Lines        1387     1474      +87     
  Branches      147      150       +3     
==========================================
+ Hits         1202     1261      +59     
- Misses        153      183      +30     
+ Partials       32       30       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

/// <summary>
/// The index for the "targeting key" property when the EvaluationContext is serialized or expressed as a dictionary.
/// </summary>
internal const string TargetingKeyIndex = "targetingKey";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the TargetingKeyIndex here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we would need instead a value index if anything, that would serve the same purpose when the TrackingEventDetails are serialized.

Comment thread src/OpenFeature/FeatureProvider.cs Outdated
public virtual Channel<object> GetEventChannel() => this.EventChannel;

/// <summary>
/// Use this method to track user interactions and the application state. The implementation of this method is optional.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Use this method to track user interactions and the application state. The implementation of this method is optional.
/// Track a user action or application state, usually representing a business objective or outcome. The implementation of this method is optional.

Comment thread test/OpenFeature.Tests/OpenFeatureClientTests.cs
/// <summary>
///A predefined value field for the tracking details.
/// </summary>
public readonly double? Value;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional is good.

Comment thread README.md
@toddbaert toddbaert self-requested a review December 6, 2024 20:33

@toddbaert toddbaert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the only blocker.

@chrfwow chrfwow requested a review from toddbaert December 9, 2024 07:58

@askpt askpt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a couple of concerns I found during the review. Otherwise, it seems good!

Comment thread src/OpenFeature/OpenFeatureClient.cs Outdated
Comment thread src/OpenFeature/OpenFeatureClient.cs Outdated
@chrfwow chrfwow requested a review from askpt December 9, 2024 11:01
Comment thread README.md Outdated
@chrfwow chrfwow requested a review from lukas-reining December 9, 2024 15:36

@toddbaert toddbaert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but consider @askpt 's points. I think it's fine to throw for whitespace event keys as well.

Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
@chrfwow chrfwow requested a review from toddbaert December 10, 2024 13:27
@toddbaert toddbaert merged commit cbf4f25 into open-feature:main Dec 11, 2024
@chrfwow chrfwow deleted the logging branch December 12, 2024 06:20
toddbaert pushed a commit that referenced this pull request Dec 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.2.0](v2.1.0...v2.2.0)
(2024-12-12)


### ✨ New Features

* Feature Provider Enhancements-
[#321](#321)
([#324](#324))
([70f847b](70f847b))
* Implement Tracking in .NET
[#309](#309)
([#327](#327))
([cbf4f25](cbf4f25))
* Support Returning Error Resolutions from Providers
([#323](#323))
([bf9de4e](bf9de4e))


### 🧹 Chore

* **deps:** update dependency fluentassertions to v7
([#325](#325))
([35cd77b](35cd77b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
kylejuliandev pushed a commit to kylejuliandev/dotnet-sdk that referenced this pull request Jan 9, 2025
## This PR
Adds support for tracking

### Related Issues

Closes open-feature#309

---------

Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
kylejuliandev pushed a commit to kylejuliandev/dotnet-sdk that referenced this pull request Jan 9, 2025
🤖 I have created a release *beep* *boop*
---

##
[2.2.0](open-feature/dotnet-sdk@v2.1.0...v2.2.0)
(2024-12-12)

### ✨ New Features

* Feature Provider Enhancements-
[open-feature#321](open-feature#321)
([open-feature#324](open-feature#324))
([70f847b](open-feature@70f847b))
* Implement Tracking in .NET
[open-feature#309](open-feature#309)
([open-feature#327](open-feature#327))
([cbf4f25](open-feature@cbf4f25))
* Support Returning Error Resolutions from Providers
([open-feature#323](open-feature#323))
([bf9de4e](open-feature@bf9de4e))

### 🧹 Chore

* **deps:** update dependency fluentassertions to v7
([open-feature#325](open-feature#325))
([35cd77b](open-feature@35cd77b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Implement Tracking in .NET

4 participants