FFLSDK-272: Send additional Flagging state telemetry on flag requests by typotter · Pull Request #4030 · DataDog/dd-sdk-android

typotter · 2026-10-22T15:50:46Z

What does this PR do?

Send 3 new fields in the precomputed assignments POST:

sdk.instance_id - uniquely identifies the in-memory configuration-owning client instance
fetch.sequence - a logical fetch operation in the SDK's lifecycle
fetch.trigger - the SDK's internal accounting of what triggered the fetch

Motivation

Distinguish configuration-owning SDK instances and logical fetches when investigating repeated assignment requests in edge request logs.

Implementation

EvaluationsManager owns the UUIDv7 instance ID and fetch sequence. Each accepted fetch captures an immutable diagnostics value, which travels with that fetch to the existing request factory. The factory adds its fields directly to the structured request body and serializes the complete body once.

The in-package UUIDv7Generator supplies the instance ID. A diagnostics trigger enum uses initialization and context_change, consistently across SDKs.

Additional Notes

Local validation: all 613 Flags unit tests, Detekt, ktlint and API checks passed. The API check used the unchanged generated reference with apiDump excluded to avoid the existing Gradle task-ordering conflict. Tests cover instance identity, logical sequencing, concurrent acceptance and the complete request body. Verify deployed edge/proxy acceptance and the EVP schema prerequisite before SDK release.

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

linear-code · 2026-10-22T15:50:52Z

FFLSDK-272

chatgpt-codex-connector · 2026-10-22T15:52:48Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-10-23T18:45:04.096574Z	`61f6dce`	New commits
🔒 Security Review	✅ Completed 2026-10-23T18:48:45.489009Z	`61f6dce`	New commits

ℹ️ 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" or "@codex security review".

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05fcdef98b

ℹ️ 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".

chatgpt-codex-connector · 2026-10-22T15:56:59Z

+            val snapshots = executor.invokeAll((0 until 64).map { Callable { JSONObject(owner.accept()!!) } })
+                .map { it.get().getJSONObject("fetch") }


Bound the concurrency test's waits

If accept() develops a deadlock, invokeAll and the subsequent Future.get() calls wait indefinitely, so this concurrency regression test can hang the Gradle worker instead of failing. Use timed task submission or bounded get/latch waits so the test terminates with a useful failure.

AGENTS.md reference: AGENTS.md:L197-L197

Useful? React with 👍 / 👎.

+1 to bounding this wait. We would classify this as P3 rather than P1: no current production deadlock is demonstrated; the impact is a regression test hanging a developer or test worker if accept() stops completing. We would use the timed invokeAll overload, fail explicitly if any tasks are cancelled or unfinished, and retain the finally cleanup. A timeout only on the later get() calls would not bound the preceding invokeAll wait.

datadog-prod-us1-6

More details

The diagnostics preserve existing assignment data while maintaining a stable per-client identity and synchronized fetch sequence. The completed static review identified no actionable defect, but focused tests could not execute because a required aapt2 dependency was unavailable in the offline cache.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 05fcdef}

datadog-prod-us1-6 · 2026-10-22T16:00:57Z

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 97.22%
• Overall Coverage: 71.76% (-0.12%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 61f6dce | Docs | View more details | Give us feedback!}

vjfridge

Looks like we have merge conflict to resolve and I left a couple non-blocking comments but otherwise looks good to me!

vjfridge · 2026-10-23T02:48:41Z

