Repository navigation
FFLSDK-258: Add experimentation opt-out modes - #4024
Conversation
Add native and OpenFeature opt-out configuration with timestamp validation. Require server acknowledgment and isolate cached assignments and pending contexts.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: e1504f4 | Docs | View more details | Give us feedback! |
Bind assignment acknowledgements and persisted caches to the selected mode and exact subject cutoff. Cover request, restart, and stale-response behavior.
leoromanovsky
changed the title
Deliver committed state changes and callbacks outside the policy lock. Read one assignment snapshot and retain the attempted subject for error telemetry.
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f335b48a3d
ℹ️ 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".
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The most critical concern is that EvaluationContextCallback completion callbacks can be delivered on the UI thread — both via drain() executing on whichever thread calls it and via the direct onFailure call in the invalid-context path — violating the background-thread contract. A separate issue in DatadogFlagsClient.kt causes the React Native snapshot path to block for up to 200 ms (two sequential 100 ms waits) when persistence is loading and the opt-out policy is disabled.
🤖 Bits Code Review · Commit f335b48
|
Created DOCS-16087 for the editorial review. |
Use Gregorian dates and explicit UTC bounds for opt-out timestamps. Derive request options from the effective policy and validate acknowledgements.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b7ab71778
ℹ️ 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".
| Invalid timestamps return application defaults with `INVALID_CONTEXT` and do not send an assignment request. | ||
|
|
||
| The backend skips experiment allocations and continues through ordinary feature allocations. | ||
| The SDK requires the backend to acknowledge exclusion before using the response. |
There was a problem hiding this comment.
🤖 Comment from Codex working with Leo:
Recorded the precompute acknowledgement and activation-metadata prerequisites in 9c18ede. Local checks confirm that Android rejects missing or mismatched acknowledgements and recovers after a valid response. These checks use supplied responses and do not confirm backend deployment. Release enablement still requires verified site coverage for the acknowledgement, fallback, and activation changes linked here.
This release gate remains open until site deployment is verified. No backend deployment was performed in this SDK work.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c18edeca8
ℹ️ 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".
leoromanovsky
changed the title
|
🤖 Comment from Claude working fully autonomously overnight: Heads up: Please consider adding the run details to the Validation row. The Validation row names the tested commit |
This comment was marked as off-topic.
This comment was marked as off-topic.
Handle superseded completions and initialization timeout state. Preserve bridge error telemetry and add context-bound snapshots, regression coverage, and delivery documentation. Ref: FFLSDK-258
|
🤖 Comment from Codex working with @leoromanovsky: Addressed the description feedback. The description now lists behavior changes that affect every mode and states the |
|
🤖 Comment from Codex working with @leoromanovsky: Regarding the run provenance: the retained manifests identify Android commit |
|
🤖 Comment from Codex working with @leoromanovsky: Acknowledged the release prerequisites. The description retains acknowledgement, ordinary fallback, and activation delivery as prerequisites. This feedback pass does not establish backend deployment or completion of the companion work. It adds the Android snapshot API; the React Native caller change remains outstanding. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d57dd98e1d
ℹ️ 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".
Ref: FFLSDK-258
leoromanovsky
added
the
ready-for-human-review
ChatGPT Codex Connector
|
🤖 Comment from Claude working with Aaron Silverman: [P3] Two small corrections to the PR description:
|
aarsilv
left a comment
aarsilv
left a comment
There was a problem hiding this comment.
Thanks for iterating! Only a few p3/nits remain you can take or leave as you see fit
FFE reviewer selectionReviewers: @typotter, @sameerank Why: android — The PR adds Android Flags SDK experimentation opt-out modes (DISABLED, IMMEDIATE, FUTURE_EXPERIMENTS_ONLY). Reviewers come from the matched pod. 🤖 Automated selection. Selection detailsInitiative: fallback / android Why this match: The PR adds Android Flags SDK experimentation opt-out modes (DISABLED, IMMEDIATE, FUTURE_EXPERIMENTS_ONLY). These modes exclude experiment allocations based on a subject opt-out timestamp, and they bind caches and request lifecycle to the policy. Evaluation and exposure telemetry controls stay independent, so this changes allocation eligibility, not event-egress suppression. ufc-compliance explicitly excludes that kind of change. It is also not layers or mutual exclusion (experimentation-decisioning), holdouts, or bandits. No registered project fits, so the change routes to the Android SDK fallback.
Additional review: openfeature-architecture — The OpenFeature provider's lifecycle and error contract changes for all modes, including DISABLED. Provider error events for non-timeout failures now map to GeneralError instead of ProviderFatalError. A new FlagsInvalidContextException maps to InvalidContextError. A superseded context request now surfaces as CancellationException from initialize/onContextSet. The context conversion coerces a non-string opt-out attribute, and the new ErrorCode.INVALID_CONTEXT maps to OpenFeature INVALID_CONTEXT. Coverage: @typotter. Evidence: features/dd-sdk-android-flags-openfeature/src/main/kotlin/com/datadog/android/flags/openfeature/DatadogFlagsProvider.kt, features/dd-sdk-android-flags-openfeature/src/main/kotlin/com/datadog/android/flags/openfeature/internal/FlagsClientExt.kt, features/dd-sdk-android-flags-openfeature/src/main/kotlin/com/datadog/android/flags/openfeature/internal/adapters/Converters.kt The PR author was excluded. Existing reviewers and teams were preserved. Debug or fix a problemWorkflow run · Bits Code session
Evidence and run details✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head. Generated asynchronously by the FFE reviewer-routing workflow. Changed paths: Evaluated head: Registry revision: Pinned skill source: Instructions: workflow-injected snapshot (no plugin loaded) Instruction snapshot SHA-256: |
Preserve newer failures at initialization timeout, improve rejection telemetry, and cover lifecycle and request contracts. Ref: FFLSDK-258
Ref: FFLSDK-258
|
🤖 Comment from Codex working with @leoromanovsky: Updated the description in response to these corrections. The future-timestamp statement now applies explicitly to |
There was a problem hiding this comment.
Approving the PR.
However there are several things that I'd like to mention that potentially can be improved in the future. Not blocking the PR, just some observations from me.
- In this PR we introduce a new thread in the flags module here. Threads are expensive and ideally should be reused as much as possible across the SDK. This isn't the first time in the flags module, for example here. This is a topic for this ticket I believe: RUM-16893.
EvaluationsManageris pretty hard to read by a human because of very non-linear control flow because of a lot of async logic and a lot of very granular locking. The async part is normally solved (IMHO) by using some sort of RxJava, Kotlin coroutines/flow frameworks or maybe something custom. Indeed we don't have anything like this in the codebase, because we generally haven't ever had this kind of complex async code before. For the locking part - hard to say, but I find it very unlikely that the flags logic actually requires this kind of complexity.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1504f4dc5
ℹ️ 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".
| val error = FlagsInvalidContextException(ExperimentationOptOut.INVALID_CONTEXT_MESSAGE) | ||
| synchronized(initializationTerminalLock) { | ||
| experimentationOptOut.ifCurrent(request) { enqueueState(FlagsClientState.Error(error)) } | ||
| enqueueCompletion(InitializationResult(callback), error) |
There was a problem hiding this comment.
When two threads set contexts concurrently, a malformed request can be replaced after prepare() records it but before this branch acquires initializationTerminalLock. Although ifCurrent(request) then correctly suppresses the stale Error state, the callback is still completed with FlagsInvalidContextException; this contradicts the documented behavior that a request replaced before its result is processed receives FlagsContextSupersededException, and can make coroutine callers treat a cancelled update as an input-validation failure. Select the completion error based on the result of the current-request check, as the fetch success and failure paths already do.
Useful? React with 👍 / 👎.
| private fun complete(result: InitializationResult?, error: Throwable? = null) { | ||
| effects.notifySafely { result?.cancelTimeout?.invoke() } | ||
| val callback = result?.callback ?: return | ||
| callbackExecutor.executeSafe("Deliver evaluation context callback", internalLogger) { |
There was a problem hiding this comment.
When callback delivery is delayed—for example, because an earlier customer callback is blocking this serial dispatcher—the client state can change after an initialization timeout is committed but before its callback runs. setEvaluationContextSuspend decides whether that timeout is usable by reading the state at callback-delivery time, so a timeout committed as Stale with matching cached assignments can incorrectly fail after a newer request moves the state to Reconciling or Error; conversely, a timeout committed as Error can incorrectly succeed if a later request becomes ready first. Preserve the timeout's cache/state outcome when it is claimed rather than deferring that decision across this asynchronous dispatch.
Useful? React with 👍 / 👎.
| operationName = FETCH_AND_STORE_OPERATION_NAME, | ||
| internalLogger = internalLogger | ||
| ) { | ||
| completionAwareExecutor(onFetchFinished).executeSafe(FETCH_AND_STORE_OPERATION_NAME, internalLogger) { |
There was a problem hiding this comment.
When the core backpressure queue reaches capacity—for example, with a slow assignment request and a small configured capacity—the default IGNORE_NEWEST mitigation reports a successful offer after discarding this network task, so executeSafe cannot detect the loss. In an enabled opt-out mode, prepare() has already made the discarded request current; every older queued result is then rejected as superseded, while the latest callback never fires, resolution remains PROVIDER_NOT_READY, and onFetchFinished never releases its per-subject telemetry bookkeeping. Submit context updates through a non-dropping path or arrange for dropped tasks to complete and clean up explicitly.
Useful? React with 👍 / 👎.
What does this PR do?
Add immediate and future-experiments-only opt-out to
FlagsClient.Builder. The OpenFeature provider uses the same configuration.DISABLED, the defaultIMMEDIATEFUTURE_EXPERIMENTS_ONLYOmit the timestamp for eligible subjects. Invalid enabled input returns
INVALID_CONTEXTwithout an assignment request. InIMMEDIATEmode, future timestamps take effect immediately.Require acknowledgement of the effective policy and, for future mode, the exact cutoff. Bind caches to the mode and context. Enabled modes withhold older contexts during refresh, including when the timestamp is absent. Flag resolution waits for persistence once before acquiring the policy lock. Request cleanup uses a nonblocking snapshot.
A client-owned queue delivers callbacks on background workers. Each worker shuts down after its drain task is submitted. A blocked fetch cannot delay initialization timeout callbacks. Callback exceptions cannot prevent later delivery. Non-timeout client errors now map to
GeneralErrorinstead ofProviderFatalErrorthrough OpenFeature. Invalid opt-out timestamps map toInvalidContextError.Motivation
Applications need to exclude subjects from experimentation while retaining ordinary feature delivery. Experimentation and evaluation telemetry controls remain independent. Future mode uses allocation activation, not prior subject enrollment. Retained allocations still perform normal targeting and exposure logging.
In enabled modes, the three granular tracking bridge methods reject snapshots from superseded contexts. Timestamp documentation describes the supported RFC3339 subset. Tests cover both modes, mode-only acknowledgement rejection, invalid-context mapping, and worker-start recovery.
Changes that also affect
DISABLEDclients:DISABLEDclients emit a once-only telemetry warning when they reject a response marked as experimentation-filtered.ErrorCode.INVALID_CONTEXTis a new enum constant.In enabled modes, requests replaced before result processing complete as cancellation, whether fetching succeeds or fails, unless timeout already completed them. An initialization timeout preserves a newer request’s existing error.
The disabled-mode not-ready message is preserved. Tests cover cancellation mapping, timeout state, bridge telemetry, acknowledgement, cache separation, and snapshot ownership.
Additional Notes
The historical 187-case receipt does not bind its result to an immutable profile and harness revision. Reproducible shared-profile evidence remains outstanding. Backend acknowledgement, ordinary fallback, and activation delivery remain release prerequisites. Companion mobile work remains outstanding.
Validation
DataDog/dd-sdk-android@d95597bc31e78c280547cb33e6c10f242df21e06Review checklist (to be filled by reviewers)
FFE reviewer selection
Reviewers: @typotter, @sameerank
Why: android — The PR adds an Android Flags SDK experimentation opt-out mode (ExperimentationOptOutMode, set via FlagsClient.Builder and the OpenFeature provider). Reviewers come from the matched pod.
🤖 Automated selection.
Selection details
Initiative: fallback / android
Why this match: The PR adds an Android Flags SDK experimentation opt-out mode (ExperimentationOptOutMode, set via FlagsClient.Builder and the OpenFeature provider). It uses the dd_experimentation_opt_out_at context attribute to exclude subjects from experiment allocations, either immediately or only for allocations activated after the cutoff. It also changes the precompute mapping, request and cache binding, and callback dispatch to match. No registered project fits this behavior. The PR keeps evaluation and exposure telemetry controls separate and still logs exposures for retained allocations, so it is not ufc-compliance's per-context event suppression. It is also not layers or mutual exclusion (experimentation-decisioning), holdouts, or bandits. The change is Kotlin in the Android flags and OpenFeature modules, so it routes to the Android fallback.
The PR author was excluded. Existing reviewers and teams were preserved.
Debug or fix a problem
Workflow run · Bits Code session
dry_run=false. Unchanged completed runs are skipped. If the receipt saysneeds_reconciliationor a write outcome is uncertain, ask the workflow owner to reconcile GitHub and the receipt before retrying; do not delete the lock blindly.Evidence and run details
✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head.
Generated asynchronously by the FFE reviewer-routing workflow.
Changed paths:
features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/ExperimentationOptOutMode.kt,features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/ExperimentationOptOut.kt,features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/repository/net/PrecomputeMapper.kt,features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt,features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/evaluation/EvaluationsManager.kt,features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt,features/dd-sdk-android-flags-openfeature/src/main/kotlin/com/datadog/android/flags/openfeature/DatadogFlagsProvider.ktEvaluated head:
0a4f91eed3bdd0f1b2f8800e30dd72776e6d6e71Registry revision:
revision:1946aa2a-530e-487a-84a7-83aff44a1089Pinned skill source:
d214cfc65ae154b97d39af5cf32caf48aaec9bf0Instructions: workflow-injected snapshot (no plugin loaded)
Instruction snapshot SHA-256:
f23b2a88c13c0da90e2dd58246a14f257af91e2195c555039bd555b52e6f3516