-
Notifications
You must be signed in to change notification settings - Fork 21
feat: Implement hooks in multi provider #594
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 hooks in multi provider #594
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #594 +/- ##
==========================================
+ Coverage 89.52% 89.76% +0.23%
==========================================
Files 77 77
Lines 3084 3166 +82
Branches 350 364 +14
==========================================
+ Hits 2761 2842 +81
+ Misses 256 253 -3
- Partials 67 71 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
651693a to
1d400ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Multi-Provider feature in the OpenFeature .NET SDK by implementing support for provider hooks with context isolation and improving the overall hook execution mechanism.
- Added hook execution support for individual providers within the Multi-Provider
- Implemented context isolation between providers to prevent hook modifications from affecting other providers
- Updated evaluation logic to pass logger instance and use modified contexts from hooks
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/OpenFeature.Providers.MultiProvider/ProviderExtensions.cs |
Added comprehensive hook execution logic with before/after/error/finally hook support and context isolation |
src/OpenFeature.Providers.MultiProvider/MultiProvider.cs |
Added GetProviderHooks method and updated evaluation calls to include logger parameter |
test/OpenFeature.Providers.MultiProvider.Tests/ProviderExtensionsTests.cs |
Updated test calls to include logger parameter and added new tests for hook execution |
test/OpenFeature.Providers.MultiProvider.Tests/MultiProviderTests.cs |
Added comprehensive hook testing including context isolation and error handling scenarios |
README.md |
Updated documentation to reflect hook support and removed hook limitations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Put this PR in draft. Testing a couple of cases which seems to do double hook execution instead of a single one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
This PR
This pull request introduces support for provider hooks in the Multi-Provider feature, ensuring that each provider's hooks are executed with context isolation as per the OpenFeature specification. It also updates the documentation to reflect these changes and refines how evaluation contexts and logging are handled in provider evaluations.
Enhancements to Multi-Provider hook support:
ProviderExtensions.csand integrated into the evaluation logic inMultiProvider.cs. [1] [2] [3] [4]ExecuteBeforeEvaluationHooksAsyncandExecuteAfterEvaluationHooksAsyncinProviderExtensions.csto manage hook execution and error handling for each provider.Documentation updates:
README.mdto clarify that provider hooks are now supported in Multi-Provider, and events are not fully supported. The limitations section was revised to remove the statement that hooks are not supported. [1] [2]Internal improvements:
ProviderExtensions.csandMultiProvider.csto pass a logger instance for better diagnostics and error logging during hook execution. [1] [2] [3]System.Collections.ImmutableandMicrosoft.Extensions.Logging. [1] [2]Related Issues
Fixes #551