Repository navigation
Conversation
|
✅ All CI checks and tests passed. Datadog automation helped this PR pass. 🎉 All green!🧪 All tests passed 🔄 Datadog retried 1 test - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 0da02ff | Docs | View more details | Give us feedback! |
a08f580 to
44e4e3e
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Cache misses and failures now cause repeated synchronous 100 ms read delays until flags are successfully installed.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds a one-shot Android Flags callback for the first installed cache or network configuration, independent of client readiness.
Changes:
- Adds immutable events and
FlagsClient.onFirstFlags(). - Retains first-installation keys for late registrations.
- Adds tests, documentation, sample usage, and Detekt configuration.
| File | Description |
|---|---|
| sample/kotlin/src/test/kotlin/com/datadog/android/sample/flags/FirstFlagsSampleTest.kt | Tests sample logging and evaluation. |
| sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt | Registers the sample callback. |
| sample/kotlin/src/main/kotlin/com/datadog/android/sample/flags/OpenFeatureFragment.kt | Saves a Boolean flag selection. |
| sample/kotlin/src/main/kotlin/com/datadog/android/sample/flags/FirstFlagsSample.kt | Logs keys and evaluates the saved flag. |
| sample/kotlin/build.gradle.kts | Adds sample test dependencies. |
| features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/model/FlagsClientEventTest.kt | Tests event immutability and shape. |
| features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsLatchTest.kt | Tests latch delivery and concurrency. |
| features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsInstallationTest.kt | Tests cache/network installation ordering. |
| features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FirstFlagsIntegrationTest.kt | Tests real-client callback behavior. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/model/FlagsClientEventType.kt | Defines the configuration event type. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/model/FlagsClientEvent.kt | Adds immutable event values. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FlagsRepository.kt | Exposes the installation latch internally. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/FirstFlagsLatch.kt | Retains first keys and delivers listeners. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepository.kt | Signals first installation and changes read waiting. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/NoOpFlagsClient.kt | Handles callback registration without installations. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/DatadogFlagsClient.kt | Delivers retained events and isolates exceptions. |
| features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt | Declares and documents the callback API. |
| features/dd-sdk-android-flags/README.md | Documents events and sample usage. |
| features/dd-sdk-android-flags/api/dd-sdk-android-flags.api | Records binary API additions. |
| features/dd-sdk-android-flags/api/apiSurface | Records public API additions. |
| detekt_custom_safe_calls_third_party.yml | Allows the atomic installation operation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thanks for the review, Sameeran. will follow up with additional tests. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0771ff9298
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
aarsilv
left a comment
There was a problem hiding this comment.
Thanks for iterating! Deleting FirstFlagsIntegrationTest leaves the real client's onFirstFlags() untested, but this looks by design as you want to move it elsewhere 👌



