feat: support isolated API instances#1928
feat: support isolated API instances#1928marcozabel wants to merge 3 commits intoopen-feature:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for isolated OpenFeature API instances by adding a createIsolated method to OpenFeatureAPI and a new OpenFeatureAPIFactory in a dedicated package. These changes allow for independent state management (providers, hooks, context) across different instances, satisfying specification requirements. Feedback includes a necessary fix for a compilation error in the tests due to unhandled exceptions, a recommendation to move the shared process-wide lock to an instance level for better performance isolation, and a concern regarding the completeness of global state restoration in the test cleanup logic.
518c67f to
dcf7a52
Compare
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
dcf7a52 to
e7b91f3
Compare
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1928 +/- ##
============================================
+ Coverage 92.25% 93.22% +0.96%
- Complexity 653 658 +5
============================================
Files 59 60 +1
Lines 1589 1595 +6
Branches 179 179
============================================
+ Hits 1466 1487 +21
+ Misses 76 62 -14
+ Partials 47 46 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @return a new API instance | ||
| * @see OpenFeatureAPI#createIsolated() | ||
| */ | ||
| public static OpenFeatureAPI createAPI() { |
There was a problem hiding this comment.
I don't fully see the point of this method/class. OpenFeatureAPI.createIsolated() has the exact same visibility level and does the same thing. I think this will only lead to confusion
| // relies on a ready event to be emitted | ||
| emitterExecutor.submit(() -> { | ||
| try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) { | ||
| try (var ignored = localLock != null ? localLock.readLockAutoCloseable() : null) { |
There was a problem hiding this comment.
Why is the lock optional? IIRC, we needed that lock to avoid race conditions when emitting events
| // package-private multi-read/single-write lock | ||
| static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); | ||
| // package-private multi-read/single-write lock (instance-level for isolation) | ||
| AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); |
|
|
||
| apiLock = setupLock(apiLock, mockInnerReadLock(), mockInnerWriteLock()); | ||
| OpenFeatureAPI.lock = apiLock; | ||
| api.lock = apiLock; |
There was a problem hiding this comment.
Instead of setting the lock like this, can we also add a package private constructor to the api that takes a lock?



This PR
This PR introduces support for creating isolated OpenFeature API instances, each with their own providers, hooks, context, and event handling - enabling multi-tenant or side-by-side usage without shared global state.
Related Issues
Fixes #1925
Notes
Follow-up Tasks
How to test