Repository navigation
[Feature Flags] Bind discovery results to the Agent endpoint - #9352
Merged
Merged
Conversation
Invalidate cached capabilities on endpoint replacement and reject stale responses before publishing configuration or hashes. Report EVP identity-header forwarding support without changing product routing. Environment: Datadog workspace
This was referenced Oct 1, 2026
leoromanovsky
marked this pull request as ready for review
October 1, 2026 22:52
leoromanovsky
requested review from
zacharycmontoya
and removed request for
a team
October 1, 2026 22:52
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: b16a7dcc32
ℹ️ 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".
BenchmarksBenchmark execution time: 2026-10-07 17:03:01 Comparing candidate commit 12a4638 in PR branch Found 0 performance improvements and 12 performance regressions! Performance is the same for 60 metrics, 0 unstable metrics, 76 known flaky benchmarks, 50 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9352) and master. ✅ No regressions detected |
typotter
approved these changes
Oct 2, 2026
vjfridge
approved these changes
Oct 2, 2026
vjfridge
left a comment
vjfridge
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working to preserve the EVP origin headers!!
This was referenced Oct 5, 2026
Keep the discovery snapshot get-only, consistent with the other Agent configuration properties. Environment: Datadog workspace
bouwkast
approved these changes
Oct 8, 2026
anna-git
approved these changes
Oct 8, 2026
leoromanovsky
added a commit
that referenced
this pull request
Oct 9, 2026
…er (#9353) ## Motivation Short-lived applications can lose exposures queued while another batch is being sent. Shutdown currently signals the worker without waiting for a final drain, and a concurrent evaluation can enqueue after shutdown begins. ```mermaid flowchart LR E[Evaluations] --> Q[Exposure queue] S[Shutdown] -. missing final drain .-> Q Q --> A[Agent EVP v2] ``` ## Changes and Decisions Separate the existing local sender from exposure batching, give the Feature Flags module ownership of its lifetime, and dispose its settings subscriptions. Exposure shutdown rejects new enqueue attempts, drains queued events and waits at most ten seconds for the worker; the timed wait does not depend on an available ThreadPool worker, so a stalled network cannot indefinitely block exposure disposal. This PR targets master following the merge of [discovery #9352](#9352). Both configuration modes keep their historical fixed EVP v2 route, headers and redirect defaults. There is no discovery-based selection or direct-intake activation here; the complete Agentless policy remains in [#9235](#9235). ```mermaid flowchart LR S[Shutdown] --> R[Reject new exposures] R --> F[Final queue drain] F --> T[Existing fixed-v2 sender] T --> A[Local Agent] F --> B[Bounded wait] ```
leoromanovsky
added a commit
that referenced
this pull request
Oct 15, 2026
## Motivation
Agentless configuration lets customers evaluate flags without running an
Agent, but exposure delivery still requires its local EVP proxy.
Evaluations can succeed while their experiment exposures are lost.
```mermaid
flowchart LR
C[Agentless configuration] --> E[Flag evaluation]
E --> X[Exposure batch]
X --> A[Local Agent required]
X -. missing route .-> D[Direct EVP intake]
```
## Changes and Decisions
This is the single-commit final activation after [endpoint-bound
discovery #9352](#9352)
and [bounded exposure lifecycle
#9353](#9353). It keeps
Remote Config on its historical fixed EVP v2 behavior and does not add
flag-evaluation emission.
Agentless delivery prefers local EVP v4, then v2, only when discovery
confirms forwarding of both SDK identity headers. Configured Agent path
prefixes are preserved. Otherwise it uses canonical HTTPS intake; API
keys remain direct-only, relay headers local-only, and redirects are
disabled for Agentless delivery.
Direct selection is sticky. An unavailable route has bounded recovery;
each batch has bounded discovery waiting. The current batch can move to
direct only after local 404/405 or a proven pre-send connection failure.
Ambiguous failures never replay that batch, preventing duplicate
exposures. Identity, credentials and this replay policy ship together.
```mermaid
flowchart LR
E[Agentless exposure batch] --> R{Selected route}
R -->|Identity-capable v4 or v2| A[Local relay]
R -->|No eligible relay| D[Direct HTTPS intake]
A -->|404 / 405 or proven pre-send failure| D
A -->|Ambiguous failure| N[No current-batch replay]
D --> S[Direct remains selected]
```
vjfridge
added a commit
that referenced
this pull request
Oct 19, 2026
…gh the Agent (#9351) ## Pre-merge blocker: no-Agent behavior - [#9352](#9352) and [#9353](#9353) are merged and incorporated into this branch. [#9235](#9235) remains open; revisit no-Agent `flagevaluation` delivery when that transport work lands. - The related shared/exposure transport work does not yet wire up no-Agent `flagevaluation` emission. Before merging this PR, decide and validate transport reuse or an explicit no-Agent gating policy. The current implementation remains Agent-only and can build/queue events that cannot be delivered without an Agent. [Review discussion](#9351 (comment)). ## Summary of changes Adds aggregated `flagevaluation` track EVP events for successful and unsuccessful Datadog OpenFeature evaluations, protecting targeting keys and context by default without changing flag results. - **Agent-only delivery today**, with the no-Agent decision blocked as described above; related transport: [#9235](#9235); replaces the stale [#9139](#9139) approach. - Follows merged [Go](DataDog/dd-trace-go#5151), [Java](DataDog/dd-trace-java#12042), [Python](DataDog/dd-trace-py#20354), and [Ruby](DataDog/dd-trace-rb#6222) implementations, with [PII guidance](https://github.com/DataDog/ffe-codegen-tools/pull/9) and [JS](DataDog/dd-trace-js#10464) as additional references. ## Key decisions / assumptions - **Private by default.** Hash valid targeting keys at output, omit context, and allowlist error codes. Only boolean `true` for `observeFullEvaluationData` allows raw keys and bounded context; consent and timestamp are captured once per evaluation. - **One consent value across merged RCM configs.** Full data requires consent from every currently active file (AND), recomputed on each update—not combined with historical configurations. Any false, absent, null, or malformed value selects protected mode. Existing flag-merging behavior stays unchanged. - **Background processing.** The caller captures metadata/bounded context and enqueues without waiting for space. A dedicated worker aggregates and sends, batching partial queues for up to 50 ms. Explicit flush and shutdown wake it immediately; periodic sends remain every 10 seconds. Flush/shutdown waits are bounded. The five-second request timeout does not bound legacy pipe/socket connection setup; that existing shared-transport gap is [deferred from this PR](#9351 (comment)). - **Stable service tags.** Capture service/env/version when the writer starts, after replaying the latest settings, and keep them for its lifetime—as Go/Java/Ruby/JS do. Later tag changes require writer recreation; Agent-address updates remain independent. - **Bounded entries, not a fixed memory ceiling.** Queue: **4,096**, matching Go/Java/Python/Ruby; full buckets: 10,000 per flag / 131,072 globally; degraded buckets: 32,768. Degraded events omit key/context and exclude consent from bucket identity; exhausted capacity drops telemetry, not evaluations. - **Bounded consented context.** Capture at most 256 fields with depth/size/visit limits, without scanning or sorting arbitrarily large inputs. Rare hash collisions in oversized maps can change the retained subset; only telemetry is affected. - **Scoped transport.** Use the Agent’s EVP v2 route, with no direct-intake fallback, discovery, or retries. Delivery is best-effort; Agent 7.84.0 preserved SDK identity headers in testing, while 7.83.1 stripped them. ## Measured performance overhead **Historical comparison: `b32fca42cc` vs master `f0114c8c71`, before the worker-batching change; 4,096-entry queue.** ARM64 Linux/glibc, .NET 10.0.12; 100,000-evaluation bursts, medians of three runs. CPUs are affinity slots, not quotas; process CPU/allocation include final flush, caller latency does not. | CPUs | Mode | Evaluation p50 / p95 (µs) | CPU (µs/evaluation) | Allocation (bytes/evaluation) | | --- | --- | --- | --- | --- | | 1 | Baseline | 48.47 / 60.21 | 50.47 | 6,088 | | 1 | Events disabled | 55.93 / 68.32 | 56.44 | 6,584 | | 1 | Default protected | 67.77 / 112.38 | 79.26 | 6,837 | | 1 | Full consent | 70.08 / 139.30 | 91.60 | 11,472 | | 2 | Baseline | 10.85 / 11.38 | 11.86 | 6,088 | | 2 | Events disabled | 12.23 / 12.79 | 13.49 | 6,584 | | 2 | Default protected | 23.42 / 27.38 | 34.74 | 6,836 | | 2 | Full consent | 28.18 / 31.96 | 43.64 | 11,470 | - Default protected mode adds **~19 µs / ~13 µs** median evaluation latency and **~57% / ~193%** process CPU per evaluation on one/two CPUs. Disabling events avoids the writer but is **not zero-overhead**. - Consented contexts add allocation. All **88 trials** passed the delivery/privacy audit, including paced, sparse, high-cardinality, and oversized-context workloads; **all 3,136,867 expected observations, including warmup, were retained**. - These synthetic measurements make the cost visible; they do not establish acceptable production overhead. .NET reviewer assessment remains open. A later [paired worker-batching comparison](#9351 (comment)) improved protected burst latency/CPU, with a documented overload/drop tradeoff; it is not a fresh comparison against master. Hook-list reuse ([FFL-3356](https://datadoghq.atlassian.net/browse/FFL-3356)) is now included through merged [#9334](#9334); the historical measurements above predate that change. [Historical methodology, full workload matrix, and memory results](#9351 (comment)). [Earlier experiments and validation history](#9351 (comment)) used different artifacts/settings and are not an isolated optimization comparison. ## Test coverage - **Latest local post-merge validation at [`cef427882a`](cef427882a854bad0710542bacc107d26191dd65):** targeted feature-flag, telemetry, and metadata tests: **1,312 passed / two skipped on each of .NET 10 and .NET Core 3.1**. Tracer builds passed for all four target frameworks with zero warnings/errors. Subsequent commits changed only CODEOWNERS; these runs do not replace final-head external/performance validation. - **At `7fe8e0651f`:** 780 feature-flag tests passed / two platform skips; benchmark project built with zero warnings/errors. Full tracer suite: **13,507 passed / 71 skipped / zero failed**. [Writer-attribution fix and regression coverage](7fe8e06). - **At `b32fca42cc`:** 746 feature-flag tests passed / one skipped; full tracer suite 13,473 passed / 70 skipped / zero failed. Benchmark project built without warnings/errors. - **[System-tests at `b32fca42cc`](#9351 (comment) all 51 Agent-only FFE tests passed, including all 12 EVP cases; baseline passed the other 39 and failed only those 12 unimplemented cases. All 12,000 degradation observations retained. - **[Dogfooding at `b32fca42cc`](#9351 (comment) identical results across 81 live, 24 controlled-consent, and six no-Agent requests. Verified emission, SDK identity, strict consent, and mixed-version/uninstrumented safety. - Signed companion harness fixes: [system-tests #7951](DataDog/system-tests#7951) and [ffe-dogfooding #131](https://github.com/ddoghq/ffe-dogfooding/pull/131), both awaiting review. [Earlier evidence](#9351 (comment)) is historical. ### Reviewer QA Compare flag results with emission on/off and with the Agent disconnected. Verify strict consent, protected payloads, and unchanged exposure/span-enrichment behavior; disabling emission must produce no new `flagevaluation` events. ## Blast radius - Updates the provider/tracer metadata bridge and adds a hook, worker, Agent requests, and diagnostic metrics. Both updated components are required for full functionality; mixed-version and uninstrumented behavior have dedicated tests. - Events default on when feature flagging is active; `DD_FLAGGING_EVALUATION_COUNTS_ENABLED=false` disables them. Hook registration is decided when the provider is constructed; enabling events after starting disabled requires provider recreation or an application restart. Adds CPU/allocation and best-effort shutdown work; consented snapshots retain more data. - No changes to targeting rules, returned values, exposure payloads, or configuration-fetching transport. Exposure Agent requests also receive timeout protection, subject to the legacy connection-setup limitation above. Existing Remote Config and Agentless configuration sources remain supported; event delivery is Agent-only. ## External follow-ups External/performance results above predate the latest review fixes and need a final-head refresh. Remaining coordination, in addition to the no-Agent blocker above: - Release owner: confirm provider/tracer release versions. - Companion harness PRs [#7951](DataDog/system-tests#7951) / [#131](https://github.com/ddoghq/ffe-dogfooding/pull/131): review and manual merge. [FFL-3356]: https://datadoghq.atlassian.net/browse/FFL-3356?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Changing the Agent endpoint at runtime must not make the SDK trust capabilities learned from the previous Agent. Cached configuration and late responses currently cross that boundary, which can misroute telemetry or lose its producer identity.
flowchart LR A[Old Agent request] --> C[Cached capabilities] E[Endpoint changes] -. missing invalidation .-> C C --> B[Replacement Agent]Changes and Decisions
Bind each discovery result to the immutable exporter settings used for its request. Replacing the endpoint clears cached configuration and refresh hashes; late responses from the old endpoint cannot publish capabilities or hashes. An identical response from the replacement still notifies consumers.
Discovery also reports whether the relay forwards both EVP producer-identity headers. This exposes a capability; it does not change any product's route selection or enable direct intake.
This is the foundation of the delivery stack: discovery here, bounded exposure lifecycle in #9353, then Agentless fallback in #9235.
flowchart LR E[Current exporter settings] --> R[Discovery request] R --> G{Still current?} G -->|Yes| C[Publish capabilities for this endpoint] G -->|No| D[Discard old response]