What changed
Adds
FlagsClient.onFirstFlags(listener): FlagsSubscriptionso applications can register on an existing client and learn when its first flags are installed from disk or the network, including before the first call tosetEvaluationContext.Each registration receives one retained
FlagsClientEventwithtype = CONFIGURATION_CHANGEDandflagsChanged, all keys from that first accepted configuration, not all flags defined on the server. A valid empty configuration delivers an empty key list. Cache misses, invalid cache and failed fetches do not complete the signal. Late registrations receive the same retained event immediately from memory, without SDK I/O. The event retains the first keys, not an assignment snapshot: evaluations always read the client's current flags.The Kotlin example app registers on its constructed client before creating its OpenFeature provider:
The sample helper logs the supplied keys and evaluates
my-flag-keydirectly with afalsedefault:For shorter-lived owners, retain the returned
FlagsSubscriptionand callunsubscribe()during cleanup. Cancellation is thread-safe, idempotent and local to that registration. It removes pending captures; an already-claimed callback may still run. It does not interrupt callbacks or undo synchronous replay.Pending callbacks run on a dedicated background worker. Available results replay synchronously on the registering thread before
onFirstFlagsreturns, so a later registration can run before an earlier queued one. Callbacks run outside internal locks. Callback Exceptions are logged and isolated; Errors are not caught. Dispatch UI work to the main thread.Registration is directly on the public
FlagsClientinterface. Custom implementations implement this method, and Kotlin interface delegation forwards it. The event listener and subscription remain named functional interfaces; no general event bus is introduced. Events are constructed internally by the SDK; no public event constructor or builder is exposed.Malformed network responses also change behavior for callers that do not subscribe. Previously, parse failure installed an empty configuration and completed the context update successfully. It now follows the failed-fetch path: installed flags and their context are retained, and the update reports failure. Valid empty responses still install successfully. This prevents malformed input from consuming the first-flags signal; it does not introduce a new policy for retaining flags after failed context updates.
Structure and signal flow
Before — existing installation flow
flowchart TB C["DatadogFlagsClient"] E["EvaluationsManager"] N["PrecomputedAssignmentsDownloader<br/>via PrecomputedAssignmentsReader"] R["DefaultFlagsRepository<br/>implements FlagsRepository<br/>Atomic flags and context"] P["FlagsPersistenceManager"] D["DataStoreHandler"] C -->|"setEvaluationContext"| E C -->|"Read current flags and context"| R E -->|"Fetch"| N N -->|"Response parsed by PrecomputeMapper"| E E -->|"setFlagsAndContext"| R R -->|"Construct and save network flags"| P P -->|"Load callback: install only if empty"| R P -->|"Read and write"| D D -->|"Read and write completion"| PAfter — direct registration and first-flags delivery
flowchart TB A["Application"] C["DatadogFlagsClient<br/>implements FlagsClient.onFirstFlags<br/>Retains first FlagsClientEvent"] E["EvaluationsManager"] N["PrecomputedAssignmentsDownloader<br/>via PrecomputedAssignmentsReader"] R["DefaultFlagsRepository<br/>implements FlagsRepository<br/>Current flags and context"] P["FlagsPersistenceManager"] D["DataStoreHandler"] L["FirstFlagsLatch<br/>Retains first installed keys<br/>Owns pending registrations"] W["One-shot ExecutorService<br/>flags-first-flags"] A -->|"onFirstFlags(listener)"| C C -->|"Return FlagsSubscription"| A C -->|"firstFlags.whenComplete"| L A -.->|"unsubscribe: clear this pending registration"| L C -->|"setEvaluationContext"| E C -->|"Evaluate current flags"| R E -->|"Fetch"| N N -->|"Response parsed by PrecomputeMapper"| E E -->|"setFlagsAndContext(context, flags, onInstalled)"| R R -->|"Submit save after network installation"| P P -->|"Cache load: compare-and-set only if empty"| R P -->|"Read and write"| D D -->|"Read and write completion"| P R -.->|"Network: onInstalled settles bookkeeping and context callback"| E R -.->|"Complete after first accepted install and network hook"| L L -.->|"Pending batch"| W W -.->|"Claim listener and deliver keys outside lock"| C L -.->|"Available result: synchronous replay on caller"| C C -.->|"Build or reuse first event; invoke listener outside lock"| AThe repository continues to own persistence and its separate persistence-load barrier. A cached configuration completes the first-flags latch only if its compare-and-set installation succeeds. For a first network installation, the repository submits persistence, runs
onInstalledto settle initialization bookkeeping and context completion, then completes the latch infinally. Disk write completion is not awaited, and ordinary storage-submission exceptions are logged without aborting an accepted installation.The latch keeps the first installed keys even when the repository later replaces its current flags. The client constructs and retains one event from those keys. Pending delivery uses a one-shot worker; late replay uses the caller thread. Cancellation can suppress a queued listener until it is claimed for delivery.
Why
Cached flags can be available before network initialization finishes. Applications need a reliable signal that flags have been installed so they can evaluate them directly. Registering on an existing client avoids constructor callback timing problems, and retaining the first result prevents fast disk or network completion from being missed.
This notification is independent of readiness and does not change OpenFeature evaluation gates or evaluation reasons. Tracks FFLSDK-254.
The manager-level regression for successful installation after initialization timeout remains included. Client integration coverage is deferred to reliability/single-fit or RUM FIT/FLEX, including Flags module support as needed.