Repository navigation
Conversation
Keep JSON values typed when constructing OpenFeature contexts, and explicitly flush before multi-target bursts so tests start with an empty aggregation window. Co-Authored-By: Codex <noreply@openai.com>
Contributor
|
|
vjfridge
added a commit
to DataDog/dd-trace-dotnet
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
Make the .NET weblog accurately exercise feature-flag privacy and burst aggregation for dd-trace-dotnet #9351. Converting non-string JSON context values to strings changes the input being tested; starting a burst just before a periodic flush can split its aggregation window.
Changes
Testing
b32fca42cc../format.sh --checkpassed.Workflow
Draft for review; no merge requested. .NET Agent EVP release gates still need the selected release versions.