feat(openfeature): emit provider events from the DevCycle config lifecycle - #199
feat(openfeature): emit provider events from the DevCycle config lifecycle#199jonathannorris wants to merge 2 commits into
Conversation
093cb83 to
25c4380
Compare
6a831c0 to
c19d2b9
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances the Java Server SDK’s OpenFeature Local Bucketing integration by emitting standard OpenFeature provider lifecycle events driven by the DevCycle config polling/SSE lifecycle, allowing downstream consumers to distinguish healthy config refresh from prolonged failure.
Changes:
- Make
DevCycleProvideranEventProviderand emitPROVIDER_READY,PROVIDER_CONFIGURATION_CHANGED,PROVIDER_STALE, andPROVIDER_ERRORbased on config fetch outcomes. - Add a
ConfigUpdateListenerhook toEnvironmentConfigManagerand forward those callbacks throughDevCycleLocalClientto the provider (including replay of fatal errors). - Replace the
initialize()busy-wait with a latch-based first-config signal and fail fast for unauthorized SDK keys; add integration/unit tests and documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/devcycle/sdk/server/openfeature/DevCycleProvider.java | Converts provider to EventProvider, tracks degraded/failed init state, and emits provider events from config lifecycle callbacks. |
| src/main/java/com/devcycle/sdk/server/local/managers/EnvironmentConfigManager.java | Adds ConfigUpdateListener wiring and notifies on config load/error instead of only logging failures. |
| src/main/java/com/devcycle/sdk/server/local/managers/ConfigUpdateListener.java | Introduces an interface for config lifecycle callbacks (success + failure, with fatal flag). |
| src/main/java/com/devcycle/sdk/server/local/api/DevCycleLocalClient.java | Forwards config lifecycle callbacks to the OpenFeature provider and replays fatal errors if provider is created late. |
| src/test/java/com/devcycle/sdk/server/openfeature/DevCycleProviderTest.java | Adds unit coverage for provider shutdown behavior and init-time failure when the client never initializes. |
| src/test/java/com/devcycle/sdk/server/openfeature/DevCycleProviderEventsTest.java | Adds integration tests validating emitted OpenFeature provider events across config updates, failures, recovery, and unauthorized keys. |
| src/test/java/com/devcycle/sdk/server/helpers/LocalConfigServer.java | Extends the test config server to support ETag changes and controllable HTTP error responses. |
| OpenFeature.md | Documents emitted provider events and their meaning for Local vs Cloud bucketing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| private DevCycleProvider openFeatureProvider; | ||
| // volatile: read from the config polling and SSE threads via configUpdateListener | ||
| private volatile DevCycleProvider openFeatureProvider; |
There was a problem hiding this comment.
Do we really need to mark this volatile? This seems like a quick and dirty method instead of properly accessing the various callbacks safely. I'm worried about potential side effects in the actual regular use of the provider from this.
6dc4426 to
37d447b
Compare
Summary
DevCycleProvidernow extendsEventProviderand emitsPROVIDER_CONFIGURATION_CHANGED,PROVIDER_STALE,PROVIDER_ERROR, andPROVIDER_READYConfigUpdateListenertoEnvironmentConfigManagerso config fetch outcomes are no longer swallowed into the loggerinitialize()with a latch released by the first config load, and fails fast withFatalErrorwhen the SDK key is unauthorizedWithout these events, nothing downstream could tell a config refreshing normally from one that had been failing for hours.
PROVIDER_READYPROVIDER_CONFIGURATION_CHANGEDPROVIDER_STALEPROVIDER_ERRORPROVIDER_FATALwhen the SDK key is unauthorized.Implementation
EnvironmentConfigManagercallsonConfigLoaded(etag, firstLoad, changed)on every successful fetch andonConfigError(error, fatal)from the polling catch block.changedis derived from the ETag, so a 304 or an out-of-order config doesn't report a change.DevCycleLocalClientforwards to the provider once one exists, and replays a fatal error to a provider created after the failure.Worth calling out for review:
initialize()never emitsPROVIDER_READY/PROVIDER_ERRORitself.FeatureProviderStateManageralready sets those states from theinitialize()outcome, andEventProvider's javadoc says the same.initialize()emitsPROVIDER_READYexplicitly. The SDK doesn't callinitialize()again, so otherwise the provider stays inERRORforever after the config arrives.shutdown()callssuper.shutdown().EventProvider.shutdown()is concrete, so an override that forgets it compiles fine and leaks the emitter thread pool. Regression test included.flagsChangedis intentionally unset. DevCycle resolves variables per-user at evaluation time, so the changed key set isn't knowable at fetch time. The ETag goes ineventMetadatainstead.DevCycleCloudClientis unaffected: no config to change,isInitialized()always true. Docs added toOpenFeature.md.