Skip to content

Conversation

@chrfwow
Copy link
Contributor

@chrfwow chrfwow commented Dec 4, 2024

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
Copy link

codecov bot commented Dec 4, 2024

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
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
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.

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
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.

/// <summary>
///A predefined value field for the tracking details.
/// </summary>
public readonly double? Value;
Copy link
Member

Choose a reason for hiding this comment

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

Optional is good.

@toddbaert toddbaert self-requested a review December 6, 2024 20:33
Copy link
Member

@toddbaert toddbaert left a comment

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
Copy link
Member

@askpt askpt left a comment

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!

@chrfwow chrfwow requested a review from askpt December 9, 2024 11:01
@chrfwow chrfwow requested a review from lukas-reining December 9, 2024 15:36
Copy link
Member

@toddbaert toddbaert left a comment

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
13 of 15 checks passed
@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