Repository navigation
Conversation
Capture consent with assignments and preserve it through cache, evaluation and aggregation. Validate native and Kotlin provider paths using release AARs. Ref: FFL-3430 Ref: FFL-3434 Environment: Datadog workspace
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: d414bca | Docs | View more details | Give us feedback! |
Exercise native and Kotlin provider release builds with real Android storage, scheduled uploads, HTTP retries, and consent refresh. Run the suite in the existing API 23, 28, and 37 emulator jobs. Ref: FFL-3434 Environment: Datadog workspace
Use the artifact dependency mode for the emulator suite and record native/provider hashes in each CI job. Document the connected-device task. Ref: FFL-3434 Environment: Datadog workspace
Escape unpaired surrogates before UTF-8 cache storage so restored evaluations omit malformed targeting keys without losing counts. Exercise the byte boundary and offline telemetry in regression tests. Ref: FFL-3430 Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Use the sanitized aggregation error code directly, centralize consent field names, and clean up documentation and obsolete test state. Ref: FFL-3430 Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Keep consent with the atomic evaluation snapshot and preserve strict parsing through generated cache models, obfuscated assignments, and lossless Unicode persistence. Cover consent and subject opt-out together. Ref: FFL-3430 Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Keep nullable telemetry identity separate from the assignment subject while preserving snapshot consent, exposure attribution and opt-out. Ref: FFL-3430 Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
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. |
There was a problem hiding this comment.
More details
Consent propagation, cache restoration, telemetry sanitization, and snapshot replay appear consistent with the intended privacy behavior. Runtime behavior remains unverified because this review used static evidence only.
🤖 Bits Code Review · Commit 7e5a7ea
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5a7eab41
ℹ️ 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".
| private const val HEX_DIGITS = "0123456789abcdef" | ||
|
|
||
| fun targetingKey(value: String?, observeFullEvaluationData: Boolean): String? { | ||
| if (value == null || observeFullEvaluationData || value.isEmpty()) return value |
There was a problem hiding this comment.
Validate malformed keys before honoring full-data consent
When observeFullEvaluationData=true and a targeting key contains an unpaired UTF-16 surrogate, this early return bypasses strict UTF-8 validation and forwards the malformed string to JSON/byte serialization, where it can be replacement-encoded instead of omitted. This contradicts the newly documented contract in features/dd-sdk-android-flags/README.md that malformed Unicode keys are omitted without losing the evaluation; validate the string before returning the raw full-data value.
Useful? React with 👍 / 👎.
| val digest = MessageDigest.getInstance("SHA-256") | ||
| digest.update(bytes) | ||
| "sha256_" + digest.digest().joinToString("") { byte -> |
There was a problem hiding this comment.
Cache the protected targeting-key digest
With the default trackEvaluations=true, absent/false consent, and a nonempty targeting key, every flag resolution performs UTF-8 encoding, a crypto-provider lookup, SHA-256, and a 32-element joinToString on the caller thread. Public flag checks can run repeatedly on the UI thread, so recomputing the same subject digest for every evaluation adds avoidable CPU work and allocations; compute it once when the atomic assignment/context state is installed and reuse it during aggregation.
AGENTS.md reference: AGENTS.md:L142-L142
Useful? React with 👍 / 👎.
|
@hamorillo is out of office, assigning @ambushwork as next week IH. |
|
Waiting for codex comments and merge conflicts to be addressed before human reviewing. |
| private const val HEX_DIGITS = "0123456789abcdef" | ||
|
|
||
| fun targetingKey(value: String?, observeFullEvaluationData: Boolean): String? { | ||
| if (value == null || observeFullEvaluationData || value.isEmpty()) return value |
There was a problem hiding this comment.
nit:
value.isNullOrEmpty()| "" | ||
| } | ||
|
|
||
| private fun escapeUnpairedSurrogates(json: String): String { |
There was a problem hiding this comment.
could be fun String.escapeUnpairedSurrogates() : String
Validate telemetry identities in both consent modes and reuse prepared keys. Preserve captured bridge consent, merge develop, and extend device coverage. Ref: FFL-3430 Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
domalessi
left a comment
There was a problem hiding this comment.
Some editorial feedback inline. Approving so you're not blocked on me!
|
|
||
| ## React Native handoff | ||
|
|
||
| Native Android owns evaluation telemetry protection. The internal assignment snapshot can now |
There was a problem hiding this comment.
| Native Android owns evaluation telemetry protection. The internal assignment snapshot can now | |
| Native Android owns evaluation telemetry protection. The internal assignment snapshot can |
| bridge overloads with retained consent, successful and default/error evaluations, unknown error | ||
| codes, distinct omitted and empty telemetry keys, and no exposure or RUM side effects. The bridge | ||
| fixtures use `doLog=true` to prove that evaluation-only hooks do not trigger exposure logging. | ||
| `FlagsPrivacyTest` uses `doLog=false` to ensure it cannot suppress evaluation telemetry. Existing |
There was a problem hiding this comment.
| `FlagsPrivacyTest` uses `doLog=false` to ensure it cannot suppress evaluation telemetry. Existing | |
| `FlagsPrivacyTest` uses `doLog=false` to verify that it cannot suppress evaluation telemetry. Existing |
| protected native release before shipping its integration. SDK-owner review, release cutoff and | ||
| published dependency versions remain release-owner decisions tracked by FFL-3437. |
There was a problem hiding this comment.
| protected native release before shipping its integration. SDK-owner review, release cutoff and | |
| published dependency versions remain release-owner decisions tracked by FFL-3437. | |
| protected native release before shipping its integration. |
This is a public repo. The Jira key and the internal release-process sentence don't belong in the README.
| Protected nonempty targeting keys are SHA-256 hashes of the exact UTF-8 text, prefixed with | ||
| `sha256_`. Empty keys remain empty; missing keys and malformed Unicode are omitted without losing | ||
| the evaluation in both consent modes. Valid Unicode is preserved exactly, without normalization. | ||
| Android continues to omit `context.evaluation` in both modes. This setting does |
There was a problem hiding this comment.
| Android continues to omit `context.evaluation` in both modes. This setting does | |
| Android continues to omit `context.evaluation` in both modes. The consent setting does |
|
|
||
| The Datadog Feature Flags SDK for Android allows you to evaluate feature flags and experiments in your Android application and automatically send flag evaluation data to Datadog for monitoring and analysis. | ||
|
|
||
| ## Evaluation telemetry privacy |
There was a problem hiding this comment.
This section sits above "Getting started", so readers hit privacy internals before they learn how to install and use the SDK. Consider moving it below the usage sections. Also, the "Internal bridge replay" subsection (line 24) is aimed at SDK maintainers and the React Native bridge, not app developers. Consider moving it to the reliability/single-fit/flags/README.md handoff section, where it's already covered.
| `instrumented/integration`, `reliability/core-it` and `reliability/with-backend` contain instrumented tests that run on a connected device or emulator. | ||
|
|
||
| `reliability/single-fit/*` contains E2E tests that run on the JVM without a device, e.g. `./gradlew :reliability:single-fit:rum:testReleaseUnitTest` (this is what CI runs). | ||
| `reliability/single-fit/*` contains E2E tests that run on the JVM without a device, e.g. `./gradlew :reliability:single-fit:rum:testReleaseUnitTest` (this is what CI runs). The Flags module also has an ART privacy suite: `./gradlew :reliability:single-fit:flags:connectedReleaseAndroidTest` requires a connected device or emulator. Its README documents the exact-AAR option. |
There was a problem hiding this comment.
| `reliability/single-fit/*` contains E2E tests that run on the JVM without a device, e.g. `./gradlew :reliability:single-fit:rum:testReleaseUnitTest` (this is what CI runs). The Flags module also has an ART privacy suite: `./gradlew :reliability:single-fit:flags:connectedReleaseAndroidTest` requires a connected device or emulator. Its README documents the exact-AAR option. | |
| `reliability/single-fit/*` contains E2E tests that run on the JVM without a device, e.g. `./gradlew :reliability:single-fit:rum:testReleaseUnitTest` (this is what CI runs). The Flags module also has a privacy suite that runs on a connected device or emulator: `./gradlew :reliability:single-fit:flags:connectedReleaseAndroidTest`. Its README documents the exact-AAR option. |
| not change evaluation inputs, assignment requests, exposure events, or RUM integration. | ||
|
|
||
| Evaluation telemetry error messages contain stable error codes in both consent modes, with | ||
| `GENERAL` for unknown failures. `trackEvaluations(false)` disables this track; assignment `doLog` |
There was a problem hiding this comment.
| `GENERAL` for unknown failures. `trackEvaluations(false)` disables this track; assignment `doLog` | |
| `GENERAL` for unknown failures. `trackEvaluations(false)` disables this track. Assignment `doLog` |
| ### Internal bridge replay | ||
|
|
||
| Native Android owns telemetry validation and hashing. The installed configuration precomputes | ||
| its telemetry identity, so repeated flag evaluations reuse it; retained evaluations keep that |
There was a problem hiding this comment.
| its telemetry identity, so repeated flag evaluations reuse it; retained evaluations keep that | |
| its telemetry identity, so repeated flag evaluations reuse it. Retained evaluations keep that |
| time, including errors without a flag, and pass null for an absent telemetry identity or an | ||
| empty string for an explicitly empty identity. The original four-argument overload derives | ||
| consent from the supplied flag and defaults to protected mode when it is unavailable. | ||
| Neither overload emits exposures or RUM annotations; use the separate hooks for those tracks. |
There was a problem hiding this comment.
| Neither overload emits exposures or RUM annotations; use the separate hooks for those tracks. | |
| Neither overload emits exposures or RUM annotations. Use the separate hooks for those tracks. |
What does this PR do?
Protect Android flag evaluation telemetry by default and validate the Kotlin OpenFeature path in the same repository.
data.attributes.observeFullEvaluationData=trueas opt-in; persist it with assignments, including empty configurations and offline cache restoration.sha256_plus lowercase hex); preserve empty keys, omit missing/malformed keys without dropping counts, and omit evaluation context. Android's existing context omission in full-data mode remains unchanged.GENERALfor unknown failures, and separate consent modes in aggregation without retaining discarded context.Motivation
FFL-3430 and FFL-3434, following the October 21 client decisions in FFL-3427 and FFL-3437. Normal configuration refresh governs consent. This does not add new context bounds, queues, degradation tiers, or server-parity requirements.
Additional Notes
Review follow-up —
d414bca614c1f255738c5522b1892bf50f93df98develop, preserve the separate native tracking hooks, apply both style suggestions, and regenerate the internal API surface.Validation
CI=true(warnings as errors). Changed library Detekt and Android lint passed; formatting passed in all three modules. Both libraries passed all five generated-file checks.reliability/single-fit/flags/README.md.Android emulator validation
Verified 48/48 privacy instrumentation tests passed, with zero failures or skips, on
d414bca614c1f255738c5522b1892bf50f93df98. All 16 expected direct SDK/OpenFeature cases were present on each API level.Static checks, unit/coverage checks, API compatibility, and all single-feature integration jobs also passed. The broader pipeline still has legacy/NDK instrumentation jobs running.
Current pipeline
The device suite uses real Android core, disk persistence, scheduled aggregation/upload and HTTP requests to a loopback server. It covers direct SDK and OpenFeature paths, consent refresh, stable errors, malformed/empty/valid Unicode keys in both modes, opt-out, cache restoration (including malformed keys in both modes), and a real 503 retry. CI builds and copies both release AARs, logs their hashes, and supplies them through
flagsArtifactDirectory. Offline restart cases invoke the provider directly to isolate cached-state behavior from OpenFeature initialization policy. Shutdown cases use the feature stop callback and core test drain hook. This does not claim live backend acceptance or process-kill recovery.Exact local artifacts — version
3.16.0-SNAPSHOT, built from the validated source tree:21a6826e18e252c0ab81a8ee12458a21b938368b73b366154ee6ae983f93508b.02393e3612dedd1f2b12625be4a9541e6f22dc1cae29814df9c6b0e2308238ba.React Native handoff
The optional internal
UnparsedFlagWithConsentpreservesUnparsedFlagABI compatibility. The bridge must serialize the consent boolean, preserve it through the JS snapshot/cache, and reconstruct the consent-aware interface when replaying. Old bridge implementations remain protected; native Android owns validation and hashing.Existing snapshot replay signatures remain intact, including the nullable
telemetryTargetingKeyoverload for React Native #1510. The separatetrackFlagEvaluationEventhook now also has an overload accepting captured consent and a nullable telemetry identity. Use that overload for errors without assignments; the original four-argument hook derives consent from the supplied flag and safely defaults to protected mode when unavailable. Exposure attribution and subject opt-out continue to use the original context. Evaluation-only hooks emit no exposures or RUM annotations.No React Native, JS, or iOS source is changed here. SDK-owner review, release cutoff, and published native/provider dependency versions remain release-owner decisions tracked by FFL-3437.
Review checklist (to be filled by reviewers)