Repository navigation
[Feature Flags] Add per-subject evaluation telemetry opt-out - #9426
leoromanovsky wants to merge 6 commits into
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9426) and master. ✅ No regressions detected |
BenchmarksBenchmark execution time: 2026-10-30 17:42:50 Comparing candidate commit 8fb04c5 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 8 known flaky benchmarks, 117 flaky benchmarks without significant changes.
|
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. |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Static review supports the per-evaluation isolation of opt-out decisions and preservation of flag results and exposures. Runtime verification was blocked by the missing .NET 11 SDK.
🤖 Bits Code Review · Commit 9bde62f
| /// <summary> Gets provider hooks for flag evaluation metrics tracking. </summary> | ||
| /// <returns> Returns the list of provider hooks. </returns> | ||
| public override IImmutableList<Hook> GetProviderHooks() => _providerHooks; | ||
| public override IImmutableList<Hook> GetProviderHooks() |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Heads up: #9351 adds flagevaluation EVP events through a new provider hook. Both PRs edit DatadogProvider.cs and DatadogProvider.Experimental.cs, so whichever lands second has two small follow-ups.
I merged #9351's head (e452d59) with this PR's head (d7a8fdd) and kept both sides of each conflict. This method then wraps the EVP hook as well: with the parameter on, the probe saw no EVP enqueue call for an opted-out subject, async or sync. This PR's tests also catch a resolution that drops its changes. What remains:
README.md:143says "The provider does not currently produce evaluation EVP events." That becomes false.- No test asserts EVP suppression. The Java PR (Add per-subject evaluation telemetry opt-out dd-trace-java#12884) lists EVP events among the suppressed telemetry.
Evidence — #9351 merged with this PR
The handwritten conflicts are in DatadogProvider.cs (the comment above _providerHooks) and DatadogProvider.Experimental.cs (ResolveSync). I kept #9351's _evpHook argument to Resolve and this PR's GetProviderHooks() call. The four generated ConfigurationKeys.FeatureFlags.g.cs files also conflict; building Datadog.Trace regenerated the net6.0 copy with both keys. GetProviderHooks() itself merges cleanly, because #9351 does not change it. The merged provider suite passes 834/834 on net8.0.
Probe with an injected FlagEvalEVPHook (test-only seam below):
| Path | Parameter | Opted out | Enqueue callback calls |
|---|---|---|---|
| Async | off | yes | 1 |
| Async | on | no | 1 |
| Async | on | yes | 0 |
| Sync | on | no | 1 |
| Sync | on | yes | 0 |
Resolutions that drop this PR's changes:
- Restoring
GetProviderHooks() => _providerHooksturns 27 of this PR's tests red. - Taking [Feature Flags] Add privacy-protected flagevaluation EVP events through the Agent #9351's side of
ResolveSync(_providerHooks) turns 1 red:SynchronousEvaluationUsesItsSuppliedContext.
Possible EVP test — needs an injection seam for the EVP hook
Seam in the internal constructor of the merged provider:
internal DatadogProvider(DatadogProviderOptions options, FeatureFlagsSdk.EvaluationCallback? evaluate, Action<long?, bool, string?, bool, string, object?>? accumulateSpanEnrichment, FlagEvalEVPHook? evpHook = null)
{
// ...
if (evpHook is not null)
{
_evpHook = evpHook;
}
else if (FeatureFlagsSdk.IsEvaluationEventsEnabled())
{
_evpHook = new FlagEvalEVPHook();
}
// ...
}Test, as I ran it in the merged tree:
// <copyright file="ProbeEvpCompositionTests.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>
#nullable enable
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Datadog.Trace.FeatureFlags;
using OpenFeature;
using OpenFeature.Model;
using Xunit;
using Xunit.Abstractions;
namespace Datadog.FeatureFlags.OpenFeature.Tests;
// REVIEW PROBE (local only): #9351's EVP hook inside #9426's opt-out wrapper.
[Collection(nameof(EvaluationTelemetryOptOutCollection))]
public class ProbeEvpCompositionTests(ITestOutputHelper output) : IAsyncLifetime
{
public Task InitializeAsync() => Task.CompletedTask;
public Task DisposeAsync() => Api.Instance.ShutdownAsync();
[Theory]
[InlineData(false, true, 1)]
[InlineData(true, false, 1)]
[InlineData(true, true, 0)]
public async Task AsyncEvpEventsFollowTheOptOut(bool enabled, bool optedOut, int expected)
{
var enqueues = 0;
using var provider = CreateProvider(enabled, () => Interlocked.Increment(ref enqueues));
var domain = Guid.NewGuid().ToString("N");
await Api.Instance.SetProviderAsync(domain, provider);
var result = await Api.Instance.GetClient(domain).GetBooleanDetailsAsync("flag", false, Context(optedOut));
output.WriteLine($"async enabled={enabled} optedOut={optedOut} value={result.Value} evp_enqueues={enqueues}");
Assert.True(result.Value);
Assert.Equal(expected, enqueues);
}
[Theory]
[InlineData(false, 1)]
[InlineData(true, 0)]
public void SyncEvpEventsFollowTheOptOut(bool optedOut, int expected)
{
var enqueues = 0;
using var provider = CreateProvider(enabled: true, () => Interlocked.Increment(ref enqueues));
#pragma warning disable DDFF001
var result = provider.ResolveBooleanValue("flag", false, Context(optedOut));
#pragma warning restore DDFF001
output.WriteLine($"sync optedOut={optedOut} value={result.Value} evp_enqueues={enqueues}");
Assert.True(result.Value);
Assert.Equal(expected, enqueues);
}
private static DatadogProvider CreateProvider(bool enabled, Action onEnqueue)
=> new(
new DatadogProviderOptions { EnableEvaluationTelemetryOptOutParameter = enabled },
(key, _, _, _, _) => new StubEvaluation(key),
accumulateSpanEnrichment: null,
evpHook: new FlagEvalEVPHook(() => true, (_, _, _, _, _, _, _, _) => onEnqueue(), () => { }, FlagEvaluationContextSnapshot.Capture));
private static EvaluationContext Context(bool optedOut)
=> EvaluationContext.Builder().SetTargetingKey("subject").Set("dd_evaluation_telemetry_opt_out", optedOut).Build();
private sealed class StubEvaluation(string flagKey) : IEvaluation
{
public string FlagKey => flagKey;
public object? Value => true;
public EvaluationReason Reason => EvaluationReason.Static;
public string? Variant => "on";
public string? Error => null;
public IDictionary<string, string>? FlagMetadata { get; } = new Dictionary<string, string> { ["__dd_allocation_key"] = "allocation" };
}
}There was a problem hiding this comment.
Leaving this open as the merge-time follow-up for #9351, which is still unmerged. This branch has no EVP evaluation-event producer. When the branches combine, the remaining work is to preserve the opt-out wrappers on both paths, add EVP enqueue-count tests, and update the README. The current shared runs explicitly exclude EVP evaluation events; they verify metrics, span annotations, unchanged exposures, targeting, and context retention.
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Thanks! With #9351 merged, this PR lands second, so the follow-ups from my heads-up now apply here.
I merged master at 5bd282b into this head to check. 16 files conflict, 6 of them hand-written. After keeping both sides, all 835 existing provider tests pass on net8.0, and opted-out evaluations sent no EVP events in my probe. Three items remain for the rebase:
- README line 146 says "The provider does not currently produce evaluation EVP events." That becomes false.
- No test asserts EVP suppression yet.
- Resolution-time capture still runs for opted-out subjects. vjfridge raised this on
EvaluationTelemetryOptOutHook.cs:41; I replied there with a tested change and a test that covers item 2.
Conflicts — the conflicts in my merge
Hand-written: Datadog.FeatureFlags.OpenFeature.csproj (master already has the DiagnosticSource reference), DatadogProvider.cs (a comment), DatadogProvider.Experimental.cs (ResolveSync), Sdk/FeatureFlagsSdk.cs, FeatureFlags/FeatureFlagsSettings.cs, and FeatureFlagsSettingsTests.cs.
Generated: the four ConfigurationKeys.FeatureFlags.g.cs and four InstrumentationDefinitions.g.cs files, supported_calltargets.g.json, and generated_calltargets.g.cpp.
In ResolveSync I kept master's _evpHook argument to Resolve and this PR's GetProviderHooks() call.
| { | ||
| internal static CallTargetReturn<bool> OnMethodEnd<TTarget>(bool returnValue, Exception? exception, in CallTargetState state) | ||
| { | ||
| var enabled = Datadog.Trace.Tracer.Instance.Settings.FeatureFlags.EvaluationTelemetryPerSubjectOptOutEnabled; |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Optional: add a unit test for this OnMethodEnd, so a regression here fails without the native profiler.
The new FeatureFlagsSettingsTests cover the configuration side well. No unit test calls this method, though. If it returned returnValue (the provider's environment fallback) instead of the tracer setting, all 47 FeatureFlagsSettingsTests would still pass. Without such a test, only an instrumented run where the tracer setting differs from the environment fallback would expose it.
Evidence — a broken integration passes all 47 FeatureFlagsSettingsTests
At 773c321 on net8.0, Datadog.Trace.Tests filtered to FeatureFlagsSettingsTests plus the test below:
| Integration | 47 FeatureFlagsSettingsTests |
Test below (2 rows) |
|---|---|---|
| As written | pass | pass |
Returns returnValue |
pass | 2 red |
Suggested test — tracer/test/Datadog.Trace.Tests/FeatureFlags/OpenFeatureSdkIsEvaluationTelemetryOptOutEnabledIntegrationTests.cs
// <copyright file="OpenFeatureSdkIsEvaluationTelemetryOptOutEnabledIntegrationTests.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>
#nullable enable
using Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.OpenFeature;
using Datadog.Trace.Configuration;
using Datadog.Trace.TestHelpers;
using FluentAssertions;
using Xunit;
namespace Datadog.Trace.Tests.FeatureFlags;
[Collection(nameof(TracerInstanceTestCollection))]
[TracerRestorer]
public class OpenFeatureSdkIsEvaluationTelemetryOptOutEnabledIntegrationTests
{
[Theory]
[InlineData(true)]
[InlineData(false)]
public void ReplacesTheProviderFallbackWithTheTracerSetting(bool configured)
{
Tracer.Configure(TracerSettings.Create(new()
{
{ ConfigurationKeys.FeatureFlags.EvaluationTelemetryPerSubjectOptOutEnabled, configured },
{ ConfigurationKeys.StartupDiagnosticLogEnabled, false },
{ ConfigurationKeys.TracerMetricsEnabled, false },
}));
// The provider's uninstrumented result is the opposite, so only the tracer setting can pass.
OpenFeatureSdkIsEvaluationTelemetryOptOutEnabledIntegration
.OnMethodEnd<object>(!configured, exception: null, state: default)
.GetReturnValue()
.Should()
.Be(configured);
}
}
|
|
||
| Before OpenFeature 2.5.0, the provider cannot see a replacement context when a later Before hook throws. | ||
| Setting opt-out in the supplied context covers this aborted evaluation path on those versions. | ||
| The suite checks both this limitation and the corrected behavior in newer OpenFeature versions. |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Optional: say which OpenFeature version CI runs this test with. As written, a reader may assume CI covers both behaviors.
The test adapts to the installed OpenFeature version, and that works: it passes on OpenFeature 2.3.0, 2.4.0, 2.5.0, and 2.14.1, and on 2.14.1 it still catches an injected bug in the Finally fallback. But the test project gets OpenFeature 2.3.0 from the provider's reference and adds none of its own. So CI runs only the pre-2.5.0 branch.
Evidence — runs at 773c321 on net8.0
Datadog.FeatureFlags.OpenFeature.csproj:40references OpenFeature 2.3.0.Datadog.FeatureFlags.OpenFeature.Tests.csprojhas no OpenFeature reference.- With the test project forced to each version, the provider suite passes 750/750 on 2.3.0, 2.4.0, 2.5.0, and 2.14.1.
- Mutant
(_optedOut ?? false)inEvaluationTelemetryOptOutHook.FinallyAsync: on 2.14.1,BeforeFailureUsesTheContextExposedByOpenFeaturegoes red (4 red in total). On 2.3.0 that test stays green (3 red, all from other tests).
Suggested text — in place of line 65
"The error-path test adapts to the installed OpenFeature version. CI runs it with OpenFeature 2.3.0, so the newer behavior is checked only when the test project resolves 2.5.0 or later."
andrewlock
left a comment
andrewlock
left a comment
There was a problem hiding this comment.
Please follow the PR template.
Mostly just looked at the apm-owned files, and I don't think they should be changed. Also noticed problematic changes in the csproj file, but didn't review the FF SDK code much.
I'd strongly recommend rewriting the README by hand, as it's written for customers so should clearly explain the feature, not simply be a list list of caveats and design decisions.
| An unset option uses the tracer's normal configuration sources, including environment variables, | ||
| `datadog.json`, application configuration, and stable configuration. |
There was a problem hiding this comment.
nit: I would probably remove the explicit listing of configuration sources.
| An unset option uses the tracer's normal configuration sources, including environment variables, | |
| `datadog.json`, application configuration, and stable configuration. | |
| An unset option uses the tracer's normal configuration sources. |
| An explicit `true` or `false` option overrides tracer configuration. | ||
| An unset option uses the tracer's normal configuration sources, including environment variables, | ||
| `datadog.json`, application configuration, and stable configuration. | ||
| Without instrumentation, the provider reads the process environment only. |
There was a problem hiding this comment.
Without instrumentation, none of this works anyway?
| Without instrumentation, the provider reads the process environment only. |
| An unset option uses the tracer's normal configuration sources, including environment variables, | ||
| `datadog.json`, application configuration, and stable configuration. | ||
| Without instrumentation, the provider reads the process environment only. | ||
| Missing or invalid values disable the parameter. |
There was a problem hiding this comment.
This is just how configuration works in general, I don't think it should be listed here, especially as the flag is a boolean in code, so they can't get it wrong
| Missing or invalid values disable the parameter. |
| `datadog.json`, application configuration, and stable configuration. | ||
| Without instrumentation, the provider reads the process environment only. | ||
| Missing or invalid values disable the parameter. | ||
| The provider copies the resolved setting at construction. Later configuration or option changes do not affect it. |
There was a problem hiding this comment.
What happens if they call await OpenFeature.Api.Instance.SetProviderAsync(provider); again? Is that something we have tested or support in general? Should we?
| Only boolean `true` suppresses new Datadog evaluation metrics and automatic flag | ||
| span annotations. Missing values, strings, numbers, null, and boolean `false` | ||
| preserve telemetry. The provider does not currently produce evaluation EVP events. |
There was a problem hiding this comment.
This doesn't seem like it should be customer-facing documentation, I find it more confusing than anything?
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Agreed, and moving these out doesn't conflict with my earlier asks. The EVP sentence on line 146 was on my #9351 follow-up list, so removing it settles that item. The error-path paragraph at lines 156–162 already has the OpenFeature version range I asked for. If it leaves the README, docs/development/FeatureFlagsOpenFeature.md still describes the error path.
| <ItemGroup Condition="'$(TargetFramework)' == 'net8.0'"> | ||
| <!-- FlagEvalMetrics needs DiagnosticSource 9; newer OpenFeature versions do not bring it transitively. --> | ||
| <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="9.0.0" /> | ||
| </ItemGroup> | ||
|
|
There was a problem hiding this comment.
This is almost certainly not the correct approach 🤔 .NET 8 includes System.Diagnostics.DiagnosticSource v8.0.0, which is the same version that OpenFeature references by default. I don't see any reason that you should be forcing 9.0.0 here? If you're using features only introuduced in 9.0.0 then you would break other TFMs too 🤷♂️
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
I suggested this reference, so here is the reasoning. #9351 has since added the same reference to master. When this branch rebases, the conflict here can simply take master's block.
The net8.0 build binds DiagnosticSource 9.0.0.0 with or without this line. That comes from the provider's minimum OpenFeature version, not from 9.0-only APIs: OpenFeature 2.3.0 brings Microsoft.Extensions.Logging.Abstractions 9.0.0 on net8.0, which requires DiagnosticSource 9.0.0. You're right that newer OpenFeature only needs 8.0.0 (I checked 2.4.0, 2.5.0 and 2.14.1). So a .NET 8 app that resolves one of those has no 9.0.0.0 assembly unless this package or another dependency brings 9.0 or later, and then new DatadogProvider() throws. The net462 and netstandard2.0 builds don't reference DiagnosticSource at all, and .NET 9 ships 9.0.0.0.
Evidence — assembly references, restore, and a .NET 8 repro
Built at 773c321 and read with System.Reflection.Metadata:
| Build | DiagnosticSource reference | System.Diagnostics.Metrics type refs |
|---|---|---|
| net462 | none | 0 |
| netstandard2.0 | none | 0 |
| net8.0 | 9.0.0.0 | 2 |
| net9.0 | 9.0.0.0 | 2 |
net8.0, this ItemGroup removed |
9.0.0.0 | 2 |
| published 2.3.2, net8.0 | 9.0.0.0 | 2 |
- OpenFeature 2.3.0's net8.0 dependency group lists
Microsoft.Extensions.Logging.Abstractions9.0.0. That package's net8.0 group listsSystem.Diagnostics.DiagnosticSource9.0.0. - Pinning
Version="8.0.0"fails restore:NU1605: Detected package downgrade: System.Diagnostics.DiagnosticSource from 9.0.0 to 8.0.0, viaOpenFeature 2.3.0 -> Microsoft.Extensions.Logging.Abstractions 9.0.0 -> System.Diagnostics.DiagnosticSource (>= 9.0.0). - A .NET 8.0.32 console app that only calls
new DatadogProvider(), with OpenFeature 2.14.1: the published 2.3.2 throwsFileNotFoundExceptionfor System.Diagnostics.DiagnosticSource 9.0.0.0, and the 2.3.3 package packed from this head constructs. A net9.0 app with the published 2.3.2 works, because .NET 9 ships 9.0.0.0. - master has the same net8.0
ItemGroupfrom [Feature Flags] Add privacy-protected flagevaluation EVP events through the Agent #9351 (29487ea), plus 9.0.0 pins forSystem.Collections.ImmutableandSystem.Threading.Channelson targets older than net8.0.
There was a problem hiding this comment.
That PR is also wrong, and it should almost certainly be reverted. You're setting yourself up for a world of pain
EDIT: Urgh, but it looks like OpenFeature themselves have screwed this up. My recommendation would be to bump the minimum version to 2.4.0, then this whole problem goes away, and you can remove this (which will cause problems for all downstream consumers of our package)
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Agreed. I built and tested your suggestion, and the binding mismatch goes away without any explicit System.* references. With OpenFeature 2.4.0 as the minimum, the net462, netstandard2.0 and net8.0 builds reference only the 8.0.0.0 versions of System.Collections.Immutable, System.Threading.Channels and (on net8.0) System.Diagnostics.DiagnosticSource. The net9.0 build references the 9.0 versions that ship with .NET 9. You're also right about downstream consumers: with master's 9.0 references, a .NET 8 app that pins DiagnosticSource 8.0.1 fails restore with NU1605, and so does a .NET 6 app that pins Immutable 8.0.0.
The bump has these follow-ups:
- Apps that pin OpenFeature 2.3.x must move to 2.4.0 or later.
Samples.OpenFeaturemust stop restoring 2.3.x. Its defaultApiVersionis 2.3.0, and the generated test matrices include 2.3.0 and 2.3.2.- The DataDog/system-tests .NET weblog pins OpenFeature 2.3.0.
- Following the csproj's versioning guidance, the package version would become 2.4.x.
Drop #9351's pins only together with the bump. Without them, a .NET 6 app that uses OpenFeature 2.14.1 fails to compile against this PR's netstandard2.0 build.
Evidence — assembly references per build
I built master's approach and your suggestion on this PR merged with current master (82d51b1, which has #9351's pins), and this PR's head at 773c321. Release builds, read with System.Reflection.Metadata:
| Build | Master's approach: OpenFeature 2.3.0 + 9.0 pins | This PR's head: OpenFeature 2.3.0, net8.0 pin only | Your suggestion: OpenFeature 2.4.0, no pins |
|---|---|---|---|
| net462 | Immutable 9.0.0.0, Channels 9.0.0.0 | Immutable 9.0.0.0, Channels 9.0.0.0 | Immutable 8.0.0.0, Channels 8.0.0.0 |
| netstandard2.0 | Immutable 9.0.0.0, Channels 9.0.0.0 | Immutable 9.0.0.0, Channels 9.0.0.0 | Immutable 8.0.0.0, Channels 8.0.0.0 |
| net8.0 | DiagnosticSource 9.0.0.0; Immutable and Channels 8.0.0.0 | same as master | DiagnosticSource, Immutable and Channels 8.0.0.0 |
| net9.0 | all 9.0.0.0 (shipped by .NET 9) | same | same |
With your suggestion, the packed nuspec lists only OpenFeature (>= 2.4.0) in each dependency group.
Consumer apps — construct the provider and run one sync evaluation
Each app references a locally packed provider and runs new DatadogProvider(), GetProviderHooks() and one ResolveBooleanValue. "OK" means it restored, built and ran with no exception. The evaluation returns ProviderNotReady because no configuration is loaded. Runtimes: 6.0.36, 8.0.32 and 9.0.21.
| App | Master's approach | Your suggestion |
|---|---|---|
| .NET 8, OpenFeature 2.14.1 | OK | OK |
| .NET 8, OpenFeature 2.3.0 | OK | NU1605: the provider needs OpenFeature (>= 2.4.0) |
.NET 8, OpenFeature 2.14.1 + System.Diagnostics.DiagnosticSource 8.0.1 |
NU1605: the provider needs DiagnosticSource >= 9.0.0 |
OK |
| .NET 6 (netstandard2.0 build), OpenFeature 2.14.1 | OK | OK |
.NET 6, OpenFeature 2.14.1 + System.Collections.Immutable 8.0.0 |
NU1605: the provider needs Immutable >= 9.0.0 |
OK |
| .NET 9, OpenFeature 2.14.1 | not run | OK |
This PR's head package in the .NET 6 app with OpenFeature 2.14.1 fails to compile with CS1705: Datadog.FeatureFlags.OpenFeature uses System.Collections.Immutable, Version=9.0.0.0, which is higher than the referenced 8.0.0.0.
Tests and knock-on checks
- Provider suite on net8.0 with your suggestion: 835 of 835 pass with OpenFeature 2.4.0, 2.5.0 and 2.14.1.
- OpenFeature 2.4.0 ships the same four target frameworks, so
TargetFrameworksdoes not change. Samples.OpenFeaturerestore with your suggestion:ApiVersion=2.3.0fails withNU1605: Detected package downgrade: OpenFeature from 2.4.0 to 2.3.0; 2.4.0 and 2.10.0 restore.PackageVersionsGeneratorDefinitions.jsonlistsMinVersion2.3.0 andSpecificVersions2.3.0 and 2.10.0, andPackageVersionsLatestMinors.g.propsincludes 2.3.2.- The system-tests weblog (
utils/build/docker/dotnet/weblog/app.csprojat f2cbbc9015) pinsOpenFeature2.3.0 next toDatadog.FeatureFlags.OpenFeature Version="*". I did not build it. A console app that references OpenFeature 2.3.0 and the locally packed provider fails restore with the sameNU1605.
Suggested change — what I ran, plus the follow-ups
In Datadog.FeatureFlags.OpenFeature.csproj, raise the reference and delete both conditional ItemGroups (the Immutable/Channels one and the DiagnosticSource one):
<ItemGroup>
<!-- If you update this version, be sure to update the TargetFrameworks element above-->
<PackageReference Include="OpenFeature" Version="2.4.0" />
</ItemGroup>Not run:
- In
tracer/build/PackageVersionsGeneratorDefinitions.json, set the OpenFeature entry to"MinVersion": "2.4.0"and"SpecificVersions": ["2.4.0", "2.10.0"]. - In
Samples.OpenFeature.csproj, change the defaultApiVersionto 2.4.0. - Run the
GeneratePackageVersionstarget. It rewrites thePackageVersions*.g.propsmatrices, the integration tests'PackageVersions*.g.csfiles andsupported_versions.json, whoseminVersionTestedInclusiveis 2.3.0 today. - Set
<Version>to a 2.4.x number. - Update the system-tests weblog pin before a 2.4.0-minimum package ships.
|
|
||
| return null; | ||
| } | ||
| public static bool? ToBoolean(this string value) => BooleanConfigurationParser.Parse(value); |
There was a problem hiding this comment.
What was the reason for this change? This seems entirely unrelated to the intent of this PR, so should be extracted to a separate PR, explaining why it's needed etc
| { | ||
| try | ||
| { | ||
| var value = Environment.GetEnvironmentVariable(EvaluationTelemetryOptOutEnabled); |
There was a problem hiding this comment.
This seems wrong - AFAICT you're trying to "handle" the case where instrumentation is failed, but 1. You shouldn't do that - if instrumentation has failed, everything is broken anyway 2. This adds complexity and confusion 3. It only partially works anyway
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
This fallback was my suggestion, so here is the case for it, and a tested way to remove it if you prefer.
The case I had in mind is not failed instrumentation. It is a newer provider package running with an older tracer home. v3.55.2 has CallTarget entries for the seven FeatureFlagsSdk methods it already instrumented, but none for IsEvaluationTelemetryOptOutEnabled. Reading the generated CallTarget table, the existing evaluation entry points stay instrumented in that pairing. When no code option is set, the stub body alone decides this setting. Without the fallback, the env var is silently ignored there; the code option still works. You're right that it only partly works: it never reads datadog.json or stable config.
If you'd rather match IsSpanEnrichmentEnabled and return false, the removal is small; I built and ran it. It also answers your StringExtensions question: that change exists only so this fallback can share the tracer's boolean parser, and the file reverts to the merge base.
Evidence — what v3.55.2 instruments
tracer/src/Datadog.Tracer.Native/Generated/generated_calltargets.g.cpp at v3.55.2 has FeatureFlagsSdk entries for AccumulateSpanEnrichment, Evaluate, HasConfiguration, InitializeAsync, IsAvailable, IsSpanEnrichmentEnabled and RegisterOnNewConfigEventHandler. It has none for IsEvaluationTelemetryOptOutEnabled, so the stub body runs. The StringExtensions change came in 9bde62f, the commit that added this fallback.
Removal I ran — a 6-file patch, plus the test cleanup it implies
Sdk/FeatureFlagsSdk.cs:
/// <returns> True when the tracer setting is on </returns>
[MethodImpl(MethodImplOptions.NoInlining)]
internal static bool IsEvaluationTelemetryOptOutEnabled() => false;Then:
- delete
ProviderEnvironmentConfiguration.cs,tracer/src/Datadog.Trace/Configuration/BooleanConfigurationParser.cs, and its<Compile Include=... Link="Sdk\BooleanConfigurationParser.cs" />line; - delete
ProviderEnvironmentConfigurationTests.cs; - restore
StringExtensions.cswithgit checkout c31cfdff70 -- tracer/src/Datadog.Trace/ExtensionMethods/StringExtensions.cs; - drop README line 140, which also settles your comment there, and the fallback sentence on line 60 of
docs/development/FeatureFlagsOpenFeature.md; - delete or rewrite the 7 env-var tests in
EvaluationTelemetryOptOutTests.cslisted below. My run left them in place, which is why they fail; - switch
EnvironmentActivationAloneDoesNotSuppressTelemetryto the code option and rename it, for exampleActivationAloneDoesNotSuppressTelemetry. It still passes after the removal, but only because activation is then off.
The patch I ran covered the stub change and the first three bullets: 6 files, +44/−156. Results at 773c321 on net8.0:
| Suite | Result |
|---|---|
| Provider tests | 716 of 723 pass. The 7 failures are the env-var tests that depend on the fallback: EnvironmentActivationPreservesResultAndExposureWithoutEvaluationTelemetry, EnvironmentIsCopiedAtConstruction (2 rows), ParameterlessProviderReadsEnvironmentConfiguration, and the 3 env-only rows of EnvironmentConfigurationRespectsExplicitProviderOptions |
Datadog.Trace.Tests: StringExtensions, StringConfigurationSource, FeatureFlagsSettingsTests |
104 of 104 pass |
|
|
||
| public class FeatureFlagsSettingsTests | ||
| { | ||
| [Theory] |
There was a problem hiding this comment.
I don't think these tests add any value - they're just testing the config system, which is already extensively tested.
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
One data point, since my review suggested tests here: these rows do pin this setting's key. If FeatureFlagsSettings reads the wrong key (I tried FeatureFlagsEnabled), all 8 new rows fail, and the other 39 tests in the class still pass. If you want fewer, a single true/false case keeps that check. As you say, the alias, invalid-value and JSON rows mostly re-test the config system.
| // These tests change the OpenFeature API context and transaction propagator. | ||
| [CollectionDefinition(nameof(EvaluationTelemetryOptOutCollection), DisableParallelization = true)] | ||
| public class EvaluationTelemetryOptOutCollection | ||
| { | ||
| } |
There was a problem hiding this comment.
Can't you write the tests without exercising global state?
| { | ||
| // Sync calls skip Before. An application Before failure can also prevent this wrapper's | ||
| // Before from running. Both paths use the immutable context supplied to Finally. | ||
| return (_optedOut ?? IsOptedOut(context.EvaluationContext)) |
There was a problem hiding this comment.
Non-blocking integration follow-up: gate evaluation capture as well as Finally when bringing in #9351.
Could we carry the subject opt-out decision into the producer's resolution-time capture when these changes are integrated? The global DD_FLAGGING_EVALUATION_COUNTS_ENABLED=false path omits that producer. Wrapping only its Finally hook would suppress delivery after some evaluation-only work has already happened.
Source trace and the validation to add on the integrated branch
At this PR's 773c321 head there is no EVP evaluation producer, so this is a follow-up for integration, not an executed failure on the current branch. In the now-merged #9351, FeatureFlagsSdk.Resolve calls CaptureEvaluation before Finally.
Capture creates an EvaluatedContext/weak-table entry even in protected mode; full-data mode additionally takes a bounded context snapshot. Suppressing Finally alone also skips its explicit table-entry removal. This is source-traced; I have not built or executed a combined branch.
When integrating, please guard resolution-time capture with the applicable opt-out policy and retain the Finally guard for SDK errors before resolution. Keep the ordinary targeting-context conversion and exposure path. A focused test should assert zero capture/snapshot calls and table insertions for opted-out evaluations in protected/full-data and sync/async modes, with an allowed-subject positive control. The .NET exclusion from the shared EVP system test should also be revisited once the producer is included.
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Confirmed by running it. I merged current master into this head. For an opted-out subject the wrapper stops the EVP event, but resolution still captures. When the EVP hook is enabled and admits the capture, each such evaluation leaves a weak-table entry that Finally would have removed, and full-data mode also takes a context snapshot.
Deciding opt-out before Resolve, and passing no EVP hook for opted-out subjects, removes that work in both modes and on both paths. The change also takes in your ResolveSync early return, using the wrapper's predicate.
Evidence — capture work per evaluation, parameter on
I merged master at 5bd282b (it contains #9351) into 773c321 and kept both sides of each conflict. Runs are on net8.0. A test-only constructor parameter injects a FlagEvalEVPHook with counting delegates. The leftover count reads the hook's weak table after the evaluation returns.
| Opted-out evaluation | Before: canEnqueue / snapshots / leftover entries / events |
After |
|---|---|---|
| async, protected | 1 / 0 / 1 / 0 | 0 / 0 / 0 / 0 |
| async, full-data | 1 / 1 / 1 / 0 | 0 / 0 / 0 / 0 |
| sync, protected | 1 / 0 / 1 / 0 | 0 / 0 / 0 / 0 |
| sync, full-data | 1 / 1 / 1 / 0 | 0 / 0 / 0 / 0 |
Allowed subjects are the same before and after: 2 canEnqueue calls (capture, then Finally), 1 snapshot in full-data mode, no leftover entry, and 1 event. The leftover entry has a weak key, so it goes away with the result; it is still work and retained data that the opt-out should avoid.
All 835 existing tests in the merged provider suite pass before and after the change. The test below fails its 4 opted-out rows before the change and passes after it. In the existing harness, which has no EVP hook, the early return cuts allocation for opted-out sync calls from 3,279 to 2,847 bytes per evaluation; allowed calls stay at 3,328.
Suggested change — for the rebased branch
EvaluationTelemetryOptOutHook.cs: share the predicate.
internal static bool IsOptedOut(EvaluationContext? context)DatadogProvider.cs: add the helper, and pass it in all five Resolve*Async methods, for example:
// Opted-out evaluations skip evaluation-event capture as well as the telemetry Finally hooks.
private FlagEvalEVPHook? GetEvaluationEventsHook(EvaluationContext? context)
=> _evaluationTelemetryOptOutParameterEnabled && EvaluationTelemetryOptOutHook.IsOptedOut(context) ? null : _evpHook; var res = FeatureFlagsSdk.Resolve<bool>(flagKey, Trace.FeatureFlags.ValueType.Boolean, defaultValue, context, _evaluate, GetEvaluationEventsHook(context));DatadogProvider.Experimental.cs, in ResolveSync:
var optedOut = _evaluationTelemetryOptOutParameterEnabled && EvaluationTelemetryOptOutHook.IsOptedOut(context);
var resolution = FeatureFlagsSdk.Resolve(flagKey, valueType, defaultValue, context, _evaluate, optedOut ? null : _evpHook);
if (optedOut)
{
// Every built-in provider hook is a Finally-only telemetry hook.
return resolution;
}
var hooks = GetProviderHooks();The wrapper's Finally guard stays for SDK errors before resolution. The README's existing pre-2.5.0 error-path limitation is unchanged.
Suggested test — needs a test seam for the EVP hook
Seam in the internal constructor:
internal DatadogProvider(DatadogProviderOptions options, FeatureFlagsSdk.EvaluationCallback? evaluate, Action<long?, bool, string?, bool, string, object?>? accumulateSpanEnrichment, FlagEvalEVPHook? evpHook = null) if (evpHook is not null)
{
_evpHook = evpHook;
}
else if (FeatureFlagsSdk.IsEvaluationEventsEnabled())
{
_evpHook = new FlagEvalEVPHook();
}New tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/EvaluationEventsOptOutTests.cs:
// <copyright file="EvaluationEventsOptOutTests.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>
#nullable enable
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Datadog.Trace.FeatureFlags;
using OpenFeature;
using OpenFeature.Model;
using Xunit;
namespace Datadog.FeatureFlags.OpenFeature.Tests;
[Collection(nameof(EvaluationTelemetryOptOutCollection))]
public class EvaluationEventsOptOutTests : IAsyncLifetime
{
public Task InitializeAsync() => Task.CompletedTask;
public Task DisposeAsync() => Api.Instance.ShutdownAsync();
[Theory]
[InlineData(false, false)]
[InlineData(false, true)]
[InlineData(true, false)]
[InlineData(true, true)]
public async Task OptedOutAsyncEvaluationsSkipEvaluationEventCapture(bool fullData, bool optedOut)
{
var events = new EventsProbe();
using var provider = events.CreateProvider(fullData);
var domain = Guid.NewGuid().ToString("N");
await Api.Instance.SetProviderAsync(domain, provider);
var result = await Api.Instance.GetClient(domain).GetBooleanDetailsAsync("flag", false, Context(optedOut));
Assert.True(result.Value);
events.AssertCaptured(fullData, optedOut);
}
[Theory]
[InlineData(false, false)]
[InlineData(false, true)]
[InlineData(true, false)]
[InlineData(true, true)]
public void OptedOutSyncEvaluationsSkipEvaluationEventCapture(bool fullData, bool optedOut)
{
var events = new EventsProbe();
using var provider = events.CreateProvider(fullData);
#pragma warning disable DDFF001 // Verify the experimental synchronous API.
var result = provider.ResolveBooleanValue("flag", false, Context(optedOut));
#pragma warning restore DDFF001
Assert.True(result.Value);
events.AssertCaptured(fullData, optedOut);
}
private static EvaluationContext Context(bool optedOut)
=> EvaluationContext.Builder().SetTargetingKey("subject").Set("dd_evaluation_telemetry_opt_out", optedOut).Build();
private sealed class EventsProbe
{
private int _canEnqueueCalls;
private int _snapshots;
private int _enqueues;
public DatadogProvider CreateProvider(bool fullData)
=> new(
new DatadogProviderOptions { EnableEvaluationTelemetryOptOutParameter = true },
(key, _, _, _, _) => new StubEvaluation(key, fullData),
accumulateSpanEnrichment: null,
evpHook: new FlagEvalEVPHook(
() =>
{
_canEnqueueCalls++;
return true;
},
(_, _, _, _, _, _, _, _) => _enqueues++,
() => { },
(EvaluationContext? context, out int omissionReasons) =>
{
_snapshots++;
return FlagEvaluationContextSnapshot.Capture(context, out omissionReasons);
}));
public void AssertCaptured(bool fullData, bool optedOut)
{
// Allowed: resolution captures, then Finally sends one event. Opted out: neither runs.
Assert.Equal(optedOut ? 0 : 2, _canEnqueueCalls);
Assert.Equal(!optedOut && fullData ? 1 : 0, _snapshots);
Assert.Equal(optedOut ? 0 : 1, _enqueues);
}
}
private sealed class StubEvaluation(string flagKey, bool fullData) : IEvaluation
{
public string FlagKey => flagKey;
public object? Value => true;
public EvaluationReason Reason => EvaluationReason.Static;
public string? Variant => "on";
public string? Error => null;
public IDictionary<string, string>? FlagMetadata { get; } = fullData
? new Dictionary<string, string> { [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = "true" }
: new Dictionary<string, string>();
}
}| cancellationToken.ThrowIfCancellationRequested(); | ||
| var resolution = FeatureFlagsSdk.Resolve(flagKey, valueType, defaultValue, context, _evaluate); | ||
| return _providerHooks.Count == 0 | ||
| var hooks = GetProviderHooks(); |
There was a problem hiding this comment.
Non-blocking optimization: could we check opt-out immediately after resolution, before GetProviderHooks()? When telemetry hooks are installed, opted-out synchronous calls still prepare wrappers and hook inputs that those hooks will not consume. Returning here would avoid that work while leaving evaluation in place.
Why this helps, and executed validation
With the capability enabled and boolean dd_evaluation_telemetry_opt_out: true, this path still constructs a HookContext, a FlagEvaluationDetails, and a FeatureProviderException for error results, plus the wrapper list. The wrapper eventually skips Finally; the current metrics and optional span-enrichment hooks have no work in After or Error.
I compiled the actual experimental dispatcher, selector and telemetry hooks with a controlled resolver. The original and early-return variants each passed 94 behavior cases with OpenFeature 2.3.0 and 2.5.0 on net9.0, covering all five sync entry points, typed/capability controls, optional span enrichment, errors and cancellation. The early return reduced those hook-input constructions and selector calls to zero for opted-out evaluations, preserving the resolver call and returned resolution object. Native targeting/exposure execution and the package's other target frameworks were not exercised.
Keep this check after FeatureFlagsSdk.Resolve, and keep the existing wrapper guard for the normal client/async path. This shortcut relies on the current private hooks all being wrapped, Finally-only telemetry hooks; older targets have only the optional span hook. Sharing the strict opt-out predicate with the wrapper and pinning that hook invariant in a test would keep this assumption explicit.
The tested change at this line is:
| var hooks = GetProviderHooks(); | |
| // All built-in provider hooks are Finally-only evaluation telemetry hooks. | |
| if (_evaluationTelemetryOptOutParameterEnabled | |
| && context is not null | |
| && context.TryGetValue("dd_evaluation_telemetry_opt_out", out var optOut) | |
| && optOut is { IsBoolean: true, AsBoolean: true }) | |
| { | |
| return resolution; | |
| } | |
| var hooks = GetProviderHooks(); |
| /// <summary> Gets provider hooks for flag evaluation metrics tracking. </summary> | ||
| /// <returns> Returns the list of provider hooks. </returns> | ||
| public override IImmutableList<Hook> GetProviderHooks() => _providerHooks; | ||
| public override IImmutableList<Hook> GetProviderHooks() |
| return _evaluationTelemetryOptOutParameterEnabled && _providerHooks.Count != 0 | ||
| ? _providerHooks.Select(hook => (Hook)new EvaluationTelemetryOptOutHook(hook)).ToImmutableList() | ||
| : _providerHooks; |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P3] Optional: build the wrapped list with an exact-size ImmutableArray builder. With two hooks, this roughly halves the allocation inside GetProviderHooks() when the parameter is on.
When the parameter is on and the provider has hooks, the LINQ Select plus ToImmutableList() allocates a LINQ iterator and new immutable-list nodes on every evaluation. OpenFeature only enumerates this result: 2.3.0 calls Concat(...).ToList(), and 2.5.0 and 2.14.1 call Concat(...).ToImmutableList(). The disabled path does not change.
Measurement — bytes per GetProviderHooks() call, two hooks
Release build, GC.GetAllocatedBytesForCurrentThread() over 100,000 calls after warm-up, on net8.0 and net9.0:
| Parameter | Current | Suggested |
|---|---|---|
| Off | 0 B | 0 B |
| On | 312 B | 160 B |
The provider builds without warnings for net462, netstandard2.0, net8.0 and net9.0. The full provider suite passes 752/752 on net8.0 and net9.0.
Suggested change
public override IImmutableList<Hook> GetProviderHooks()
{
// OpenFeature obtains this list once for each evaluation. Fresh wrappers isolate Before
// decisions across nested/concurrent calls; disabled providers keep their cached hooks.
var hooks = _providerHooks;
if (!_evaluationTelemetryOptOutParameterEnabled || hooks.Count == 0)
{
return hooks;
}
var wrapped = ImmutableArray.CreateBuilder<Hook>(hooks.Count);
for (var i = 0; i < hooks.Count; i++)
{
wrapped.Add(new EvaluationTelemetryOptOutHook(hooks[i]));
}
return wrapped.MoveToImmutable();
}Also remove using System.Linq; from this file. Nothing else in it uses LINQ.
| } | ||
|
|
||
| // Tests connect the real evaluator and span accumulator across the native profiler boundary. | ||
| internal DatadogProvider(DatadogProviderOptions options, FeatureFlagsSdk.EvaluationCallback? evaluate, Action<long?, bool, string?, bool, string, object?>? accumulateSpanEnrichment) |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P3] Optional: give this callback a named delegate, like FeatureFlagsSdk.EvaluationCallback. A six-argument Action<long?, bool, string?, bool, string, object?> hides which bool and which string is which.
The parameter names below match FeatureFlagsSdk.AccumulateSpanEnrichment. The method group still converts, so the CallTarget instrumentation of that method is unchanged. The test lambdas also convert without edits.
Suggested change — three files
Sdk/FeatureFlagsSdk.cs, next to EvaluationCallback:
internal delegate void SpanEnrichmentAccumulator(long? serialId, bool doLog, string? targetingKey, bool hasVariant, string flagKey, object? value);DatadogProvider.cs:
internal DatadogProvider(DatadogProviderOptions options, FeatureFlagsSdk.EvaluationCallback? evaluate, FeatureFlagsSdk.SpanEnrichmentAccumulator? accumulateSpanEnrichment)SpanEnrichmentHook.cs:
private readonly FeatureFlagsSdk.SpanEnrichmentAccumulator _accumulate;
internal SpanEnrichmentHook(FeatureFlagsSdk.SpanEnrichmentAccumulator accumulate)The provider builds without warnings for net462, netstandard2.0, net8.0 and net9.0. The full provider suite passes 752/752 on net8.0 and net9.0.
|
🤖 Comment from Claude working fully autonomously overnight: [P3] Please make the system-test row in the validation table reproducible. Add the system-tests revision, the scenario names and the test IDs that ran. The row says "environment and JSON modes" and "Four scenarios passed: 20 checks". The opt-out test on system-tests Your earlier reply reports JSON |
|
🤖 Comment from Claude working fully autonomously overnight: Reminder: After the first tracer release that contains this PR, update the system-tests .NET manifest. No code change is needed here.
|
| /// <summary>Gets a value indicating whether per-subject evaluation telemetry opt-out is enabled.</summary> | ||
| /// <returns>The tracer configuration, or the process environment when instrumentation is unavailable.</returns> | ||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| internal static bool IsEvaluationTelemetryOptOutEnabled() => ProviderEnvironmentConfiguration.IsEvaluationTelemetryOptOutEnabled(); |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P3] Please settle this method's name before 2.3.3 ships. CallTarget binds to it by name, so later tracers must keep an integration for this exact name.
The setting is EvaluationTelemetryPerSubjectOptOutEnabled in ConfigurationKeys and FeatureFlagsSettings. This stub (IsEvaluationTelemetryOptOutEnabled) and ProviderEnvironmentConfiguration.EvaluationTelemetryOptOutEnabled drop "PerSubject". A grep for PerSubjectOptOut misses both. After the package ships, a tracer that drops the old stub name loses the tracer-configuration path for 2.3.3. When the explicit option is unset, those applications then fall back to the environment variable only. So a later rename means keeping the old integration as well.
Suggested change — align the two provider names with the registry key
internal static bool IsEvaluationTelemetryPerSubjectOptOutEnabled() => ProviderEnvironmentConfiguration.IsEvaluationTelemetryPerSubjectOptOutEnabled();Then rename ProviderEnvironmentConfiguration.EvaluationTelemetryOptOutEnabled and its method to match. Update MethodName in OpenFeatureSdkIsEvaluationTelemetryOptOutEnabledIntegration, the constructor call in DatadogProvider.cs, and the tests, and regenerate the CallTarget definitions. Keep EnableEvaluationTelemetryOptOutParameter as it is. It matches EnableEvaluationTelemetryOptOutParameter in dd-trace-go#5611 and enableEvaluationTelemetryOptOutParameter in dd-trace-java#12884.
If you prefer the current names, that is fine too. Only the stub name is bound by CallTarget. The constant and the helper stay internal and can change later.
| { | ||
| // OpenFeature obtains this list once for each evaluation. Fresh wrappers isolate Before | ||
| // decisions across nested/concurrent calls; disabled providers keep their cached hooks. | ||
| return _evaluationTelemetryOptOutParameterEnabled && _providerHooks.Count != 0 |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
Heads up: this is about the rebase onto master, not the current head. Master has checks that find the EVP hook by its type in GetProviderHooks(). After the rebase, with the parameter on, this method returns EvaluationTelemetryOptOutHook wrappers, so those checks would report no EVP hook even when one exists.
This branch does not have those checks yet, and master has no wrappers. In master's versions of these checks I see nothing that turns the parameter on, so I expect them to keep passing after the rebase. A later test that turns the parameter on and reuses them would get a false negative.
Where — the type checks on master
tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FlagEvalEVPHookTests.cs:327:provider.GetProviderHooks().OfType<FlagEvalEVPHook>()tracer/test/test-applications/integrations/Samples.OpenFeature/EvaluationEventsSample.cs:84andEvaluator.cs:28:hook.GetType().Name == "FlagEvalEVPHook"
If you want them to stay valid with the parameter on, expose the wrapped hook (for example internal Hook Inner => _inner;) and unwrap it in the unit test. The sample cannot see internals, so it would need its own check.
FFE reviewer selectionReviewers: @vjfridge, @sameerank Why: Privacy opt-out and event logging rules — The PR adds a per-subject privacy opt-out. Reviewers come from the matched pod. 🤖 Automated selection. Selection detailsInitiative: project / ufc-compliance Why this match: The PR adds a per-subject privacy opt-out. When the evaluation context sets dd_evaluation_telemetry_opt_out=true, the .NET OpenFeature provider stops emitting evaluation telemetry (flag evaluation metrics and span annotations) but still evaluates flags normally. The opt-out is enabled by a tracer setting or a provider option. That is per-context suppression of evaluation event egress, which is this project's scope. flag-evaluation-evp explicitly excludes this kind of suppression.
Additional review: openfeature-architecture — The provider's public API and hook contract change. DatadogProvider gets a new public constructor that takes DatadogProviderOptions. GetProviderHooks no longer returns a cached list: when the opt-out is enabled, it builds new wrapper hooks on every evaluation so that Before-hook decisions stay separate across nested and concurrent calls. Coverage: @sameerank. Evidence: tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.cs, tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.Experimental.cs, tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProviderOptions.cs, tracer/src/Datadog.FeatureFlags.OpenFeature/EvaluationTelemetryOptOutHook.cs The PR author was excluded. Existing reviewers and teams were preserved. Debug or fix a problemWorkflow run · Bits Code session
Evidence and run details✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head. Generated asynchronously by the FFE reviewer-routing workflow. Changed paths: Evaluated head: Registry revision: Pinned skill source: Instructions: workflow-injected snapshot (no plugin loaded) Instruction snapshot SHA-256: |
|
🤖 Comment from Codex working with @leoromanovsky: In response to #9426 (comment): the saved summary identifies system-tests I could not recover the original JSON wrapper or raw captures. I cannot supply a complete reproduction command or verify the original collected test IDs. The validation row should be treated as a historical report, not a reproducible receipt. The JSON-only automated regression test is being handled separately in its inline thread. |
Summary of changes
Add per-subject opt-out for flag evaluation metrics and automatic flag span annotations.
Flag results and experiment exposures remain unchanged.
Reason for change
Applications need to control evaluation telemetry independently of feature delivery and experimentation participation.
Implementation details
DD_FEATURE_FLAGS_EVALUATION_TELEMETRY_PER_SUBJECT_OPT_OUT_ENABLED=truethrough normal tracer configuration, or use the provider option below. Support is disabled by default.false.trueatdd_evaluation_telemetry_opt_outsuppresses new evaluation telemetry. Previously recorded data remains unchanged.Test coverage
Tests cover explicit activation, configuration precedence, typed-boolean handling, context overrides, hook failures, and nested and concurrent evaluations.
They also check unchanged results, targeting attributes, exposures, and exposure deduplication.
Runtime validation
DataDog/dd-trace-dotnet@773c32182e4972531bbc20bdedbd62978711a112Other details
FFE reviewer selection
Reviewers: @greghuels, @sameerank
Why: Privacy opt-out and event logging rules — Adds a per-subject opt-out: when the evaluation context has boolean true at dd_evaluation_telemetry_opt_out, the .NET OpenFeature provider suppresses new evaluation metrics and… Global reviewers cover a shortage in the matched pod.
🤖 Automated selection.
Selection details
Initiative: project / ufc-compliance
Why this match: Adds a per-subject opt-out: when the evaluation context has boolean true at dd_evaluation_telemetry_opt_out, the .NET OpenFeature provider suppresses new evaluation metrics and flag span annotations. The new EvaluationTelemetryOptOutHook does this by skipping the inner telemetry hook's Finally. Flag results, targeting and exposures stay unchanged. Support is gated by a new tracer configuration key and a provider option. This matches per-context suppression of evaluation event egress for user opt-out, which the flag-evaluation-evp catalog entry explicitly assigns to ufc-compliance. It does not add EVP payloads or consent propagation.
Why these owners: The matched initiative has fewer than two approved, non-author candidates. The approved global pool supplies the remaining review owners; proposed memberships were not activated.
The PR author was excluded. Existing reviewers and teams were preserved.
Debug or fix a problem
Workflow run · Bits Code session
dry_run=false. Unchanged completed runs are skipped. If the receipt saysneeds_reconciliationor a write outcome is uncertain, ask the workflow owner to reconcile GitHub and the receipt before retrying; do not delete the lock blindly.Evidence and run details
✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head.
Generated asynchronously by the FFE reviewer-routing workflow.
Changed paths:
tracer/src/Datadog.FeatureFlags.OpenFeature/EvaluationTelemetryOptOutHook.cs,tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.cs,tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProviderOptions.cs,tracer/src/Datadog.FeatureFlags.OpenFeature/ProviderEnvironmentConfiguration.cs,tracer/src/Datadog.FeatureFlags.OpenFeature/SpanEnrichmentHook.cs,tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs,tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkIsEvaluationTelemetryOptOutEnabledIntegration.cs,tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsSettings.cs,tracer/src/Datadog.Trace/Configuration/supported-configurations.yamlEvaluated head:
773c32182e4972531bbc20bdedbd62978711a112Registry revision:
revision:1946aa2a-530e-487a-84a7-83aff44a1089Pinned skill source:
d214cfc65ae154b97d39af5cf32caf48aaec9bf0Instructions: workflow-injected snapshot (no plugin loaded)
Instruction snapshot SHA-256:
f23b2a88c13c0da90e2dd58246a14f257af91e2195c555039bd555b52e6f3516