+    @Test
+    fun `M emit only three fields with UUIDv7 identity W accepting logical fetches`() {
+        val owner = FlagsRequestDiagnostics(1_700_000_000_000)
+        val first = JSONObject(owner.accept()!!)


Non-blocking nitpick: could we follow the test conventions here and in the other added tests—Given/When/Then sections, checkNotNull with a useful failure message instead of !!, and imports instead of fully qualified names? That would make the setup/action/assertion boundaries and unexpected-null failures easier to read.

val first = JSONObject(checkNotNull(wner.accept())

Btw would it be prettier to generate Json here, and compare strings instead?
the assertions is almost impossible to understand in such form:

val expected = JSONObject( mapOf( "sdk" to mapOf("instance_id" to instanceId), "fetch" to mapOf( "sequence" to sequence, "trigger" to trigger.wireValue ) ) ) .toString() assertThat(actual).isEqual(expected)

WDYT?

vjfridge · 2026-10-23T02:48:42Z

+            val snapshots = executor.invokeAll((0 until 64).map { Callable { JSONObject(owner.accept()!!) } })
+                .map { it.get().getJSONObject("fetch") }


+1 to bounding this wait. We would classify this as P3 rather than P1: no current production deadlock is demonstrated; the impact is a regression test hanging a developer or test worker if accept() stops completing. We would use the timed invokeAll overload, fail explicitly if any tasks are cancelled or unfinished, and retain the finally cleanup. A timeout only on the later get() calls would not bound the preceding invokeAll wait.

satween · 2026-10-23T13:32:09Z

+        // JSON operations are contained by the catch; no caller-supplied values enter the envelope.
+        @Suppress("UnsafeThirdPartyFunctionCall")
+        try {
+            JSONObject()


nit: this one looks bit more readable

try { JSONObject( mapOf( "sdk" to mapOf("instance_id" to instanceId), "fetch" to mapOf( "sequence" to sequence, "trigger" to trigger.wireValue ) ) ) .toString() .takeIf { it.toByteArray(Charsets.UTF_8).size <= MAX_ENVELOPE_BYTES } } catch (_: Exception) { null }

satween · 2026-10-23T13:35:00Z

+        datadogContext: DatadogContext,
+        diagnostics: String?
+    ): String? {
+        val request = requestFactory.create(context, datadogContext, diagnostics) ?: return null


nit:

return requestFactory.create(context, datadogContext, diagnostics) ?.let(::executeDownloadRequest)

satween · 2026-10-23T13:36:37Z

+    @Test
+    fun `M emit only three fields with UUIDv7 identity W accepting logical fetches`() {
+        val owner = FlagsRequestDiagnostics(1_700_000_000_000)
+        val first = JSONObject(owner.accept()!!)


val first = JSONObject(checkNotNull(wner.accept())

satween · 2026-10-23T13:38:17Z

+    @Test
+    fun `M emit only three fields with UUIDv7 identity W accepting logical fetches`() {
+        val owner = FlagsRequestDiagnostics(1_700_000_000_000)
+        val first = JSONObject(owner.accept()!!)


Btw would it be prettier to generate Json here, and compare strings instead?
the assertions is almost impossible to understand in such form:

val expected = JSONObject( mapOf( "sdk" to mapOf("instance_id" to instanceId), "fetch" to mapOf( "sequence" to sequence, "trigger" to trigger.wireValue ) ) ) .toString() assertThat(actual).isEqual(expected)

WDYT?

satween · 2026-10-23T13:39:17Z

        )

-        whenever(mockAssignmentsDownloader.readPrecomputedFlags(context, fakeDatadogContext))
+        whenever(mockAssignmentsDownloader.readPrecomputedFlags(eq(context), eq(fakeDatadogContext), any()))


here and below: is it possible to use expected value instead of any() ?

Capture typed diagnostics with each logical fetch and serialize them directly in the precomputed request body for edge request-log correlation.

typotter marked this pull request as ready for review October 22, 2026 15:52

typotter requested review from a team as code owners October 22, 2026 15:52

typotter requested a review from vjfridge October 22, 2026 15:52

chatgpt-codex-connector Bot reviewed Oct 22, 2026

View reviewed changes

typotter requested a review from btthomas October 22, 2026 15:59

datadog-prod-us1-6 Bot reviewed Oct 22, 2026

View reviewed changes

vjfridge previously approved these changes Oct 23, 2026

View reviewed changes

maciejburda requested a review from aleksandr-gringauz October 23, 2026 07:06

satween reviewed Oct 23, 2026

View reviewed changes

typotter dismissed vjfridge’s stale review via 5b892de October 23, 2026 16:54

typotter force-pushed the typo/android-flags-request-diagnostics branch from 05fcdef to 5b892de Compare October 23, 2026 16:54

FFLSDK-272: Add instance and fetch diagnostics to flag requests

61f6dce

Capture typed diagnostics with each logical fetch and serialize them directly in the precomputed request body for edge request-log correlation.

typotter force-pushed the typo/android-flags-request-diagnostics branch from 5b892de to 61f6dce Compare October 23, 2026 18:44

		val snapshots = executor.invokeAll((0 until 64).map { Callable { JSONObject(owner.accept()!!) } })
		.map { it.get().getJSONObject("fetch") }

Conversation

typotter commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Implementation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

linear-code Bot commented Oct 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datadog-prod-us1-6 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

datadog-prod-us1-6 Bot commented Oct 22, 2026 • edited by datadog-official Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vjfridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

typotter commented Oct 22, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 22, 2026 •

edited

Loading

datadog-prod-us1-6 Bot commented Oct 22, 2026 •

edited by datadog-official Bot

Loading