Repository navigation
[Feature Flags] Add agentless EVP fallback transport - #9235
Merged
leoromanovsky merged 11 commits intoOct 15, 2026
Merged
Conversation
BenchmarksBenchmark execution time: 2026-10-15 03:13:41 Comparing candidate commit 89e841e in PR branch Found 0 performance improvements and 12 performance regressions! Performance is the same for 60 metrics, 0 unstable metrics, 75 known flaky benchmarks, 51 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9235) and master. ✅ No regressions detected |
leoromanovsky
marked this pull request as ready for review
September 21, 2026 14:40
leoromanovsky
requested review from
anna-git,
danyal002 and
vjfridge
and removed request for
a team
September 21, 2026 14:40
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. |
leoromanovsky
requested review from
pavlokhrebto
and removed request for
danyal002
September 21, 2026 14:40
![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: 7eba078530
ℹ️ 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".
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
Preserve historical event routing while draining queued exposures safely during module shutdown. Environment: Datadog workspace
Prefer identity-capable local relays and keep a credential-isolated direct route with conservative replay and sticky selection. Environment: Datadog workspace
This was referenced Oct 1, 2026
leoromanovsky
force-pushed
the
leo.romanovsky/fflsdk-189-evp-fallback-dotnet-master
branch
from
October 1, 2026 22:29
f439192 to
8d55124
Compare
leoromanovsky
changed the base branch from
master
to
leo.romanovsky/fflsdk-189-dotnet-exposure-lifecycle
October 1, 2026 22:31
leoromanovsky
added a commit
that referenced
this pull request
Oct 8, 2026
## 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.
```mermaid
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](#9353), then
[Agentless fallback in
#9235](#9235).
```mermaid
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]
```
Base automatically changed from
leo.romanovsky/fflsdk-189-dotnet-exposure-lifecycle
to
master
October 9, 2026 00:01
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] ```
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Keep recovery state bound to its Agent endpoint, bound asynchronous direct sends on .NET Framework, and align the UDS test with the selected target framework. Environment: Datadog workspace
Block the old Agent notification between discovery subscribers and prove it cannot authorize the replacement endpoint or release its discovery waiter. Environment: Datadog workspace
Remove the UTF-8 BOM to match the repository formatter. Environment: Datadog workspace
leoromanovsky
requested review from
zacharycmontoya
and removed request for
anna-git
October 9, 2026 15:42
zacharycmontoya
approved these changes
Oct 14, 2026
zacharycmontoya
left a comment
zacharycmontoya
left a comment
Contributor
There was a problem hiding this comment.
LGTM for sdk-capabilities
Contributor
|
@leoromanovsky can you rebase your stack or otherwise pull in the latest |
Bring in the current SDK capabilities team ownership and tracer ownership corrections. Environment: Datadog workspace
Keep exception construction out of the HTTP parsing path without changing EOF classification or messages. Environment: Datadog workspace
There was a problem hiding this comment.
More details
The reviewed routing separates direct credentials from local relay headers and avoids replaying batches after ambiguous local failures. Runtime behavior could not be exercised because the required .NET 11 preview SDK is unavailable.
🤖 Bits Code Review · Commit 89e841e
anna-git
self-requested a review
October 15, 2026 08:53
anna-git
approved these changes
Oct 15, 2026
Contributor
Author
|
/merge |
|
View all feedbacks in Devflow UI.
The merge queue is disabled for this repository/branch. |
leoromanovsky
deleted the
leo.romanovsky/fflsdk-189-evp-fallback-dotnet-master
branch
October 15, 2026 17:42
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
andrewlock
added a commit
that referenced
this pull request
Oct 22, 2026
) ## Summary of changes Removes the opt-in `enforceAsyncTimeout` flag (and the unused `_asyncTimeout` plumbing) from `ApiWebRequestFactory`/`ApiWebRequest`, its callers (`AgentTransportStrategy`, `FeatureFlagsEvpTransport`, tests), and deletes `ApiWebRequestFactoryTests.AsyncDeadlineAbortsStalledJsonSendOnlyWhenEnabled`. ## Reason for change #9235 added an opt-in `Timer` + `Abort()` deadline for async `HttpWebRequest` sends. The refactor in #9145 replaces the send path, and #9144 enforces `HttpWebRequest.Timeout` for every async send, so the flag no longer does anything. The deleted test asserted behaviour specific to the opt-in design: it disables `HttpWebRequest.Timeout`, expects a separate deadline to fire anyway, and expects a `WebException(RequestCanceled)`. The previous implementation had multiple issues: - Body writer stuck on something other than the request stream would hang indefinitely - Race condition when response returned just as the timer fires, killing a response that's already returned - Orphaned GetResponseAsync that succeeds after the abort is not handled ## Implementation details Sits between #9145 (base) and #9144 in the stack. ## Test coverage The deadline behaviour stays covered by: - `ApiRequestTests`, added in #9144 - `FlagEvaluationTransportTests.SlowTcpAgentTimesOutWithoutReplayingBatch` - `ExposureApiTimeoutTests.LegacyExposureRequestsAbortStalledConnection` On their own, #9145 and this PR have no async deadline on .NET Framework, so `SlowTcpAgentTimesOutWithoutReplayingBatch` fails on net48 until #9144 is applied. With the full stack applied, all transport and Feature Flags timeout tests pass locally on net48 and net10.0. ## Other details Merge together with #9144. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
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 and bounded exposure lifecycle #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.
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]