Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement transaction context #312

Merged
merged 25 commits into from
Jan 7, 2025

Conversation

askpt
Copy link
Member

@askpt askpt commented Nov 11, 2024

Transaction Context

This pull request introduces transaction context propagation to the OpenFeature library. The changes include adding a new interface for transaction context propagation, implementing a no-op and an AsyncLocal-based propagator, and updating the API to support these propagators.

Related Issues

Fixes #243

Notes

Transaction Context Propagation:

API Enhancements:

@askpt askpt linked an issue Nov 11, 2024 that may be closed by this pull request
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.49%. Comparing base (2ef9955) to head (95c1fb7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #312      +/-   ##
==========================================
+ Coverage   85.13%   85.49%   +0.36%     
==========================================
  Files          36       38       +2     
  Lines        1480     1517      +37     
  Branches      151      154       +3     
==========================================
+ Hits         1260     1297      +37     
  Misses        188      188              
  Partials       32       32              

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

Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Nice start! Left some thoughts and references to how we are doing it in the other languages.

src/OpenFeature/Model/TransactionContext.cs Outdated Show resolved Hide resolved
src/OpenFeature/Model/ContextBuilder.cs Outdated Show resolved Hide resolved
src/OpenFeature/Api.cs Outdated Show resolved Hide resolved
src/OpenFeature/Api.cs Outdated Show resolved Hide resolved
@askpt
Copy link
Member Author

askpt commented Nov 26, 2024

Nice start! Left some thoughts and references to how we are doing it in the other languages.

Thanks for the review @lukas-reining. I based the implementation on the JS client. I'll look into more detail on the Java version instead.

@askpt askpt force-pushed the askpt/243-feature-implement-transaction-context branch from 35b90bf to 9158a92 Compare December 4, 2024 17:05
@askpt askpt closed this Dec 10, 2024
@askpt askpt force-pushed the askpt/243-feature-implement-transaction-context branch from a0a7f36 to c471c06 Compare December 10, 2024 08:34
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
@askpt askpt reopened this Dec 10, 2024
askpt added 16 commits December 12, 2024 08:18
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
@askpt askpt requested a review from lukas-reining December 20, 2024 08:52
@askpt askpt marked this pull request as ready for review December 20, 2024 08:52
@askpt askpt requested a review from a team as a code owner December 20, 2024 08:52
src/OpenFeature/Api.cs Outdated Show resolved Hide resolved
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert
Copy link
Member

toddbaert commented Dec 31, 2024

This looks right to me, though unless there's a good reason, I think we should remove this. Besides that I'm ready to approve.

I also pushed a small change to add docs to the README. @askpt please check them and make sure they make sense to you (it's a similar example to other SDKs).

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.

Approving, but highlighting this thread: #312 (comment)

src/OpenFeature/AsyncLocalTransactionContextPropagator.cs Outdated Show resolved Hide resolved
src/OpenFeature/OpenFeatureClient.cs Outdated Show resolved Hide resolved
test/OpenFeature.Tests/OpenFeatureTests.cs Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
askpt added 3 commits January 2, 2025 14:15
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
@askpt askpt requested a review from chrfwow January 2, 2025 15:22
Copy link
Contributor

@chrfwow chrfwow left a comment

Choose a reason for hiding this comment

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

Looks good

@askpt askpt merged commit 1b5a0a9 into main Jan 7, 2025
15 checks passed
@askpt askpt deleted the askpt/243-feature-implement-transaction-context branch January 7, 2025 08:40
kylejuliandev pushed a commit to kylejuliandev/dotnet-sdk that referenced this pull request Jan 9, 2025
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## Transaction Context
<!-- add the description of the PR here -->

This pull request introduces transaction context propagation to the
OpenFeature library. The changes include adding a new interface for
transaction context propagation, implementing a no-op and an
AsyncLocal-based propagator, and updating the API to support these
propagators.

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

Fixes open-feature#243

### Notes
<!-- any additional notes for this PR -->
#### Transaction Context Propagation:

*
[`src/OpenFeature/Api.cs`](diffhunk://#diff-dc150fbd3b7be797470374ee7e38c7ab8a31eb9b6b7a52345e05114c2d32a15eR25-R26):
Added a private field `_transactionContextPropagator` and a lock for
thread safety. Introduced methods to get and set the transaction context
propagator, and to manage the transaction context using the propagator.
[[1]](diffhunk://#diff-dc150fbd3b7be797470374ee7e38c7ab8a31eb9b6b7a52345e05114c2d32a15eR25-R26)
[[2]](diffhunk://#diff-dc150fbd3b7be797470374ee7e38c7ab8a31eb9b6b7a52345e05114c2d32a15eR222-R274)
*
[`src/OpenFeature/AsyncLocalTransactionContextPropagator.cs`](diffhunk://#diff-d9ca58e32d696079f875c51837dfc6ded087b06eb3aef3513f5ea15ebc22c700R1-R25):
Implemented the `AsyncLocalTransactionContextPropagator` class that uses
`AsyncLocal<T>` to store the transaction context.
*
[`src/OpenFeature/NoOpTransactionContextPropagator.cs`](diffhunk://#diff-09ab422ed267155042b791de4d1c88f1bd82cb68d5f541a92c6af4318ceacd6aR1-R15):
Implemented a no-op version of the `ITransactionContextPropagator`
interface.
*
[`src/OpenFeature/Model/ITransactionContextPropagator.cs`](diffhunk://#diff-614f5b3e42871f4057f04d5fd27bf56157315e1c822ff0c83403255a8bf163ecR1-R26):
Defined the `ITransactionContextPropagator` interface responsible for
persisting transaction contexts.

#### API Enhancements:

*
[`src/OpenFeature/Api.cs`](diffhunk://#diff-dc150fbd3b7be797470374ee7e38c7ab8a31eb9b6b7a52345e05114c2d32a15eR291):
Updated the `ShutdownAsync` method to reset the transaction context
propagator.
*
[`src/OpenFeature/OpenFeatureClient.cs`](diffhunk://#diff-c23c8a3ea4538fbdcf6b1cf93ea3de456906e4d267fc4b2ba3f8b1cb186a7907R224):
Modified the `EvaluateFlagAsync` method to merge the transaction context
with the evaluation context.

---------

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
toddbaert added a commit that referenced this pull request Jan 31, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.3.0](v2.2.0...v2.3.0)
(2025-01-31)


### ⚠ BREAKING CHANGES

#### Hook Changes

The signature of the `finally` hook stage has been changed. The
signature now includes the `evaluation details`, as per the [OpenFeature
specification](https://openfeature.dev/specification/sections/hooks#requirement-438).
Note that since hooks are still `experimental,` this does not constitute
a change requiring a new major version. To migrate, update any hook that
implements the `finally` stage to accept `evaluation details` as the
second argument.

* Add evaluation details to finally hook stage
([#335](#335))
([2ef9955](2ef9955))

#### .NET 6

Removed support for .NET 6.

* add dotnet 9 support, rm dotnet 6
([#317](#317))
([2774b0d](2774b0d))

### 🐛 Bug Fixes

* Adding Async Lifetime method to fix flaky unit tests
([#333](#333))
([e14ab39](e14ab39))
* Fix issue with DI documentation
([#350](#350))
([728ae47](728ae47))


### ✨ New Features

* add dotnet 9 support, rm dotnet 6
([#317](#317))
([2774b0d](2774b0d))
* Add evaluation details to finally hook stage
([#335](#335))
([2ef9955](2ef9955))
* Implement Default Logging Hook
([#308](#308))
([7013e95](7013e95))
* Implement transaction context
([#312](#312))
([1b5a0a9](1b5a0a9))


### 🧹 Chore

* **deps:** update actions/upload-artifact action to v4.5.0
([#332](#332))
([fd68cb0](fd68cb0))
* **deps:** update codecov/codecov-action action to v5
([#316](#316))
([6c4cd02](6c4cd02))
* **deps:** update codecov/codecov-action action to v5.1.2
([#334](#334))
([b9ebddf](b9ebddf))
* **deps:** update codecov/codecov-action action to v5.3.1
([#355](#355))
([1e8ebc4](1e8ebc4))
* **deps:** update dependency coverlet.collector to 6.0.3
([#336](#336))
([8527b03](8527b03))
* **deps:** update dependency coverlet.msbuild to 6.0.3
([#337](#337))
([26fd235](26fd235))
* **deps:** update dependency dotnet-sdk to v9.0.101
([#339](#339))
([dd26ad6](dd26ad6))
* **deps:** update dependency fluentassertions to 7.1.0
([#346](#346))
([dd1c8e4](dd1c8e4))
* **deps:** update dependency microsoft.net.test.sdk to 17.12.0
([#322](#322))
([6f5b049](6f5b049))


### 📚 Documentation

* disable space in link text lint rule
([#329](#329))
([583b2a9](583b2a9))

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

---------

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.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 transaction context
5 participants