From 6dbb277e1bcb3a0051dc4120c2bd82687d4de059 Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Tue, 3 Nov 2026 18:41:28 -0600 Subject: [PATCH 1/5] Support dependent flag telemetry in OpenFeature Publish ordered dependency evaluations, commit exposure batches only after successful root resolution, and bound exposure delivery and context capture. Co-Authored-By: Codex GPT-5 Environment: Datadog workspace Co-authored-by: tyler.potter --- .../DatadogProvider.Experimental.cs | 11 +- .../DatadogProvider.cs | 19 +- .../ExposureCommitHook.cs | 80 ++ .../FlagEvalMetricsHook.cs | 20 +- .../Sdk/FeatureFlagsSdk.cs | 47 +- .../OpenFeatureSdkEvaluateIntegration.cs | 15 +- .../Agentless/AgentlessConfigurationSource.cs | 22 +- .../FeatureFlags/DependencyEvaluation.cs | 220 +++++ .../FeatureFlags/EvaluationTrace.cs | 26 + .../FeatureFlags/Exposure/ExposureApi.cs | 65 +- .../FeatureFlags/ExposureCommitMetadata.cs | 17 + .../FeatureFlags/FeatureFlagsEvaluator.cs | 793 +++++++++++++----- .../FeatureFlags/FeatureFlagsModule.cs | 79 +- .../Rcm/Model/ConditionConfiguration.cs | 26 +- .../FeatureFlags/Rcm/Model/EvaluatorParams.cs | 25 + .../Rcm/Model/FlagCollectionJsonConverter.cs | 25 +- .../Rcm/Model/FlagEvaluationConfiguration.cs | 32 + .../Rcm/Model/ServerConfiguration.cs | 19 + .../CanonicalFixtureTests.cs | 2 +- .../ExposureCommitHookTests.cs | 127 +++ .../FeatureFlagsSdkTests.cs | 20 +- .../FlagEvalEVPHookTests.cs | 2 +- .../SynchronousHookTests.cs | 33 +- .../AgentlessConfigurationSourceTests.cs | 15 + .../FeatureFlags/ExposureApiTests.cs | 39 + .../FeatureFlagsDependentFlagTests.cs | 380 +++++++++ .../FeatureFlagsEvaluatorTests.cs | 74 ++ .../FeatureFlags/FlagEvaluationModuleTests.cs | 153 ++++ .../UfcConfigurationParserTests.cs | 58 ++ 29 files changed, 2171 insertions(+), 273 deletions(-) create mode 100644 tracer/src/Datadog.FeatureFlags.OpenFeature/ExposureCommitHook.cs create mode 100644 tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs create mode 100644 tracer/src/Datadog.Trace/FeatureFlags/EvaluationTrace.cs create mode 100644 tracer/src/Datadog.Trace/FeatureFlags/ExposureCommitMetadata.cs create mode 100644 tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/EvaluatorParams.cs create mode 100644 tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagEvaluationConfiguration.cs create mode 100644 tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/ExposureCommitHookTests.cs create mode 100644 tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.Experimental.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.Experimental.cs index eb35f5e37254..78d3351ddd29 100644 --- a/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.Experimental.cs +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.Experimental.cs @@ -118,6 +118,7 @@ internal static ResolutionDetails RunProviderHooks(IImmutableList ho } var details = ToDetails(resolution); + var originalMetadata = resolution.FlagMetadata; Exception? error = resolution.ErrorType == ErrorType.None ? null : new FeatureProviderException(resolution.ErrorType, resolution.ErrorMessage); if (error is null) { @@ -167,6 +168,14 @@ internal static ResolutionDetails RunProviderHooks(IImmutableList ho } } + foreach (var hook in hooks) + { + if (hook is ExposureCommitHook exposureCommitHook) + { + exposureCommitHook.Complete(originalMetadata, resolution.ErrorType == ErrorType.None); + } + } + return resolution; } @@ -188,7 +197,7 @@ private static void Wait(ValueTask task) private ResolutionDetails ResolveSync(string flagKey, Trace.FeatureFlags.ValueType valueType, FlagValueType flagValueType, T defaultValue, EvaluationContext? context, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - var resolution = FeatureFlagsSdk.Resolve(flagKey, valueType, defaultValue, context, _evaluate, _evpHook); + var resolution = FeatureFlagsSdk.Resolve(flagKey, valueType, defaultValue, context, _evaluate, _evpHook, _exposureCommitHook); return _providerHooks.Count == 0 ? resolution : RunProviderHooks(_providerHooks, new HookContext(flagKey, defaultValue, flagValueType, DirectClientMetadata, _metadata, context ?? EvaluationContext.Empty), resolution); diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.cs index 366ae3954d06..987c1b6b005f 100644 --- a/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.cs +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/DatadogProvider.cs @@ -43,6 +43,7 @@ public sealed partial class DatadogProvider : global::OpenFeature.FeatureProvide private readonly SpanEnrichmentHook? _spanEnrichmentHook; // Startup gate: enabling events later requires a new provider (or application restart). private readonly FlagEvalEVPHook? _evpHook; + private readonly ExposureCommitHook _exposureCommitHook = new(); // Hooks are fixed at construction, so the list is built once per provider and reused for every evaluation. private readonly IImmutableList _providerHooks; @@ -233,7 +234,7 @@ public override async Task InitializeAsync(EvaluationContext context, Cancellati public override Task> ResolveBooleanValueAsync(string flagKey, bool defaultValue, EvaluationContext? context = null, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Boolean, defaultValue, context, _evaluate, _evpHook); + var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Boolean, defaultValue, context, _evaluate, _evpHook, _exposureCommitHook); return Task.FromResult(res); } @@ -246,7 +247,7 @@ public override Task> ResolveBooleanValueAsync(string fl public override Task> ResolveDoubleValueAsync(string flagKey, double defaultValue, EvaluationContext? context = null, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Numeric, defaultValue, context, _evaluate, _evpHook); + var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Numeric, defaultValue, context, _evaluate, _evpHook, _exposureCommitHook); return Task.FromResult(res); } @@ -259,7 +260,7 @@ public override Task> ResolveDoubleValueAsync(string f public override Task> ResolveIntegerValueAsync(string flagKey, int defaultValue, EvaluationContext? context = null, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Integer, defaultValue, context, _evaluate, _evpHook); + var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Integer, defaultValue, context, _evaluate, _evpHook, _exposureCommitHook); return Task.FromResult(res); } @@ -272,7 +273,7 @@ public override Task> ResolveIntegerValueAsync(string fla public override Task> ResolveStringValueAsync(string flagKey, string defaultValue, EvaluationContext? context = null, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.String, defaultValue, context, _evaluate, _evpHook); + var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.String, defaultValue, context, _evaluate, _evpHook, _exposureCommitHook); return Task.FromResult(res); } @@ -285,7 +286,7 @@ public override Task> ResolveStringValueAsync(string f public override Task> ResolveStructureValueAsync(string flagKey, Value defaultValue, EvaluationContext? context = null, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Json, defaultValue, context, _evaluate, _evpHook); + var res = FeatureFlagsSdk.Resolve(flagKey, Trace.FeatureFlags.ValueType.Json, defaultValue, context, _evaluate, _evpHook, _exposureCommitHook); return Task.FromResult(res); } @@ -308,17 +309,17 @@ private IImmutableList CreateProviderHooks() #if NET6_0_OR_GREATER if (_spanEnrichmentHook is not null) { - return _evpHook is null ? ImmutableList.Create(_metricsHook, _spanEnrichmentHook) : ImmutableList.Create(_metricsHook, _spanEnrichmentHook, _evpHook); + return _evpHook is null ? ImmutableList.Create(_metricsHook, _spanEnrichmentHook, _exposureCommitHook) : ImmutableList.Create(_metricsHook, _spanEnrichmentHook, _evpHook, _exposureCommitHook); } - return _evpHook is null ? ImmutableList.Create(_metricsHook) : ImmutableList.Create(_metricsHook, _evpHook); + return _evpHook is null ? ImmutableList.Create(_metricsHook, _exposureCommitHook) : ImmutableList.Create(_metricsHook, _evpHook, _exposureCommitHook); #else if (_spanEnrichmentHook is not null) { - return _evpHook is null ? ImmutableList.Create(_spanEnrichmentHook) : ImmutableList.Create(_spanEnrichmentHook, _evpHook); + return _evpHook is null ? ImmutableList.Create(_spanEnrichmentHook, _exposureCommitHook) : ImmutableList.Create(_spanEnrichmentHook, _evpHook, _exposureCommitHook); } - return _evpHook is null ? ImmutableList.Empty : ImmutableList.Create(_evpHook); + return _evpHook is null ? ImmutableList.Create(_exposureCommitHook) : ImmutableList.Create(_evpHook, _exposureCommitHook); #endif } } diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/ExposureCommitHook.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/ExposureCommitHook.cs new file mode 100644 index 000000000000..d765230ee0da --- /dev/null +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/ExposureCommitHook.cs @@ -0,0 +1,80 @@ +// +// 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. +// + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using OpenFeature; +using OpenFeature.Constant; +using OpenFeature.Model; + +namespace Datadog.FeatureFlags.OpenFeature; + +internal sealed class ExposureCommitHook : Hook +{ + private readonly ConditionalWeakTable _commits = new(); + + internal void Capture(ImmutableMetadata? metadata, Action? commit) + { + try + { + if (metadata is not null && commit is not null) + { + // The immutable metadata instance is unique to one OpenFeature invocation. The weak + // key prevents an abandoned provider result from retaining its pending exposure batch. + _commits.Add(metadata, new CommitOwner(commit)); + } + } + catch + { + // Exposure reporting is best effort and must never affect the application result. + } + } + + internal void Complete(ImmutableMetadata? metadata, bool success) + { + try + { + if (metadata is not null && _commits.TryGetValue(metadata, out var owner)) + { + _commits.Remove(metadata); + owner.Complete(success); + } + } + catch + { + // Exposure reporting is best effort and must never affect the application result. + } + } + + public override ValueTask FinallyAsync( + HookContext context, + FlagEvaluationDetails details, + IReadOnlyDictionary? hints = null, + CancellationToken cancellationToken = default) + { + Complete(details.FlagMetadata, details.ErrorType == ErrorType.None); + + return default; + } + + private sealed class CommitOwner(Action commit) + { + private Action? _commit = commit; + + public void Complete(bool success) + { + var action = Interlocked.Exchange(ref _commit, null); + if (success) + { + action?.Invoke(); + } + } + } +} diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/FlagEvalMetricsHook.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/FlagEvalMetricsHook.cs index 05f7f9cd9924..11892c65b427 100644 --- a/tracer/src/Datadog.FeatureFlags.OpenFeature/FlagEvalMetricsHook.cs +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/FlagEvalMetricsHook.cs @@ -45,9 +45,7 @@ public override ValueTask FinallyAsync( var flagKey = context.FlagKey; var variant = details.Variant ?? string.Empty; - // Reason is already in lower_snake_case format from the provider - // Use "unknown" as fallback for missing reason (matches OpenFeature SDK telemetry convention) - var reason = string.IsNullOrEmpty(details.Reason) ? "unknown" : details.Reason; + var reason = ToTelemetryReason(details.Reason); // Extract error type if present string? errorType = null; @@ -76,6 +74,22 @@ public void Dispose() _metrics.Dispose(); } + // OpenFeature reasons are canonical uppercase strings, while the Datadog telemetry + // convention uses lower_snake_case tags. Cache the standard mappings on this hot path. + private static string ToTelemetryReason(string? reason) => reason switch + { + null or "" => "unknown", + Reason.TargetingMatch => "targeting_match", + Reason.Split => "split", + Reason.Disabled => "disabled", + Reason.Default => "default", + Reason.Static => "static", + Reason.Cached => "cached", + Reason.Unknown => "unknown", + Reason.Error => "error", + _ => reason.ToLowerInvariant() + }; + private static string ErrorTypeToString(ErrorType errorType) { return errorType switch diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs index fb1c04628e3f..642f6d99e3d6 100644 --- a/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs @@ -100,14 +100,16 @@ internal static void AccumulateSpanEnrichment(long? serialId, bool doLog, string { } - public static ResolutionDetails Resolve(string flagKey, Trace.FeatureFlags.ValueType targetType, T defaultValue, EvaluationContext? context, EvaluationCallback? evaluate = null, FlagEvalEVPHook? evaluationEventsHook = null) + public static ResolutionDetails Resolve(string flagKey, Trace.FeatureFlags.ValueType targetType, T defaultValue, EvaluationContext? context, EvaluationCallback? evaluate = null, FlagEvalEVPHook? evaluationEventsHook = null, ExposureCommitHook? exposureCommitHook = null) { long? evalTimeMs = evaluationEventsHook is not null ? DateTimeOffset.UtcNow.ToUnixTimeMilliseconds() : null; var attributes = GetContextAttributes(context); var evaluation = evaluate is null ? Evaluate(flagKey, targetType, defaultValue, context?.TargetingKey, attributes) : evaluate(flagKey, targetType, defaultValue, context?.TargetingKey, attributes); + var exposureCommit = GetExposureCommit(evaluation?.FlagMetadata); var resolution = GetResolutionDetails(flagKey, defaultValue, evaluation, evalTimeMs); + exposureCommitHook?.Capture(resolution.FlagMetadata, exposureCommit); evaluationEventsHook?.CaptureEvaluation(context, resolution.FlagMetadata); return resolution; } @@ -155,7 +157,7 @@ internal static ResolutionDetails GetResolutionDetails(string flagKey, T d evaluation.FlagKey, value, errorType, - ReasonToLowerSnakeCase(evaluation.Reason), + ToOpenFeatureReason(evaluation.Reason), evaluation.Variant, evaluation.Error, ToMetadata(evaluation.FlagMetadata, evalTimeMs)); @@ -178,19 +180,17 @@ private static ErrorType ToErrorType(Datadog.Trace.FeatureFlags.EvaluationReason }; } - // Converts EvaluationReason enum to lower_snake_case string for OpenFeature Reason field. - // Uses cached strings to avoid allocation. - private static string ReasonToLowerSnakeCase(Datadog.Trace.FeatureFlags.EvaluationReason reason) => reason switch + private static string ToOpenFeatureReason(Datadog.Trace.FeatureFlags.EvaluationReason reason) => reason switch { - Datadog.Trace.FeatureFlags.EvaluationReason.Static => "static", - Datadog.Trace.FeatureFlags.EvaluationReason.Default => "default", - Datadog.Trace.FeatureFlags.EvaluationReason.TargetingMatch => "targeting_match", - Datadog.Trace.FeatureFlags.EvaluationReason.Split => "split", - Datadog.Trace.FeatureFlags.EvaluationReason.Disabled => "disabled", - Datadog.Trace.FeatureFlags.EvaluationReason.Cached => "cached", - Datadog.Trace.FeatureFlags.EvaluationReason.Unknown => "unknown", - Datadog.Trace.FeatureFlags.EvaluationReason.Error => "error", - _ => "unknown" + Datadog.Trace.FeatureFlags.EvaluationReason.Static => Reason.Static, + Datadog.Trace.FeatureFlags.EvaluationReason.Default => Reason.Default, + Datadog.Trace.FeatureFlags.EvaluationReason.TargetingMatch => Reason.TargetingMatch, + Datadog.Trace.FeatureFlags.EvaluationReason.Split => Reason.Split, + Datadog.Trace.FeatureFlags.EvaluationReason.Disabled => Reason.Disabled, + Datadog.Trace.FeatureFlags.EvaluationReason.Cached => Reason.Cached, + Datadog.Trace.FeatureFlags.EvaluationReason.Unknown => Reason.Unknown, + Datadog.Trace.FeatureFlags.EvaluationReason.Error => Reason.Error, + _ => Reason.Unknown }; private static ImmutableMetadata? ToMetadata(IDictionary? metadata, long? evalTimeMs) @@ -218,6 +218,25 @@ private static ErrorType ToErrorType(Datadog.Trace.FeatureFlags.EvaluationReason return new ImmutableMetadata(dic); } + private static Action? GetExposureCommit(IDictionary? metadata) + { + try + { + var type = metadata?.GetType(); + if (type?.FullName != "Datadog.Trace.FeatureFlags.ExposureCommitMetadata" || + type.Assembly.GetName().Name != "Datadog.Trace") + { + return null; + } + + return type.GetProperty("ExposureCommit")?.GetValue(metadata) as Action; + } + catch + { + return null; + } + } + public static Value JsonToValue(object? obj) { try diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs index e265a8e84b2f..203540260c87 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs @@ -49,10 +49,23 @@ internal static CallTargetState OnMethodBegin(ref string flagKey, Featu // via the manual API or an older OpenFeature package), start delivery on first evaluation. flags?.Activate(); - var res = flags?.Evaluate(parameters.FlagKey, parameters.TargetType, parameters.DefaultValue, parameters.TargetingKey, parameters.Attributes); + var res = flags?.Evaluate( + parameters.FlagKey, + parameters.TargetType, + parameters.DefaultValue, + parameters.TargetingKey, + parameters.Attributes, + SupportsDeferredExposures.Value); return new CallTargetReturn(res.DuckCast()); } + private static class SupportsDeferredExposures + { + // Older SDKs have no commit hook, so they retain immediate delivery. Inspecting the SDK + // assembly keeps both tracer-first and SDK-first upgrades compatible without shared state. + public static readonly bool Value = typeof(TReturn).Assembly.GetType("Datadog.FeatureFlags.OpenFeature.ExposureCommitHook", throwOnError: false) is not null; + } + private sealed record State(string FlagKey, FeatureFlags.ValueType TargetType, object? DefaultValue, string? TargetingKey, IDictionary? Attributes) { } diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Agentless/AgentlessConfigurationSource.cs b/tracer/src/Datadog.Trace/FeatureFlags/Agentless/AgentlessConfigurationSource.cs index f4772c9134bf..529bf4d6068c 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/Agentless/AgentlessConfigurationSource.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/Agentless/AgentlessConfigurationSource.cs @@ -32,6 +32,8 @@ internal sealed class AgentlessConfigurationSource : IFeatureFlagsDeliverySource { private const int MaxAttempts = 3; private const double RetryJitter = 0.2; + internal const string FeatureFlagsCapabilitiesHeader = "X-DD-FEATURE-FLAGS-CAPABILITIES"; + internal const string DependentFlagCapability = "dependent-flag-variant-key-v1"; private static readonly TimeSpan FirstRetryMin = TimeSpan.FromSeconds(2); private static readonly TimeSpan FirstRetryMax = TimeSpan.FromSeconds(10); @@ -263,6 +265,17 @@ private static HttpClientRequestFactory CreateRequestFactory(AgentlessEndpoint e #else private static ApiWebRequestFactory CreateRequestFactory(AgentlessEndpoint endpoint, FeatureFlagsSettings settings) #endif + { + var headers = GetRequestHeaders(endpoint, settings); + +#if NETCOREAPP + return new HttpClientRequestFactory(endpoint.Uri, headers, timeout: settings.RequestTimeout); +#else + return new ApiWebRequestFactory(endpoint.Uri, headers, timeout: settings.RequestTimeout); +#endif + } + + internal static KeyValuePair[] GetRequestHeaders(AgentlessEndpoint endpoint, FeatureFlagsSettings settings) { var headers = new List> { @@ -270,6 +283,7 @@ private static ApiWebRequestFactory CreateRequestFactory(AgentlessEndpoint endpo new("Accept-Encoding", "gzip"), new(TelemetryConstants.ClientLibraryLanguageHeader, TracerConstants.Language), new(TelemetryConstants.ClientLibraryVersionHeader, TracerConstants.ThreePartVersion), + new(FeatureFlagsCapabilitiesHeader, DependentFlagCapability), // Without this the poll is itself instrumented, producing a span per poll and letting // auto-instrumentation recurse through the poller's own client. @@ -283,13 +297,7 @@ private static ApiWebRequestFactory CreateRequestFactory(AgentlessEndpoint endpo headers.Add(new(TelemetryConstants.ApiKeyHeader, settings.ApiKey!)); } - // The endpoint is only the factory's default: both transports honour the URI passed to - // Create, which is what carries the current environment. -#if NETCOREAPP - return new HttpClientRequestFactory(endpoint.Uri, headers.ToArray(), timeout: settings.RequestTimeout); -#else - return new ApiWebRequestFactory(endpoint.Uri, headers.ToArray(), timeout: settings.RequestTimeout); -#endif + return headers.ToArray(); } private async Task RunAsync() diff --git a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs new file mode 100644 index 000000000000..0db7fab7ad72 --- /dev/null +++ b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs @@ -0,0 +1,220 @@ +// +// 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. +// + +#nullable enable + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Globalization; +using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; + +namespace Datadog.Trace.FeatureFlags; + +internal sealed class DependencyEvaluation( + Evaluation evaluation, + DateTime evaluationTime, + string? targetingKey, + IReadOnlyDictionary? contextAttributes) +{ + public Evaluation Evaluation { get; } = evaluation; + + public DateTime EvaluationTime { get; } = evaluationTime; + + public string? TargetingKey { get; } = targetingKey; + + public IReadOnlyDictionary? ContextAttributes { get; } = contextAttributes; + + public static DependencyEvaluation Snapshot( + Evaluation evaluation, + DateTime evaluationTime, + string? targetingKey, + IReadOnlyDictionary? contextAttributes) + { + try + { + var metadata = evaluation.FlagMetadata is null ? null : new Dictionary(evaluation.FlagMetadata); + var detached = new Evaluation( + evaluation.FlagKey, + null, + evaluation.Reason, + evaluation.Variant, + evaluation.Error, + metadata); + return new DependencyEvaluation(detached, evaluationTime, targetingKey, contextAttributes); + } + catch + { + // Telemetry detachment is best effort. Retain the dependency result without potentially + // unsafe values or attributes so instrumentation cannot affect flag resolution. + var metadata = new Dictionary(); + CopyMetadata(FeatureFlagMetadataKeys.ObserveFullEvaluationData); + CopyMetadata(FeatureFlagsEvaluator.MetadataAllocationKey); + CopyMetadata(FeatureFlagsEvaluator.MetadataErrorCode); + var detached = new Evaluation( + evaluation.FlagKey, + null, + evaluation.Reason, + evaluation.Variant, + evaluation.Error, + metadata); + return new DependencyEvaluation(detached, evaluationTime, targetingKey, null); + + void CopyMetadata(string key) + { + try + { + if (evaluation.FlagMetadata?.TryGetValue(key, out var value) == true) + { + metadata[key] = value; + } + } + catch + { + // The minimal fallback must remain non-throwing even if a future evaluation + // implementation supplies custom metadata. + } + } + } + } + + public static IReadOnlyDictionary? SnapshotContext(EvaluationContext? context, bool includeContext) + { + if (!includeContext || context is null) + { + return null; + } + + try + { + var remainingNodes = 1_280; + var attributes = new Dictionary(); + foreach (var pair in context.Attributes) + { + if (remainingNodes <= 0) + { + break; + } + + attributes[pair.Key] = Detach(pair.Value, 0, ref remainingNodes); + } + + return attributes; + } + catch + { + return null; + } + } + + private static object? Detach(object? value, int depth, ref int remainingNodes) + { + if (depth >= 32 || remainingNodes-- <= 0) + { + return null; + } + + return value switch + { + null or string or bool or byte or sbyte or short or ushort or int or uint or long or ulong or float or double or decimal or char or DateTime or DateTimeOffset or Guid => value, + JValue valueToken => Detach(valueToken.Value, depth + 1, ref remainingNodes), + JObject objectToken => DetachObject(objectToken, depth, ref remainingNodes), + JArray arrayToken => DetachArray(arrayToken, depth, ref remainingNodes), + JToken => null, + IDictionary dictionary => DetachDictionary(dictionary, depth, ref remainingNodes), + IDictionary dictionary => DetachDictionary(dictionary, depth, ref remainingNodes), + IEnumerable enumerable => DetachEnumerable(enumerable, depth, ref remainingNodes), + _ => Convert.ToString(value, CultureInfo.InvariantCulture), + }; + } + + private static Dictionary DetachDictionary( + IEnumerable> dictionary, + int depth, + ref int remainingNodes) + { + var detached = new Dictionary(); + foreach (var pair in dictionary) + { + if (remainingNodes <= 0) + { + break; + } + + detached[pair.Key] = Detach(pair.Value, depth + 1, ref remainingNodes); + } + + return detached; + } + + private static Dictionary DetachDictionary(IDictionary dictionary, int depth, ref int remainingNodes) + { + var detached = new Dictionary(); + foreach (var item in dictionary) + { + if (item is not DictionaryEntry entry) + { + continue; + } + + if (remainingNodes <= 0) + { + break; + } + + detached[Convert.ToString(entry.Key) ?? string.Empty] = Detach(entry.Value, depth + 1, ref remainingNodes); + } + + return detached; + } + + private static object?[] DetachEnumerable(IEnumerable enumerable, int depth, ref int remainingNodes) + { + var detached = new List(); + foreach (var item in enumerable) + { + if (remainingNodes <= 0) + { + break; + } + + detached.Add(Detach(item, depth + 1, ref remainingNodes)); + } + + return detached.ToArray(); + } + + private static Dictionary DetachObject(JObject value, int depth, ref int remainingNodes) + { + var detached = new Dictionary(); + foreach (var property in value.Properties()) + { + if (remainingNodes <= 0) + { + break; + } + + detached[property.Name] = Detach(property.Value, depth + 1, ref remainingNodes); + } + + return detached; + } + + private static object?[] DetachArray(JArray value, int depth, ref int remainingNodes) + { + var detached = new List(); + foreach (var item in value) + { + if (remainingNodes <= 0) + { + break; + } + + detached.Add(Detach(item, depth + 1, ref remainingNodes)); + } + + return detached.ToArray(); + } +} diff --git a/tracer/src/Datadog.Trace/FeatureFlags/EvaluationTrace.cs b/tracer/src/Datadog.Trace/FeatureFlags/EvaluationTrace.cs new file mode 100644 index 000000000000..71ae5b1c94fb --- /dev/null +++ b/tracer/src/Datadog.Trace/FeatureFlags/EvaluationTrace.cs @@ -0,0 +1,26 @@ +// +// 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. +// + +#nullable enable + +using System; +using System.Collections.Generic; +using Datadog.Trace.FeatureFlags.Exposure.Model; + +namespace Datadog.Trace.FeatureFlags; + +internal sealed class EvaluationTrace( + Evaluation details, + IReadOnlyList dependencyEvaluations, + IReadOnlyList dependencyExposures) +{ + public Evaluation Details { get; } = details; + + public IReadOnlyList DependencyEvaluations { get; } = dependencyEvaluations; + + public IReadOnlyList DependencyExposures { get; } = dependencyExposures; + + public static EvaluationTrace Empty(Evaluation details) => new(details, Array.Empty(), Array.Empty()); +} diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Exposure/ExposureApi.cs b/tracer/src/Datadog.Trace/FeatureFlags/Exposure/ExposureApi.cs index a481ababf776..ad5462be7770 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/Exposure/ExposureApi.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/Exposure/ExposureApi.cs @@ -22,6 +22,7 @@ namespace Datadog.Trace.FeatureFlags.Exposure; internal sealed class ExposureApi : IDisposable { private const int DefaultCapacity = 1 << 16; // 65536 elements + private const int DefaultBatchSize = 1_000; private static readonly TimeSpan DefaultSendInterval = TimeSpan.FromSeconds(10); private static readonly TimeSpan DefaultShutdownTimeout = TimeSpan.FromSeconds(10); @@ -39,6 +40,8 @@ internal sealed class ExposureApi : IDisposable private readonly object _lifecycleLock = new(); private readonly TimeSpan _sendInterval; private readonly TimeSpan _shutdownTimeout; + private readonly int _queueCapacity; + private readonly int _batchSize; private readonly Queue _exposures = new Queue(); private readonly ExposureCache _exposureCache = new ExposureCache(DefaultCapacity); private readonly FeatureFlagsEvpTransport _transport; @@ -54,12 +57,26 @@ internal ExposureApi( FeatureFlagsEvpTransport transport, TimeSpan? sendInterval = null, TimeSpan? shutdownTimeout = null, - IDatadogLogger? logger = null) + IDatadogLogger? logger = null, + int queueCapacity = DefaultCapacity, + int batchSize = DefaultBatchSize) { + if (queueCapacity <= 0) + { + throw new ArgumentOutOfRangeException(nameof(queueCapacity)); + } + + if (batchSize <= 0) + { + throw new ArgumentOutOfRangeException(nameof(batchSize)); + } + _log = logger ?? DatadogLogging.GetLoggerFor(typeof(ExposureApi)); _transport = transport; _sendInterval = sendInterval ?? DefaultSendInterval; _shutdownTimeout = shutdownTimeout ?? DefaultShutdownTimeout; + _queueCapacity = queueCapacity; + _batchSize = batchSize; UpdateContext(tracerSettings.Manager.InitialMutableSettings); _settingsSubscription = tracerSettings.Manager.SubscribeToChanges(changes => @@ -117,7 +134,7 @@ private async Task SendLoopAsync() _log.Debug("ExposureApi::SendLoopAsync -> Enter"); while (!_processExit.Task.IsCompleted) { - await FlushAsync().ConfigureAwait(false); + await FlushPendingAsync().ConfigureAwait(false); try { @@ -131,7 +148,7 @@ private async Task SendLoopAsync() // Dispose signals the loop and then waits for this bounded final flush. This prevents the // common short-lived-process loss mode without allowing shutdown to hang indefinitely. - var flushed = await FlushAsync().ConfigureAwait(false); + var flushed = await FlushPendingAsync().ConfigureAwait(false); _log.Debug("Feature Flags: final exposure flush handed {ExposureCount} exposure(s) to the transport; see transport outcome", flushed); _log.Debug("ExposureApi::SendLoopAsync -> Exit"); } @@ -160,6 +177,23 @@ private async Task FlushAsync() return 0; } + private async Task FlushPendingAsync() + { + int batchCount; + lock (_exposures) + { + batchCount = _exposures.Count == 0 ? 0 : ((_exposures.Count - 1) / _batchSize) + 1; + } + + var flushed = 0; + for (var i = 0; i < batchCount; i++) + { + flushed += await FlushAsync().ConfigureAwait(false); + } + + return flushed; + } + private ExposuresRequest? TryGetPayload() { List exposures; @@ -171,8 +205,12 @@ private async Task FlushAsync() return null; } - exposures = [.. _exposures]; - _exposures.Clear(); + var count = Math.Min(_batchSize, _exposures.Count); + exposures = new List(count); + for (var i = 0; i < count; i++) + { + exposures.Add(_exposures.Dequeue()); + } } return new ExposuresRequest(_context, exposures); @@ -182,18 +220,23 @@ public void SendExposure(in ExposureEvent exposure) { bool stopped; var queued = false; + var full = false; lock (_lifecycleLock) { stopped = _disposed; if (!stopped) { - queued = _exposureCache.Add(exposure); - if (queued) + lock (_exposures) { - lock (_exposures) + full = _exposures.Count >= _queueCapacity; + if (!full) { - _exposures.Enqueue(exposure); + queued = _exposureCache.Add(exposure); + if (queued) + { + _exposures.Enqueue(exposure); + } } } @@ -215,6 +258,10 @@ public void SendExposure(in ExposureEvent exposure) { _log.Debug("Feature Flags: exposure for {FlagKey} dropped: exposure reporting has stopped", exposure.Flag.Key); } + else if (full) + { + _log.Debug("Feature Flags: exposure for {FlagKey} dropped: exposure queue is full", exposure.Flag.Key); + } else if (queued) { _log.Debug( diff --git a/tracer/src/Datadog.Trace/FeatureFlags/ExposureCommitMetadata.cs b/tracer/src/Datadog.Trace/FeatureFlags/ExposureCommitMetadata.cs new file mode 100644 index 000000000000..6b586da73658 --- /dev/null +++ b/tracer/src/Datadog.Trace/FeatureFlags/ExposureCommitMetadata.cs @@ -0,0 +1,17 @@ +// +// 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. +// + +#nullable enable + +using System; +using System.Collections.Generic; + +namespace Datadog.Trace.FeatureFlags; + +internal sealed class ExposureCommitMetadata(IDictionary? metadata, Action exposureCommit) + : Dictionary(metadata ?? new Dictionary()) +{ + public Action ExposureCommit { get; } = exposureCommit; +} diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs index 4ee755ef5664..370e71841ae5 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs @@ -32,11 +32,16 @@ internal sealed class FeatureFlagsEvaluator internal static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(FeatureFlagsEvaluator)); + private const int DefaultMaxDependencyDepth = 10; + private readonly ReportExposureDelegate? _onExposureEvent; private readonly ServerConfiguration? _config; private readonly bool _spanEnrichmentEnabled; - public FeatureFlagsEvaluator(ReportExposureDelegate? onExposureEvent, ServerConfiguration? config, bool spanEnrichmentEnabled = false) + public FeatureFlagsEvaluator( + ReportExposureDelegate? onExposureEvent, + ServerConfiguration? config, + bool spanEnrichmentEnabled = false) { _onExposureEvent = onExposureEvent; _config = config; @@ -59,15 +64,39 @@ public FeatureFlagsEvaluator(ReportExposureDelegate? onExposureEvent, ServerConf internal ServerConfiguration? Configuration => _config; public Evaluation Evaluate(string flagKey, ValueType resultType, object? defaultValue, EvaluationContext? context) + { + var trace = EvaluateWithTrace(flagKey, resultType, defaultValue, context, collectDependencyEvaluations: false); + foreach (var exposure in trace.DependencyExposures) + { + try + { + _onExposureEvent?.Invoke(in exposure); + } + catch + { + // Exposure delivery is best effort and must not affect flag evaluation. + } + } + + return trace.Details; + } + + internal EvaluationTrace EvaluateWithTrace( + string flagKey, + ValueType resultType, + object? defaultValue, + EvaluationContext? context, + bool collectDependencyEvaluations = true) { // Capture before evaluating: even an exception after a configuration update keeps this consent. var consent = _config?.ObserveFullEvaluationData == true ? "true" : "false"; + DependencyEvaluationSession? session = null; try { var config = _config; if (config == null) { - return new Evaluation( + return EvaluationTrace.Empty(new Evaluation( flagKey, defaultValue, EvaluationReason.Error, @@ -76,194 +105,140 @@ public Evaluation Evaluate(string flagKey, ValueType resultType, object? default { ["errorCode"] = "PROVIDER_NOT_READY", [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); + })); } - Flag? flag = null; - var lookupResult = config.Flags?.Find(flagKey, out flag) ?? FlagLookupResult.NotFound; - if (lookupResult == FlagLookupResult.NotFound) - { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Error, - error: "FLAG_NOT_FOUND", - metadata: new Dictionary - { - ["errorCode"] = "FLAG_NOT_FOUND", - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); - } + var now = DateTime.UtcNow; + session = new DependencyEvaluationSession(this, config, flagKey, context, now, consent, collectDependencyEvaluations); + var evaluation = EvaluateFlag(flagKey, resultType, defaultValue, context, session, now, consent); + return session.Complete(evaluation); + } + catch (DependencyGraphException ex) + { + return CompleteError(Error(flagKey, defaultValue, ex.ErrorCode, ex.Message, consent)); + } + catch (FormatException ex) + { + return CompleteError(Error(flagKey, defaultValue, "PARSE_ERROR", ex.Message, consent)); + } + catch (MissingTargetingKeyException) + { + return CompleteError(Error(flagKey, defaultValue, "TARGETING_KEY_MISSING", null, consent)); + } + catch (Exception ex) + { + return CompleteError(Error(flagKey, defaultValue, "GENERAL", ex.Message, consent, ex.Message)); + } - if (lookupResult == FlagLookupResult.Invalid || flag is null) - { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Error, - error: "PARSE_ERROR", - metadata: new Dictionary - { - ["errorCode"] = "PARSE_ERROR", - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); - } + EvaluationTrace CompleteError(Evaluation error) => session?.Complete(error) ?? EvaluationTrace.Empty(error); + } - if (flag.Enabled != true) - { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Disabled, - metadata: new Dictionary - { - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); - } +#pragma warning disable SA1204 // Static helpers follow this recursive instance method for evaluator readability + private Evaluation EvaluateFlag( + string flagKey, + ValueType resultType, + object? defaultValue, + EvaluationContext? context, + DependencyEvaluationSession session, + DateTime now, + string consent) + { + Flag? flag = null; + var lookupResult = _config?.Flags?.Find(flagKey, out flag) ?? FlagLookupResult.NotFound; + if (lookupResult == FlagLookupResult.NotFound) + { + return Error(flagKey, defaultValue, "FLAG_NOT_FOUND", null, consent); + } + + if (lookupResult == FlagLookupResult.Invalid || flag is null) + { + return Error(flagKey, defaultValue, "PARSE_ERROR", null, consent); + } + + if (flag.Enabled != true) + { + return new Evaluation( + flagKey, + defaultValue, + EvaluationReason.Disabled, + metadata: ConsentMetadata(consent)); + } + + if (flag.VariationType != resultType) + { + return Error(flagKey, defaultValue, "TYPE_MISMATCH", null, consent); + } + + if (flag.Allocations is null or { Count: 0 }) + { + return new Evaluation( + flagKey, + defaultValue, + EvaluationReason.Default, + metadata: ConsentMetadata(consent)); + } + + var targetingKey = context?.TargetingKey; - if (flag.VariationType != resultType) + foreach (var allocation in flag.Allocations) + { + if (!IsAllocationActive(allocation, now)) { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Error, - error: "TYPE_MISMATCH", - metadata: new Dictionary - { - ["errorCode"] = "TYPE_MISMATCH", - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); + continue; } - if (flag.Allocations is null or { Count: 0 }) + var checkpoint = session.Checkpoint(); + var hadRules = allocation.Rules is { Count: > 0 }; + if (hadRules && !EvaluateRules(allocation.Rules!, context, session)) { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Default, - metadata: new Dictionary - { - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); + session.Rollback(checkpoint); + continue; } - var now = DateTime.UtcNow; - var targetingKey = context?.TargetingKey; - - foreach (var allocation in flag.Allocations) + if (allocation.Splits is { Count: > 0 }) { - if (!IsAllocationActive(allocation, now)) - { - continue; - } - - // Track whether this allocation has targeting rules - var hadRules = allocation.Rules is { Count: > 0 }; - if (hadRules) + foreach (var split in allocation.Splits) { - if (!EvaluateRules(allocation.Rules!, context)) + if (StringUtil.IsNullOrEmpty(split.VariationKey)) { - continue; + throw new FormatException($"Empty variation key in allocation {allocation.Key}"); } - } - if (allocation.Splits is { Count: > 0 }) - { - foreach (var split in allocation.Splits) + var allShardsMatch = true; + var hadShards = split.Shards is { Count: > 0 }; + if (hadShards) { - if (StringUtil.IsNullOrEmpty(split.VariationKey)) + foreach (var shard in split.Shards!) { - throw new FormatException($"Empty variation key in allocation {allocation.Key}"); - } - - var allShardsMatch = true; - var hadShards = split.Shards is { Count: > 0 }; - if (hadShards) - { - foreach (var shard in split.Shards!) + if (!MatchesShard(shard, targetingKey)) { - if (!MatchesShard(shard, targetingKey)) - { - allShardsMatch = false; - break; - } + allShardsMatch = false; + break; } } + } - if (allShardsMatch) - { - // Determine reason based on how the flag was resolved. - // - TargetingMatch: Allocation had targeting rules that matched - // - Default: A temporal allocation with one unsharded split matched - // - Split: Resolved via percentage split without targeting rules - // - Static: No rules, no shards - simple static value - var isTemporalDefault = !hadRules && - !hadShards && - allocation.Splits.Count == 1 && - (!StringUtil.IsNullOrEmpty(allocation.StartAt) || - !StringUtil.IsNullOrEmpty(allocation.EndAt)); - var reason = hadRules ? EvaluationReason.TargetingMatch - : isTemporalDefault ? EvaluationReason.Default - : hadShards ? EvaluationReason.Split - : EvaluationReason.Static; - - return ResolveVariant(flagKey, resultType, defaultValue, flag, split, allocation, reason, now, context, consent); - } + if (allShardsMatch) + { + var isTemporalDefault = !hadRules && + !hadShards && + allocation.Splits.Count == 1 && + (!StringUtil.IsNullOrEmpty(allocation.StartAt) || + !StringUtil.IsNullOrEmpty(allocation.EndAt)); + var reason = hadRules ? EvaluationReason.TargetingMatch + : isTemporalDefault ? EvaluationReason.Default + : hadShards ? EvaluationReason.Split + : EvaluationReason.Static; + + return ResolveVariant(flagKey, resultType, defaultValue, flag, split, allocation, reason, now, context, consent, session); } } } - // No allocation / split matched – use default - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Default, - metadata: new Dictionary - { - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); - } - catch (FormatException ex) - { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Error, - error: "PARSE_ERROR", - metadata: new Dictionary - { - ["errorCode"] = "PARSE_ERROR", - ["message"] = ex.Message, - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); - } - catch (MissingTargetingKeyException) - { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Error, - error: "TARGETING_KEY_MISSING", - metadata: new Dictionary - { - ["errorCode"] = "TARGETING_KEY_MISSING", - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); - } - catch (Exception ex) - { - return new Evaluation( - flagKey, - defaultValue, - EvaluationReason.Error, - error: ex.Message, - metadata: new Dictionary - { - ["errorCode"] = "GENERAL", - ["message"] = ex.Message, - [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent - }); + session.Rollback(checkpoint); } + + return new Evaluation(flagKey, defaultValue, EvaluationReason.Default, metadata: ConsentMetadata(consent)); } private static bool IsAllocationActive(Allocation allocation, DateTime now) @@ -283,7 +258,7 @@ private static bool IsAllocationActive(Allocation allocation, DateTime now) return true; } - private static bool EvaluateRules(List rules, EvaluationContext? context) + private static bool EvaluateRules(List rules, EvaluationContext? context, DependencyEvaluationSession session) { foreach (var rule in rules) { @@ -292,10 +267,11 @@ private static bool EvaluateRules(List rules, EvaluationContext? context) continue; } + var checkpoint = session.Checkpoint(); var allConditionsMatch = true; foreach (var condition in rule.Conditions) { - if (!EvaluateCondition(condition, context)) + if (!EvaluateCondition(condition, context, session)) { allConditionsMatch = false; break; @@ -306,18 +282,49 @@ private static bool EvaluateRules(List rules, EvaluationContext? context) { return true; } + + session.Rollback(checkpoint); } return false; } - private static bool EvaluateCondition(ConditionConfiguration condition, EvaluationContext? context) + private static bool EvaluateCondition(ConditionConfiguration condition, EvaluationContext? context, DependencyEvaluationSession session) { if (condition.Operator is null) { throw new FormatException("Condition operator can not be null"); } + var hasAttribute = condition.AttributePresent; + var hasFlagEvaluation = condition.FlagEvaluationPresent; + if (hasAttribute == hasFlagEvaluation) + { + throw new FormatException("Condition must contain exactly one of attribute or flagEvaluation"); + } + + if (hasFlagEvaluation) + { + if (condition.Operator is not ConditionOperator.ONE_OF and not ConditionOperator.NOT_ONE_OF) + { + throw new FormatException("Flag evaluation conditions only support ONE_OF and NOT_ONE_OF"); + } + + var descriptor = condition.FlagEvaluation; + if (descriptor is null + || !descriptor.KeyPresent + || descriptor.Key is not string { Length: > 0 } key + || descriptor.AdditionalProperties is { Count: > 0 }) + { + throw new FormatException("Flag evaluation must contain only a non-empty string key"); + } + + var variant = session.Resolve(key); + return condition.Operator == ConditionOperator.ONE_OF + ? IsDependencyVariantOneOf(variant, condition.Value) + : !IsDependencyVariantOneOf(variant, condition.Value); + } + if (condition.Operator == ConditionOperator.IS_NULL) { var value = ResolveAttribute(condition.Attribute, context); @@ -369,7 +376,7 @@ private static bool EvaluateCondition(ConditionConfiguration condition, Evaluati private static bool IsOneOf(object attributeValue, object? conditionValue) { - if (conditionValue is not IEnumerable enumerable) + if (conditionValue is string || conditionValue is not IEnumerable enumerable) { throw new FormatException($"Condition value is not an array {Convert.ToString(conditionValue ?? "")}"); } @@ -385,6 +392,89 @@ private static bool IsOneOf(object attributeValue, object? conditionValue) return false; } + private static bool IsDependencyVariantOneOf(string variant, object? conditionValue) + { + if (conditionValue is string || conditionValue is not IEnumerable enumerable) + { + throw new FormatException($"Condition value is not an array {Convert.ToString(conditionValue ?? "")}"); + } + + var matches = false; + foreach (var value in enumerable) + { + var member = value switch + { + string text => text, + JValue { Type: JTokenType.String, Value: string text } => text, + _ => throw new FormatException("Flag evaluation condition values must be strings"), + }; + + matches |= string.Equals(variant, member, StringComparison.Ordinal); + } + + return matches; + } + + private static int GetMaxDependencyDepth(ServerConfiguration config) + { + if (!config.EvaluatorParamsPresent) + { + return DefaultMaxDependencyDepth; + } + + var evaluatorParams = config.EvaluatorParams; + if (evaluatorParams is null || !evaluatorParams.MaxDependencyDepthPresent) + { + if (evaluatorParams is not null) + { + return DefaultMaxDependencyDepth; + } + + throw new FormatException("evaluatorParams must be an object"); + } + + var configured = evaluatorParams.MaxDependencyDepth; + + if (configured is null or bool or string || configured is not IConvertible) + { + throw new FormatException("maxDependencyDepth must be an integer from 0 through 255"); + } + + try + { + var value = Convert.ToDouble(configured, CultureInfo.InvariantCulture); + if (double.IsNaN(value) || double.IsInfinity(value) || value < 0 || value > 255 || Math.Truncate(value) != value) + { + throw new FormatException("maxDependencyDepth must be an integer from 0 through 255"); + } + + return (int)value; + } + catch (FormatException) + { + throw; + } + catch (Exception ex) + { + throw new FormatException("maxDependencyDepth must be an integer from 0 through 255", ex); + } + } + + private static Evaluation Error(string flagKey, object? defaultValue, string code, string? message, string consent, string? error = null) + { + var metadata = ConsentMetadata(consent); + metadata[MetadataErrorCode] = code; + if (message is not null) + { + metadata["message"] = message; + } + + return new Evaluation(flagKey, defaultValue, EvaluationReason.Error, error: error ?? code, metadata: metadata); + } + + private static Dictionary ConsentMetadata(string consent) + => new() { [FeatureFlagMetadataKeys.ObserveFullEvaluationData] = consent }; + private static bool CompareString(object a, object? b) { if (b is null) @@ -648,62 +738,42 @@ private static double ParseInteger(object value) internal static IDictionary FlattenContext(EvaluationContext? context) { var result = new Dictionary(); - if (context is not null) + if (context is null) { - var seen = new HashSet(); - var keys = context.Attributes.Keys; - foreach (var key in keys) - { - var stack = new Stack(); - stack.Push(new FlattenEntry(key, context.GetAttribute(key), 1)); + return result; + } - while (stack.Count > 0) + // Exposure context is intentionally scalar-only. Bound the work and never inspect a + // collection that will be omitted, because customer collection implementations may be + // arbitrarily large or throw from Count, indexers, or enumeration. + const int maximumAttributes = 128; + try + { + var inspected = 0; + foreach (var pair in context.Attributes) + { + if (inspected++ >= maximumAttributes) { - var entry = stack.Pop(); - var value = entry.Value; - - // For now only return first level entries - if (entry.Level > 1) - { - continue; - } + break; + } - if (value == null || seen.Add(value)) - { - if (value == null) - { - result[entry.Key] = null; - } - else if (value is IList list) - { - for (var i = 0; i < list.Count; i++) - { - stack.Push(new FlattenEntry($"{entry.Key}[{i}]", list[i], entry.Level + 1)); - } - } - else if (value is IDictionary dict) - { - foreach (var pairKey in dict.Keys) - { - stack.Push(new FlattenEntry($"{entry.Key}.{pairKey}", dict[pairKey!], entry.Level + 1)); - } - } - else - { - // Do not overwrite existing keys - if (!result.ContainsKey(entry.Key)) - { - result[entry.Key] = value; - } - } - } + if (pair.Value is IList or IDictionary) + { + continue; } + + result[pair.Key] = pair.Value; } } + catch + { + // Exposure construction is best effort and must not change flag resolution. + } return result; } +#pragma warning restore SA1204 private Evaluation ResolveVariant( string flagKey, ValueType resultType, @@ -714,7 +784,8 @@ private Evaluation ResolveVariant( EvaluationReason reason, DateTime evalTime, EvaluationContext? context, - string consent) + string consent, + DependencyEvaluationSession session) { var variationKey = split.VariationKey!; @@ -763,7 +834,11 @@ private Evaluation ResolveVariant( if (doLog) { - DispatchExposure(flagKey, evaluation, evalTime, context, split.SerialId); + var exposure = CreateExposure(flagKey, evaluation, evalTime, context, split.SerialId); + if (exposure.HasValue) + { + session.RecordExposure(flagKey, exposure.Value); + } } else { @@ -788,7 +863,7 @@ Evaluation ParseError(string error) } } - private void DispatchExposure( + private Exposure.Model.ExposureEvent? CreateExposure( string flagKey, Evaluation evaluation, DateTime evalTime, @@ -801,28 +876,308 @@ private void DispatchExposure( if (allocationKey == null || variantKey == null) { Log.Debug("Feature Flags: exposure not recorded for {FlagKey}: allocation key or variant key is missing", flagKey); - return; + return null; } - var evt = new Exposure.Model.ExposureEvent( + return new Exposure.Model.ExposureEvent( new DateTimeOffset(evalTime).ToUnixTimeMilliseconds(), new Exposure.Model.Allocation(allocationKey), new Exposure.Model.Flag(flagKey), new Exposure.Model.Variant(variantKey), new Exposure.Model.Subject(context?.TargetingKey ?? string.Empty, FlattenContext(context)), serialId); + } - _onExposureEvent?.Invoke(in evt); + private readonly struct ExposureCheckpoint(int exposureCount, int? frameExposureCount) + { + public int ExposureCount { get; } = exposureCount; + + public int? FrameExposureCount { get; } = frameExposureCount; } - private record struct FlattenEntry(string Key, object? Value, int Level) + private readonly struct ExposureCandidate(string flagKey, Exposure.Model.ExposureEvent exposure) { + public string FlagKey { get; } = flagKey; + + public Exposure.Model.ExposureEvent Exposure { get; } = exposure; + } + + private sealed class DependencyEvaluationSession + { + private readonly FeatureFlagsEvaluator _owner; + private readonly ServerConfiguration _config; + private readonly EvaluationContext? _context; + private readonly DateTime _evaluationTime; + private readonly string _consent; + private readonly bool _collectDependencyEvaluations; + private readonly string? _targetingKey; + private readonly bool _includeContext; + private readonly Dictionary _memo = new(); + private readonly HashSet _evaluated = []; + private readonly HashSet _active; + private readonly List _frames = []; + private readonly List _exposures = []; + private readonly List _evaluations = []; + private IReadOnlyDictionary? _contextAttributes; + private bool _contextSnapshotted; + private int? _maxDepth; + + public DependencyEvaluationSession( + FeatureFlagsEvaluator owner, + ServerConfiguration config, + string rootFlagKey, + EvaluationContext? context, + DateTime evaluationTime, + string consent, + bool collectDependencyEvaluations) + { + _owner = owner; + _config = config; + _context = context; + _evaluationTime = evaluationTime; + _consent = consent; + _collectDependencyEvaluations = collectDependencyEvaluations; + _targetingKey = context?.TargetingKey; + _includeContext = collectDependencyEvaluations && consent == "true"; + _active = new HashSet { rootFlagKey }; + } + + public ExposureCheckpoint Checkpoint() + { + var frame = _frames.Count == 0 ? null : _frames[_frames.Count - 1]; + return new ExposureCheckpoint(_exposures.Count, frame?.ExposureTrace.Count); + } + + public void Rollback(ExposureCheckpoint checkpoint) + { + _exposures.RemoveRange(checkpoint.ExposureCount, _exposures.Count - checkpoint.ExposureCount); + if (checkpoint.FrameExposureCount is { } count && _frames.Count > 0) + { + var trace = _frames[_frames.Count - 1].ExposureTrace; + trace.RemoveRange(count, trace.Count - count); + } + } + + public string Resolve(string flagKey) + { + var depth = _frames.Count + 1; + var maxDepth = _maxDepth ??= GetMaxDependencyDepth(_config); + if (depth > maxDepth) + { + if (!_memo.ContainsKey(flagKey) && !_active.Contains(flagKey)) + { + EmitDepthError(flagKey); + } + + throw new DependencyGraphException("GENERAL", "maximum flag dependency depth exceeded"); + } + + if (_active.Contains(flagKey)) + { + throw new DependencyGraphException("GENERAL", "flag dependency cycle detected"); + } + + if (_memo.TryGetValue(flagKey, out var memoized)) + { + if (depth + memoized.DescendantDepth > maxDepth) + { + throw new DependencyGraphException("GENERAL", "maximum flag dependency depth exceeded"); + } + + RecordReachedDepth(depth + memoized.DescendantDepth); + RecordExposureTrace(memoized.ExposureTrace); + return memoized.Evaluation.Variant!; + } + + Flag? flag = null; + var lookup = _owner._config?.Flags?.Find(flagKey, out flag) ?? FlagLookupResult.NotFound; + if (lookup == FlagLookupResult.NotFound) + { + var missing = Error(flagKey, DefaultValue(ValueType.String), "FLAG_NOT_FOUND", "prerequisite flag is missing", _consent); + EmitEvaluation(missing); + throw new DependencyGraphException("FLAG_NOT_FOUND", "prerequisite flag is missing"); + } + + if (lookup == FlagLookupResult.Invalid || flag?.VariationType is null) + { + var invalid = Error(flagKey, null, "PARSE_ERROR", "prerequisite flag is invalid", _consent); + EmitEvaluation(invalid); + throw new DependencyGraphException("PARSE_ERROR", "prerequisite flag is invalid"); + } + + var type = flag.VariationType.Value; + var defaultValue = DefaultValue(type); + _active.Add(flagKey); + var frame = new DependencyFrame(depth); + _frames.Add(frame); + MemoizedDependency? resolved = null; + try + { + var evaluation = _owner.EvaluateFlag(flagKey, type, defaultValue, _context, this, _evaluationTime, _consent); + if (evaluation.Reason == EvaluationReason.Error) + { + throw new DependencyGraphException(ErrorCode(evaluation) ?? "GENERAL", evaluation.Error ?? "prerequisite flag evaluation failed"); + } + + if (evaluation.Variant is null) + { + throw new DependencyGraphException("GENERAL", "prerequisite flag evaluation returned no variant"); + } + + var trace = new List(frame.ExposureTrace); + var result = new MemoizedDependency(evaluation, frame.MaxReachedDepth - depth, trace); + _memo.Add(flagKey, result); + resolved = result; + EmitEvaluation(evaluation); + return evaluation.Variant; + } + catch (DependencyGraphException ex) + { + var failed = Error(flagKey, defaultValue, ex.ErrorCode, ex.Message, _consent); + EmitEvaluation(failed); + throw; + } + catch (MissingTargetingKeyException) + { + const string message = "prerequisite flag evaluation requires a targeting key"; + EmitEvaluation(Error(flagKey, defaultValue, "TARGETING_KEY_MISSING", message, _consent)); + throw new DependencyGraphException("TARGETING_KEY_MISSING", message); + } + catch (FormatException ex) + { + EmitEvaluation(Error(flagKey, defaultValue, "PARSE_ERROR", ex.Message, _consent)); + throw new DependencyGraphException("PARSE_ERROR", ex.Message); + } + catch (Exception) + { + const string message = "prerequisite flag evaluation failed"; + EmitEvaluation(Error(flagKey, defaultValue, "GENERAL", message, _consent)); + throw new DependencyGraphException("GENERAL", message); + } + finally + { + _frames.RemoveAt(_frames.Count - 1); + _active.Remove(flagKey); + if (resolved is not null) + { + RecordReachedDepth(depth + resolved.DescendantDepth); + RecordExposureTrace(resolved.ExposureTrace); + } + } + } + + public void RecordExposure(string flagKey, Exposure.Model.ExposureEvent exposure) + { + var candidate = new ExposureCandidate(flagKey, exposure); + AppendUnique(_exposures, candidate); + if (_frames.Count > 0) + { + AppendUnique(_frames[_frames.Count - 1].ExposureTrace, candidate); + } + } + + public EvaluationTrace Complete(Evaluation details) + { + var exposures = details.Reason == EvaluationReason.Error + ? Array.Empty() + : _exposures.Select(candidate => candidate.Exposure).ToArray(); + return new EvaluationTrace(details, _evaluations.ToArray(), exposures); + } + + private static object DefaultValue(ValueType type) => type switch + { + ValueType.Boolean => false, + ValueType.Integer => 0, + ValueType.Numeric => 0d, + ValueType.String => string.Empty, + ValueType.Json => new Dictionary(), + _ => string.Empty, + }; + + private static void AppendUnique(List target, ExposureCandidate candidate) + { + if (!target.Any(existing => existing.FlagKey == candidate.FlagKey)) + { + target.Add(candidate); + } + } + + private void RecordExposureTrace(List trace) + { + foreach (var candidate in trace) + { + AppendUnique(_exposures, candidate); + if (_frames.Count > 0) + { + AppendUnique(_frames[_frames.Count - 1].ExposureTrace, candidate); + } + } + } + + private void RecordReachedDepth(int depth) + { + if (_frames.Count > 0) + { + var frame = _frames[_frames.Count - 1]; + frame.MaxReachedDepth = Math.Max(frame.MaxReachedDepth, depth); + } + } + + private void EmitEvaluation(Evaluation evaluation) + { + if (!_collectDependencyEvaluations) + { + return; + } + + if (!_evaluated.Add(evaluation.FlagKey)) + { + return; + } + + if (!_contextSnapshotted) + { + _contextAttributes = DependencyEvaluation.SnapshotContext(_context, _includeContext); + _contextSnapshotted = true; + } + + _evaluations.Add(DependencyEvaluation.Snapshot(evaluation, _evaluationTime, _targetingKey, _contextAttributes)); + } + + private void EmitDepthError(string flagKey) + { + Flag? flag = null; + var lookup = _config.Flags?.Find(flagKey, out flag) ?? FlagLookupResult.NotFound; + var defaultValue = lookup == FlagLookupResult.Found && flag?.VariationType is { } type ? DefaultValue(type) : null; + EmitEvaluation(Error(flagKey, defaultValue, "GENERAL", "maximum flag dependency depth exceeded", _consent)); + } + + private sealed class DependencyFrame(int maxReachedDepth) + { + public int MaxReachedDepth { get; set; } = maxReachedDepth; + + public List ExposureTrace { get; } = []; + } + + private sealed class MemoizedDependency(Evaluation evaluation, int descendantDepth, List exposureTrace) + { + public Evaluation Evaluation { get; } = evaluation; + + public int DescendantDepth { get; } = descendantDepth; + + public List ExposureTrace { get; } = exposureTrace; + } } private sealed class MissingTargetingKeyException : Exception { } + private sealed class DependencyGraphException(string errorCode, string message) : Exception(message) + { + public string ErrorCode { get; } = errorCode; + } + #pragma warning disable SA1201 // Elements should appear in the correct order internal delegate void ReportExposureDelegate(in Exposure.Model.ExposureEvent ev); #pragma warning restore SA1201 // Elements should appear in the correct order diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs index 4458b8afa83d..95b04f811279 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs @@ -420,6 +420,9 @@ internal void RegisterOnNewConfigEventHandler(Action? onNewConfig) } internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defaultValue, string? targetingKey, IDictionary? attributes) + => Evaluate(flagKey, resultType, defaultValue, targetingKey, attributes, deferExposures: false); + + internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defaultValue, string? targetingKey, IDictionary? attributes, bool deferExposures) { var evaluator = Volatile.Read(ref _evaluator); if (evaluator is null) @@ -439,11 +442,61 @@ internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defau return notReady; } - var evaluation = evaluator.Evaluate(flagKey, resultType, defaultValue, new EvaluationContext(targetingKey, attributes)); + var collectDependencyEvaluations = Volatile.Read(ref _availableEvaluationWriter) is not null; + var trace = evaluator.EvaluateWithTrace( + flagKey, + resultType, + defaultValue, + new EvaluationContext(targetingKey, attributes), + collectDependencyEvaluations); + foreach (var dependency in trace.DependencyEvaluations) + { + try + { + ReportDependencyEvaluation(dependency); + } + catch + { + // Dependency evaluation telemetry is best effort and must not affect resolution. + } + } + + var evaluation = trace.Details; + if (deferExposures && trace.DependencyExposures.Count > 0) + { + var exposures = trace.DependencyExposures; + var metadata = new ExposureCommitMetadata(evaluation.FlagMetadata, () => ReportExposures(exposures)); + evaluation = new Evaluation( + evaluation.FlagKey, + evaluation.Value, + evaluation.Reason, + evaluation.Variant, + evaluation.Error, + metadata); + } + else + { + ReportExposures(trace.DependencyExposures); + } LogEvaluation(evaluation, resultType, defaultValue, targetingKey, attributes, evaluator.Configuration); return evaluation; + + void ReportExposures(IReadOnlyList exposures) + { + foreach (var exposure in exposures) + { + try + { + ReportExposure(in exposure); + } + catch + { + // Exposure delivery is best effort and must not affect resolution. + } + } + } } [TestingAndPrivateOnly] @@ -880,6 +933,30 @@ private void ReportExposure(in ExposureEvent exposure) } } + private void ReportDependencyEvaluation(DependencyEvaluation dependency) + { + var writer = Volatile.Read(ref _availableEvaluationWriter); + if (writer is null) + { + return; + } + + var evaluation = dependency.Evaluation; + var consent = evaluation.FlagMetadata?.TryGetValue(FeatureFlagMetadataKeys.ObserveFullEvaluationData, out var value) == true + && value == "true"; + var attributes = consent ? dependency.ContextAttributes : null; + writer.TryEnqueue( + new FlagEvalEvent( + evaluation.FlagKey, + evaluation.Variant, + FeatureFlagsEvaluator.AllocationKey(evaluation), + dependency.TargetingKey, + new DateTimeOffset(dependency.EvaluationTime).ToUnixTimeMilliseconds(), + attributes, + FeatureFlagsEvaluator.ErrorCode(evaluation), + consent)); + } + // Created on first use because most applications never evaluate a flag, and under the lock // because the evaluation path races disposal. Only the very first exposure takes the lock: // afterwards the field is read directly, keeping the exposure path lock-free. diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs index dab28af05496..6e3dbeee6cac 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs @@ -15,10 +15,34 @@ namespace Datadog.Trace.FeatureFlags.Rcm.Model; internal sealed class ConditionConfiguration { private Regex? _regex; + private string? _attribute; + private FlagEvaluationConfiguration? _flagEvaluation; public ConditionOperator? Operator { get; set; } - public string? Attribute { get; set; } + public string? Attribute + { + get => _attribute; + set + { + AttributePresent = true; + _attribute = value; + } + } + + public FlagEvaluationConfiguration? FlagEvaluation + { + get => _flagEvaluation; + set + { + FlagEvaluationPresent = true; + _flagEvaluation = value; + } + } + + internal bool AttributePresent { get; private set; } + + internal bool FlagEvaluationPresent { get; private set; } public object? Value { get; set; } diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/EvaluatorParams.cs b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/EvaluatorParams.cs new file mode 100644 index 000000000000..3ebc3309db3a --- /dev/null +++ b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/EvaluatorParams.cs @@ -0,0 +1,25 @@ +// +// 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. +// + +#nullable enable + +namespace Datadog.Trace.FeatureFlags.Rcm.Model; + +internal sealed class EvaluatorParams +{ + private object? _maxDependencyDepth; + + public object? MaxDependencyDepth + { + get => _maxDependencyDepth; + set + { + MaxDependencyDepthPresent = true; + _maxDependencyDepth = value; + } + } + + internal bool MaxDependencyDepthPresent { get; private set; } +} diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagCollectionJsonConverter.cs b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagCollectionJsonConverter.cs index 101818093e56..ad9a76f2bf50 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagCollectionJsonConverter.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagCollectionJsonConverter.cs @@ -6,6 +6,7 @@ #nullable enable using System; +using System.Linq; using Datadog.Trace.Vendors.Newtonsoft.Json; using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; @@ -108,7 +109,7 @@ private static bool IsValid(Flag flag) foreach (var condition in rule.Conditions) { - if (condition is null || !HasValidOperand(condition)) + if (condition is null || !HasValidCondition(condition)) { return false; } @@ -144,11 +145,29 @@ private static bool IsValid(Flag flag) return true; } - private static bool HasValidOperand(ConditionConfiguration condition) + private static bool HasValidCondition(ConditionConfiguration condition) { + if (condition.AttributePresent == condition.FlagEvaluationPresent) + { + return false; + } + + if (condition.FlagEvaluationPresent) + { + var descriptor = condition.FlagEvaluation; + if (descriptor is null + || !descriptor.KeyPresent + || descriptor.Key is not string { Length: > 0 } + || descriptor.AdditionalProperties is { Count: > 0 } + || condition.Operator is not ConditionOperator.ONE_OF and not ConditionOperator.NOT_ONE_OF) + { + return false; + } + } + if (condition.Operator is ConditionOperator.ONE_OF or ConditionOperator.NOT_ONE_OF) { - return condition.Value is JArray; + return condition.Value is JArray values && values.All(value => value.Type == JTokenType.String); } if (condition.Operator is ConditionOperator.SEMVER_EQ diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagEvaluationConfiguration.cs b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagEvaluationConfiguration.cs new file mode 100644 index 000000000000..96345ef7c414 --- /dev/null +++ b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/FlagEvaluationConfiguration.cs @@ -0,0 +1,32 @@ +// +// 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. +// + +#nullable enable + +using System.Collections.Generic; +using Datadog.Trace.Vendors.Newtonsoft.Json; +using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; + +namespace Datadog.Trace.FeatureFlags.Rcm.Model; + +internal sealed class FlagEvaluationConfiguration +{ + private object? _key; + + public object? Key + { + get => _key; + set + { + KeyPresent = true; + _key = value; + } + } + + internal bool KeyPresent { get; private set; } + + [JsonExtensionData] + internal IDictionary? AdditionalProperties { get; set; } +} diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ServerConfiguration.cs b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ServerConfiguration.cs index 06ce821a80d1..4bf438d06e81 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ServerConfiguration.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ServerConfiguration.cs @@ -11,6 +11,8 @@ namespace Datadog.Trace.FeatureFlags.Rcm.Model; internal sealed class ServerConfiguration { + private EvaluatorParams? _evaluatorParams; + [JsonConverter(typeof(StrictBooleanTrueJsonConverter))] public bool ObserveFullEvaluationData { get; set; } @@ -20,6 +22,18 @@ internal sealed class ServerConfiguration public Environment? Environment { get; set; } + public EvaluatorParams? EvaluatorParams + { + get => _evaluatorParams; + set + { + EvaluatorParamsPresent = true; + _evaluatorParams = value; + } + } + + internal bool EvaluatorParamsPresent { get; private set; } + [JsonConverter(typeof(FlagCollectionJsonConverter))] public FlagCollection? Flags { get; set; } @@ -43,6 +57,11 @@ internal void Merge(ServerConfiguration other) Environment = other.Environment; } + if (other.EvaluatorParamsPresent) + { + EvaluatorParams = other.EvaluatorParams; + } + if (Flags is null) { Flags = new FlagCollection(); diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs index 43eec9ea77f5..6de5986aeb31 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs @@ -129,7 +129,7 @@ private static void AssertResolution(JsonLinq.JObject testCase, ResolutionDet actualValue = NormalizeNumbers(actualValue); Assert.True(JsonLinq.JToken.DeepEquals(expectedValue, actualValue), $"Expected {expectedValue}, got {actualValue}"); Assert.Equal(testCase["flag"]!.Value(), actual.FlagKey); - Assert.Equal(expected["reason"]!.Value()!.ToLowerInvariant(), actual.Reason); + Assert.Equal(expected["reason"]!.Value(), actual.Reason); var errorCode = expected["errorCode"]?.Value(); if (errorCode is null && expected["reason"]!.Value() == "ERROR") diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/ExposureCommitHookTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/ExposureCommitHookTests.cs new file mode 100644 index 000000000000..7c1687ec1cf9 --- /dev/null +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/ExposureCommitHookTests.cs @@ -0,0 +1,127 @@ +// +// 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. +// + +#nullable enable + +extern alias Tracer; + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Datadog.Trace.FeatureFlags; +using FluentAssertions; +using OpenFeature; +using OpenFeature.Constant; +using OpenFeature.Model; +using Xunit; +using TracerFlags = Tracer::Datadog.Trace.FeatureFlags; + +namespace Datadog.FeatureFlags.OpenFeature.Tests; + +public class ExposureCommitHookTests +{ + [Fact] + public async Task FeatureClientAfterHookFailureDiscardsPendingExposures() + { + var commits = 0; + var domain = $"exposure-failure-{Guid.NewGuid():N}"; + using var provider = new DatadogProvider((key, type, fallback, targetingKey, attributes) => + new TestEvaluation(key, () => Interlocked.Increment(ref commits))); + await Api.Instance.SetProviderAsync(domain, provider); + var client = Api.Instance.GetClient(domain); + client.AddHooks(new FailingAfterHook()); + + var result = await client.GetBooleanDetailsAsync("root", false); + + result.ErrorType.Should().Be(ErrorType.General); + result.Value.Should().BeFalse(); + Volatile.Read(ref commits).Should().Be(0); + } + + [Fact] + public async Task SuccessfulFeatureClientEvaluationsCommitTheirOwnOrderedBatchOnce() + { + var batches = new ConcurrentQueue(); + var domain = $"exposure-success-{Guid.NewGuid():N}"; + using var provider = new DatadogProvider((key, type, fallback, targetingKey, attributes) => + { + var ownedBatch = new[] { $"{key}-dependency", key }; + return new TestEvaluation(key, () => batches.Enqueue(ownedBatch)); + }); + await Api.Instance.SetProviderAsync(domain, provider); + var client = Api.Instance.GetClient(domain); + + await Task.WhenAll( + client.GetBooleanDetailsAsync("left", false), + client.GetBooleanDetailsAsync("right", false)); + + batches.Should().HaveCount(2); + batches.Should().Contain(batch => batch.SequenceEqual(new[] { "left-dependency", "left" })); + batches.Should().Contain(batch => batch.SequenceEqual(new[] { "right-dependency", "right" })); + } + + [Fact] + public void RunProviderHooksCommitsOnlyTheFinalSuccessfulResult() + { + var committed = 0; + var commitHook = new ExposureCommitHook(); + var context = BoolContext(); + var success = Resolution(); + commitHook.Capture(success.FlagMetadata, () => Interlocked.Increment(ref committed)); + + DatadogProvider.RunProviderHooks(ImmutableList.Create(commitHook), context, success); + DatadogProvider.RunProviderHooks(ImmutableList.Create(commitHook), context, success); + + committed.Should().Be(1, "the invocation owner is one-shot even if the same details are completed twice"); + + var rejected = Resolution(); + commitHook.Capture(rejected.FlagMetadata, () => Interlocked.Increment(ref committed)); + var failed = DatadogProvider.RunProviderHooks( + ImmutableList.Create(commitHook, new FailingAfterHook()), + context, + rejected); + + failed.ErrorType.Should().Be(ErrorType.General); + committed.Should().Be(1, "an After failure must discard the pending batch"); + } + + private static ResolutionDetails Resolution() + => new( + "root", + true, + ErrorType.None, + Reason.Static, + "on", + null, + new ImmutableMetadata()); + + private static HookContext BoolContext() + => new("root", false, FlagValueType.Boolean, new ClientMetadata(null, null), new Metadata("test"), EvaluationContext.Empty); + + private sealed class FailingAfterHook : Hook + { + public override ValueTask AfterAsync(HookContext context, FlagEvaluationDetails details, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + => throw new InvalidOperationException("after failed"); + } + + private sealed class TestEvaluation(string flagKey, Action commit) : IEvaluation + { + public string FlagKey { get; } = flagKey; + + public object Value => true; + + public EvaluationReason Reason => EvaluationReason.Static; + + public string Variant => "on"; + + public string? Error => null; + + public IDictionary FlagMetadata { get; } = new TracerFlags.ExposureCommitMetadata(null, commit); + } +} diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs index dcbc2ac13bf0..c511091177c9 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs @@ -16,6 +16,22 @@ namespace Datadog.FeatureFlags.OpenFeature.Tests; public class FeatureFlagsSdkTests { + [Theory] + [InlineData((int)EvaluationReason.Static, Reason.Static)] + [InlineData((int)EvaluationReason.Default, Reason.Default)] + [InlineData((int)EvaluationReason.TargetingMatch, Reason.TargetingMatch)] + [InlineData((int)EvaluationReason.Split, Reason.Split)] + [InlineData((int)EvaluationReason.Disabled, Reason.Disabled)] + [InlineData((int)EvaluationReason.Cached, Reason.Cached)] + [InlineData((int)EvaluationReason.Unknown, Reason.Unknown)] + [InlineData((int)EvaluationReason.Error, Reason.Error)] + public void ResolutionUsesCanonicalOpenFeatureReason(int evaluationReason, string expected) + { + var result = FeatureFlagsSdk.GetResolutionDetails("flag", false, new TestEvaluation(null, reason: (EvaluationReason)evaluationReason), null); + + result.Reason.Should().Be(expected); + } + [Theory] [InlineData(null, false)] [InlineData("false", false)] @@ -117,13 +133,13 @@ public void DisabledEventsDoNotInventMetadataForAnOldTracer() result.FlagMetadata.Should().BeNull(); } - private sealed class TestEvaluation(IDictionary? metadata, string? error = null) : IEvaluation + private sealed class TestEvaluation(IDictionary? metadata, string? error = null, EvaluationReason? reason = null) : IEvaluation { public string FlagKey => "flag"; public object Value => true; - public EvaluationReason Reason => error is null ? EvaluationReason.Static : EvaluationReason.Error; + public EvaluationReason Reason => reason ?? (error is null ? EvaluationReason.Static : EvaluationReason.Error); public string? Variant => "on"; diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FlagEvalEVPHookTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FlagEvalEVPHookTests.cs index 59b9fac0a200..09a0bf4d8f22 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FlagEvalEVPHookTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FlagEvalEVPHookTests.cs @@ -85,7 +85,7 @@ public async Task EvaluatorErrorMetadataReachesEventsWithoutChangingProviderResu resolution.Value.Should().BeFalse(); resolution.ErrorType.Should().Be(ErrorType.None, "the EVP fix must not change existing provider results"); - resolution.Reason.Should().Be("error"); + resolution.Reason.Should().Be(Reason.Error); resolution.ErrorMessage.Should().Be("private-evaluator-error"); fixture.Enqueues.Should().Be(1); fixture.ErrorCode.Should().Be(code); diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/SynchronousHookTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/SynchronousHookTests.cs index d860fd1da4e2..4da80d81c6e8 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/SynchronousHookTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/SynchronousHookTests.cs @@ -162,6 +162,22 @@ public void SyncHooksRunInReverseOrderForEachStage(ErrorType errorType, string r } #if NET6_0_OR_GREATER + [Theory] + [InlineData(Reason.TargetingMatch, "targeting_match")] + [InlineData("CUSTOM_REASON", "custom_reason")] + public async Task EvaluationMetricKeepsLowerSnakeCaseReasonTag(string reason, string expected) + { + var flagKey = $"reason-{Guid.NewGuid():N}"; + using var metrics = new EvaluationMetrics(flagKey); + using var hook = new FlagEvalMetricsHook(); + var context = new HookContext(flagKey, false, FlagValueType.Boolean, new ClientMetadata(null, null), new Metadata("test"), EvaluationContext.Empty); + var details = new FlagEvaluationDetails(flagKey, true, ErrorType.None, reason, "on"); + + await hook.FinallyAsync(context, details); + + metrics.Reasons.Should().Equal(expected); + } + [Theory] [InlineData("Boolean")] [InlineData("Double")] @@ -225,6 +241,7 @@ private static IEnumerable BuiltInHooks() { yield return new SpanEnrichmentHook(); yield return new FlagEvalEVPHook(); + yield return new ExposureCommitHook(); #if NET6_0_OR_GREATER yield return new FlagEvalMetricsHook(); #endif @@ -235,6 +252,7 @@ private sealed class EvaluationMetrics : IDisposable { private readonly MeterListener _listener = new(); private readonly List _errorTypes = []; + private readonly List _reasons = []; public EvaluationMetrics(string flagKey) { @@ -247,11 +265,12 @@ public EvaluationMetrics(string flagKey) }; _listener.SetMeasurementEventCallback((_, _, tags, _) => { - string? key = null, errorType = null; + string? key = null, errorType = null, reason = null; foreach (var tag in tags) { key = tag.Key == "feature_flag.key" ? tag.Value as string : key; errorType = tag.Key == "error.type" ? tag.Value as string : errorType; + reason = tag.Key == "feature_flag.result.reason" ? tag.Value as string : reason; } if (key == flagKey) @@ -259,6 +278,7 @@ public EvaluationMetrics(string flagKey) lock (_errorTypes) { _errorTypes.Add(errorType); + _reasons.Add(reason); } } }); @@ -276,6 +296,17 @@ public IReadOnlyList ErrorTypes } } + public IReadOnlyList Reasons + { + get + { + lock (_errorTypes) + { + return _reasons.ToArray(); + } + } + } + public void Dispose() => _listener.Dispose(); } #endif diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/AgentlessConfigurationSourceTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/AgentlessConfigurationSourceTests.cs index f3910dd16823..a276d672a085 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/AgentlessConfigurationSourceTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/AgentlessConfigurationSourceTests.cs @@ -38,6 +38,21 @@ public class AgentlessConfigurationSourceTests private const string EndpointUrl = "https://ufc-server.ff-cdn.datadoghq.com/api/v2/feature-flagging/config/rules-based/server"; + [Fact] + public void RequestHeadersAdvertiseDependentFlagSupport() + { + var settings = new TracerSettings(new NameValueConfigurationSource(new NameValueCollection + { + [ConfigurationKeys.ApiKey] = "test-api-key", + })); + var headers = AgentlessConfigurationSource.GetRequestHeaders(CreateEndpoint(), settings.FeatureFlags) + .ToDictionary(pair => pair.Key, pair => pair.Value); + + headers.Should().Contain( + AgentlessConfigurationSource.FeatureFlagsCapabilitiesHeader, + AgentlessConfigurationSource.DependentFlagCapability); + } + [Fact] public async Task AppliesConfigurationFromA200() { diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/ExposureApiTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/ExposureApiTests.cs index b8a1b8b5077e..1bfd49b1a108 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/ExposureApiTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/ExposureApiTests.cs @@ -160,6 +160,45 @@ public async Task SendExposureAfterDisposeIsIgnored() local.RequestsSent.Should().BeEmpty(); } + [Fact] + public async Task QueueOverflowDropsNewExposureWithoutGrowingThePendingBatch() + { + var sendStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseSend = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var logger = CreateDebugLogger(); + var local = new TestRequestFactory( + new Uri("http://agent:8126/"), + uri => new BlockingApiRequest(uri, sendStarted, releaseSend), + uri => new RecordingJsonApiRequest(uri), + uri => new RecordingJsonApiRequest(uri)); + using var transport = CreateLocalTransport(local, logger.Object); + using var api = new ExposureApi( + CreateSettings(), + transport, + TimeSpan.FromHours(1), + TimeSpan.FromSeconds(3), + logger.Object, + queueCapacity: 2, + batchSize: 1); + + api.SendExposure(CreateExposure("in-flight")); + (await Task.WhenAny(sendStarted.Task, Task.Delay(TimeSpan.FromSeconds(3)))) + .Should().BeSameAs(sendStarted.Task); + + api.SendExposure(CreateExposure("queued-1")); + api.SendExposure(CreateExposure("queued-2")); + api.SendExposure(CreateExposure("overflow")); + + ArgumentsOf(logger, "queue is full").Should().Equal("overflow"); + releaseSend.TrySetResult(true); + api.Dispose(); + + local.RequestsSent.Should().HaveCount(3); + local.RequestsSent.Cast().Should().OnlyContain( + request => JObject.Parse(request.PayloadJson)["exposures"]!.Count() == 1, + "each queue drain is bounded by the configured batch size"); + } + [Fact] public void ReportsWhatHappensToEachExposureThroughShutdown() { diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs new file mode 100644 index 000000000000..9b158bdbecb0 --- /dev/null +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs @@ -0,0 +1,380 @@ +// +// 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. +// + +#nullable enable + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Datadog.Trace.FeatureFlags; +using Datadog.Trace.FeatureFlags.Rcm.Model; +using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; +using FluentAssertions; +using Xunit; +using ValueType = Datadog.Trace.FeatureFlags.ValueType; + +namespace Datadog.Trace.Tests.FeatureFlags; + +public class FeatureFlagsDependentFlagTests +{ + [Fact] + public void EvaluatesDependencyAndCommitsOrderedExposureProvenance() + { + var config = Configuration( + StaticFlag("prerequisite", "on"), + DependentFlag( + "root", + Allocation("rejected", [Dependency("prerequisite", "on"), Attribute("plan", "paid")], "selected"), + Allocation("selected", [Dependency("prerequisite", "on")], "selected"))); + var evaluator = Evaluator(config); + + var trace = evaluator.EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject", new Dictionary { ["plan"] = "free" })); + + trace.Details.Value.Should().Be("selected"); + trace.DependencyEvaluations.Select(e => e.Evaluation.FlagKey).Should().Equal("prerequisite"); + trace.DependencyExposures.Select(e => e.Flag.Key).Should().Equal("prerequisite", "root"); + } + + [Fact] + public void RejectedDependencyKeepsTelemetryButRollsBackItsExposure() + { + var root = DependentFlag( + "root", + Allocation("dependency", [Dependency("prerequisite", "required")], "selected"), + Allocation("fallback", null, "selected")); + var evaluator = Evaluator(Configuration(StaticFlag("prerequisite", "other"), root)); + + var trace = evaluator.EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject")); + + trace.DependencyEvaluations.Should().ContainSingle().Which.Evaluation.Variant.Should().Be("other"); + trace.DependencyExposures.Select(e => e.Flag.Key).Should().Equal("root"); + } + + [Fact] + public void LaterConditionRollbackDoesNotLeakDependencyExposure() + { + var root = DependentFlag( + "root", + Allocation("rejected", [Dependency("prerequisite", "on"), Attribute("plan", "paid")], "selected"), + Allocation("fallback", null, "selected")); + var evaluator = Evaluator(Configuration(StaticFlag("prerequisite", "on"), root)); + + var trace = evaluator.EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject", new Dictionary { ["plan"] = "free" })); + + trace.DependencyEvaluations.Should().ContainSingle(); + trace.DependencyExposures.Select(e => e.Flag.Key).Should().Equal("root"); + } + + [Fact] + public void ConditionsShortCircuitBeforeUnreachedDependency() + { + var root = DependentFlag( + "root", + Allocation("rejected", [Attribute("plan", "paid"), Dependency("prerequisite", "on")], "selected"), + Allocation("fallback", null, "selected")); + var evaluator = Evaluator(Configuration(StaticFlag("prerequisite", "on"), root)); + + var trace = evaluator.EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject", new Dictionary { ["plan"] = "free" })); + + trace.DependencyEvaluations.Should().BeEmpty(); + } + + [Fact] + public void NoAssignmentRollsBackBeforeLaterAllocationAndMemoReplay() + { + var noAssignment = Allocation("no-assignment", [Dependency("prerequisite", "on")], "selected"); + noAssignment.Splits![0].Shards = [new Shard { Salt = "none", TotalShards = 1, Ranges = [] }]; + var root = DependentFlag( + "root", + noAssignment, + Allocation("selected", [Dependency("prerequisite", "on")], "selected")); + var evaluator = Evaluator(Configuration(StaticFlag("prerequisite", "on"), root)); + + var trace = evaluator.EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject")); + + trace.DependencyEvaluations.Should().ContainSingle("memoized dependencies emit evaluation telemetry once"); + trace.DependencyExposures.Select(e => e.Flag.Key).Should().Equal("prerequisite", "root"); + } + + [Fact] + public void DependencyFailurePropagatesAndRollsBackRootTransaction() + { + var evaluator = Evaluator( + Configuration(DependentFlag("root", Allocation("root", [Dependency("missing", "on")], "selected")))); + + var trace = evaluator.EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject")); + var result = trace.Details; + + result.Reason.Should().Be(EvaluationReason.Error); + result.Error.Should().Be("FLAG_NOT_FOUND"); + trace.DependencyEvaluations.Should().ContainSingle().Which.Evaluation.FlagMetadata!["errorCode"].Should().Be("FLAG_NOT_FOUND"); + trace.DependencyExposures.Should().BeEmpty(); + } + + [Fact] + public void NoAssignmentDependencyFailsRootAndPreventsFallback() + { + var noAssignment = Allocation("no-assignment", null, "on"); + noAssignment.Splits![0].Shards = [new Shard { Salt = "none", TotalShards = 1, Ranges = [] }]; + var config = Configuration( + DependentFlag("prerequisite", noAssignment), + DependentFlag( + "root", + Allocation("dependent", [Dependency("prerequisite", "on")], "selected"), + Allocation("fallback", null, "fallback"))); + + var trace = Evaluator(config).EvaluateWithTrace("root", ValueType.String, "caller-default", new EvaluationContext("subject")); + + trace.Details.Reason.Should().Be(EvaluationReason.Error); + trace.Details.Value.Should().Be("caller-default"); + trace.DependencyEvaluations.Should().ContainSingle() + .Which.Evaluation.FlagMetadata!["errorCode"].Should().Be("GENERAL"); + trace.DependencyExposures.Should().BeEmpty(); + } + + [Fact] + public void ReturnedBatchOwnsTelemetrySnapshot() + { + var nested = new Dictionary { ["plan"] = "paid" }; + var attributes = new Dictionary { ["nested"] = nested }; + var config = Configuration( + StaticFlag("prerequisite", "on"), + DependentFlag("root", Allocation("root", [Dependency("prerequisite", "on")], "on"))); + config.ObserveFullEvaluationData = true; + + var trace = Evaluator(config).EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject", attributes)); + nested["plan"] = "free"; + attributes["added"] = true; + trace.DependencyEvaluations[0].Evaluation.FlagMetadata!["mutated"] = "yes"; + + var snapshot = trace.DependencyEvaluations[0].ContextAttributes!; + ((Dictionary)snapshot["nested"]!)["plan"].Should().Be("paid"); + snapshot.Should().NotContainKey("added"); + config.Flags!.Find("prerequisite", out var prerequisite).Should().Be(FlagLookupResult.Found); + prerequisite!.Variations!.Values.Should().OnlyContain(variant => variant.Value!.ToString() == "on"); + } + + [Fact] + public void ReturnedBatchBoundsJsonTokenContextSnapshots() + { + var wide = new JArray(Enumerable.Range(0, 2_000).Select(value => new JValue(value))); + var current = new JObject { ["value"] = true }; + for (var depth = 0; depth < 40; depth++) + { + current = new JObject { ["nested"] = current }; + } + + var attributes = new Dictionary { ["deep"] = current, ["wide"] = wide }; + var config = Configuration( + StaticFlag("prerequisite", "on"), + DependentFlag("root", Allocation("root", [Dependency("prerequisite", "on")], "on"))); + config.ObserveFullEvaluationData = true; + + var trace = Evaluator(config).EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject", attributes)); + var snapshot = trace.DependencyEvaluations[0].ContextAttributes!; + + ((object?[])snapshot["wide"]!).Should().HaveCountLessOrEqualTo(1_279); + object? nested = snapshot["deep"]; + for (var depth = 0; depth < 32 && nested is Dictionary dictionary; depth++) + { + nested = dictionary.GetValueOrDefault("nested"); + } + + nested.Should().BeNull(); + } + + [Fact] + public async Task ParallelRootEvaluationsHaveIndependentBatches() + { + var config = Configuration( + StaticFlag("left-parent", "on"), + StaticFlag("right-parent", "on"), + DependentFlag("left", Allocation("left", [Dependency("left-parent", "on")], "on")), + DependentFlag("right", Allocation("right", [Dependency("right-parent", "on")], "on"))); + var evaluator = Evaluator(config); + + var traces = await Task.WhenAll( + Task.Run(() => evaluator.EvaluateWithTrace("left", ValueType.String, "fallback", new EvaluationContext("left-subject"))), + Task.Run(() => evaluator.EvaluateWithTrace("right", ValueType.String, "fallback", new EvaluationContext("right-subject")))); + + traces[0].DependencyEvaluations.Select(record => record.Evaluation.FlagKey).Should().Equal("left-parent"); + traces[1].DependencyEvaluations.Select(record => record.Evaluation.FlagKey).Should().Equal("right-parent"); + traces[0].DependencyEvaluations[0].TargetingKey.Should().Be("left-subject"); + traces[1].DependencyEvaluations[0].TargetingKey.Should().Be("right-subject"); + } + + [Fact] + public void DetectsCyclesAndInclusiveMaximumDepth() + { + var cycle = Configuration( + DependentFlag("a", Allocation("a", [Dependency("b", "on")], "on")), + DependentFlag("b", Allocation("b", [Dependency("a", "on")], "on"))); + var cycleResult = Evaluator(cycle).Evaluate("a", ValueType.String, "fallback", new EvaluationContext("subject")); + + cycleResult.Reason.Should().Be(EvaluationReason.Error); + cycleResult.Error.Should().Be("GENERAL"); + + var depth = Configuration( + DependentFlag("root", Allocation("root", [Dependency("middle", "on")], "on")), + DependentFlag("middle", Allocation("middle", [Dependency("leaf", "on")], "on")), + StaticFlag("leaf", "on")); + depth.EvaluatorParams = new EvaluatorParams { MaxDependencyDepth = 1 }; + var trace = Evaluator(depth).EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject")); + trace.Details.Reason.Should().Be(EvaluationReason.Error); + trace.DependencyEvaluations.Select(e => e.Evaluation.FlagKey).Should().Equal("leaf", "middle"); + trace.DependencyEvaluations.Should().OnlyContain(e => e.Evaluation.FlagMetadata!["errorCode"] == "GENERAL"); + + depth.EvaluatorParams.MaxDependencyDepth = 2; + Evaluator(depth).Evaluate("root", ValueType.String, "fallback", new EvaluationContext("subject")).Value.Should().Be("on"); + } + + [Theory] + [InlineData(null, false)] + [InlineData(0, true)] + [InlineData(255, false)] + [InlineData(-1, true)] + [InlineData(256, true)] + [InlineData(1.5, true)] + public void EvaluatorParamsAreOptionalAndValidated(object? maxDepth, bool expectError) + { + var config = Configuration( + StaticFlag("dependency", "on"), + DependentFlag("root", Allocation("root", [Dependency("dependency", "on")], "on"))); + if (maxDepth is not null) + { + config.EvaluatorParams = new EvaluatorParams { MaxDependencyDepth = maxDepth }; + } + + var result = Evaluator(config).Evaluate("root", ValueType.String, "fallback", new EvaluationContext("subject")); + + (result.Reason == EvaluationReason.Error).Should().Be(expectError); + if (expectError && maxDepth is not 0) + { + result.Error.Should().Be("PARSE_ERROR"); + } + } + + [Fact] + public void UnusedInvalidMaximumDepthDoesNotAffectOrdinaryFlag() + { + var config = Configuration(StaticFlag("root", "on")); + config.EvaluatorParams = new EvaluatorParams { MaxDependencyDepth = "invalid" }; + + var result = Evaluator(config).Evaluate("root", ValueType.String, "fallback", new EvaluationContext("subject")); + + result.Value.Should().Be("on"); + result.Reason.Should().NotBe(EvaluationReason.Error); + } + + [Fact] + public void ExplicitNullMaximumDepthIsIgnoredWhenUnusedAndRejectedWhenUsed() + { + var config = Configuration( + StaticFlag("ordinary", "on"), + StaticFlag("dependency", "on"), + DependentFlag("dependent", Allocation("dependent", [Dependency("dependency", "on")], "on"))); + config.EvaluatorParams = new EvaluatorParams { MaxDependencyDepth = null }; + + Evaluator(config).Evaluate("ordinary", ValueType.String, "fallback", new EvaluationContext("subject")).Value.Should().Be("on"); + var dependent = Evaluator(config).Evaluate("dependent", ValueType.String, "fallback", new EvaluationContext("subject")); + + dependent.Reason.Should().Be(EvaluationReason.Error); + dependent.Error.Should().Be("PARSE_ERROR"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void MemoizedSubtreeDepthFailureEmitsEachActuallyEvaluatedDependencyOnce(bool sharedSubtreeFirst) + { + var rootConditions = sharedSubtreeFirst + ? new List { Dependency("a", "on"), Dependency("b", "on") } + : new List { Dependency("b", "on"), Dependency("a", "on") }; + var config = Configuration( + StaticFlag("leaf", "on"), + DependentFlag("a", Allocation("a", [Dependency("leaf", "on")], "on")), + DependentFlag("b", Allocation("b", [Dependency("a", "on")], "on")), + DependentFlag("root", Allocation("root", rootConditions, "on"))); + config.EvaluatorParams = new EvaluatorParams { MaxDependencyDepth = 2 }; + var trace = Evaluator(config).EvaluateWithTrace("root", ValueType.String, "fallback", new EvaluationContext("subject")); + var result = trace.Details; + + result.Reason.Should().Be(EvaluationReason.Error); + trace.DependencyEvaluations.Select(e => e.Evaluation.FlagKey).Should().Equal("leaf", "a", "b"); + trace.DependencyEvaluations.Select(e => e.Evaluation.FlagKey).Should().OnlyHaveUniqueItems(); + } + + [Theory] + [InlineData("ONE_OF")] + [InlineData("NOT_ONE_OF")] + public void DependencyConditionMembersMustAllBeStrings(string operatorName) + { + var condition = Dependency("dependency", "on"); + condition.Operator = operatorName == "ONE_OF" ? ConditionOperator.ONE_OF : ConditionOperator.NOT_ONE_OF; + condition.Value = new JArray("on", 1); + var config = Configuration( + StaticFlag("dependency", "on"), + DependentFlag("root", Allocation("root", [condition], "on"))); + + var result = Evaluator(config).Evaluate("root", ValueType.String, "fallback", new EvaluationContext("subject")); + + result.Error.Should().Be("PARSE_ERROR"); + } + + [Fact] + public void ConditionSourceMustBeExactlyOneOfAttributeOrFlagEvaluation() + { + var condition = Dependency("dependency", "on"); + condition.Attribute = "plan"; + var config = Configuration( + StaticFlag("dependency", "on"), + DependentFlag("root", Allocation("root", [condition], "on"))); + + var result = Evaluator(config).Evaluate("root", ValueType.String, "fallback", new EvaluationContext("subject")); + + result.Error.Should().Be("PARSE_ERROR"); + } + + private static FeatureFlagsEvaluator Evaluator(ServerConfiguration config) => new(null, config); + + private static ServerConfiguration Configuration(params Flag[] flags) + { + var collection = new FlagCollection(); + foreach (var flag in flags) + { + collection.Add(flag.Key!, flag); + } + + return new ServerConfiguration { Flags = collection }; + } + + private static Flag StaticFlag(string key, string variant) + => DependentFlag(key, Allocation(key + "-allocation", null, variant)); + + private static Flag DependentFlag(string key, params Allocation[] allocations) + => new() + { + Key = key, + Enabled = true, + VariationType = ValueType.String, + Variations = allocations.SelectMany(a => a.Splits!).Select(s => s.VariationKey!).Distinct().ToDictionary(v => v, v => new Variant(v, v)), + Allocations = [.. allocations], + }; + + private static Allocation Allocation(string key, List? conditions, string variant) + => new() + { + Key = key, + DoLog = true, + Rules = conditions is null ? null : [new Rule(conditions)], + Splits = [new Split { VariationKey = variant, Shards = [] }], + }; + + private static ConditionConfiguration Dependency(string key, string variant) + => new() { FlagEvaluation = new FlagEvaluationConfiguration { Key = key }, Operator = ConditionOperator.ONE_OF, Value = new[] { variant } }; + + private static ConditionConfiguration Attribute(string key, string value) + => new() { Attribute = key, Operator = ConditionOperator.ONE_OF, Value = new[] { value } }; +} diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsEvaluatorTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsEvaluatorTests.cs index 964614650846..10a9ea00ae6f 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsEvaluatorTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsEvaluatorTests.cs @@ -6,6 +6,7 @@ #nullable enable using System; +using System.Collections; using System.Collections.Generic; using Datadog.Trace.FeatureFlags; using Datadog.Trace.FeatureFlags.Rcm.Model; @@ -313,6 +314,42 @@ public void EvaluateExposureFlagSendsNoSerialIdWhenTheSplitCarriesNone() events[0].SerialId.Should().BeNull(); } + [Fact] + public void ExposureContextDoesNotInspectDiscardedCollections() + { + var flags = new FlagCollection { ["exposure-flag"] = FeatureFlagsHelpers.CreateExposureFlag() }; + var events = new List(); + var evaluator = new FeatureFlagsEvaluator((in Trace.FeatureFlags.Exposure.Model.ExposureEvent e) => events.Add(e), new ServerConfiguration { Flags = flags }); + var context = new EvaluationContext( + "user-123", + new Dictionary + { + ["safe"] = "value", + ["throwing"] = new ThrowingList(), + }); + + var result = evaluator.Evaluate("exposure-flag", ValueType.String, "default", context); + + result.Value.Should().Be("tracked-value"); + events.Should().ContainSingle(); + events[0].Subject.Attributes.Should().HaveCount(1); + events[0].Subject.Attributes["safe"].Should().Be("value"); + } + + [Fact] + public void ExposureContextBoundsTopLevelAttributes() + { + var attributes = new Dictionary(); + for (var i = 0; i < 1_000; i++) + { + attributes[$"attribute-{i}"] = i; + } + + var flattened = FeatureFlagsEvaluator.FlattenContext(new EvaluationContext("user-123", attributes)); + + flattened.Should().HaveCount(128); + } + // --------------------------------------------------------------------- // ISO 8601 Date Parsing tests // --------------------------------------------------------------------- @@ -624,6 +661,43 @@ private static Flag CreateTimeBasedFlagWithDates(string key, string startAt, str return new Flag { Key = key, Enabled = true, VariationType = ValueType.String, Variations = variants, Allocations = new List { alloc } }; } + + private sealed class ThrowingList : IList + { + public int Count => throw new InvalidOperationException("Count must not be read"); + + public bool IsReadOnly => true; + + public bool IsFixedSize => true; + + public bool IsSynchronized => false; + + public object SyncRoot => this; + + public object? this[int index] + { + get => throw new InvalidOperationException("Indexer must not be read"); + set => throw new NotSupportedException(); + } + + public int Add(object? value) => throw new NotSupportedException(); + + public void Clear() => throw new NotSupportedException(); + + public bool Contains(object? value) => throw new InvalidOperationException("Collection must not be inspected"); + + public int IndexOf(object? value) => throw new InvalidOperationException("Collection must not be inspected"); + + public void Insert(int index, object? value) => throw new NotSupportedException(); + + public void Remove(object? value) => throw new NotSupportedException(); + + public void RemoveAt(int index) => throw new NotSupportedException(); + + public void CopyTo(Array array, int index) => throw new InvalidOperationException("Collection must not be inspected"); + + public IEnumerator GetEnumerator() => throw new InvalidOperationException("Collection must not be enumerated"); + } } #pragma warning restore SA1204 // Static elements should appear before instance elements #pragma warning restore SA1500 // Braces for multi-line statements should not share line diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FlagEvaluationModuleTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FlagEvaluationModuleTests.cs index 5abd729500c2..ef24002456ce 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FlagEvaluationModuleTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FlagEvaluationModuleTests.cs @@ -6,6 +6,7 @@ #nullable enable using System; +using System.Collections; using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Specialized; @@ -51,6 +52,105 @@ public async Task AgentlessActivationAcceptsObservationsBeforeDiscovery() } } + [Fact] + public async Task ReachedDependencyIsImmediatelyEnqueuedThroughModuleWriter() + { + using var agent = MockTracerAgent.Create(_output); + var received = new ConcurrentQueue(); + agent.EventPlatformProxyPayloadReceived += (_, args) => received.Enqueue(args.Value); + var settings = Settings(agent.Port, "remote_config"); + var module = FeatureFlagsModule.Create(settings, new MockRcmSubscriptionManager(), discoveryService: FlagEvaluationTestDiscovery.Available(settings))!; + try + { + module.Activate(); + var dependency = new Flag + { + Key = "dependency", + Enabled = true, + VariationType = FeatureFlagsValueType.String, + Variations = new Dictionary { ["on"] = new("on", "value") }, + Allocations = [new Allocation { Key = "dependency-allocation", DoLog = false, Splits = [new Split { VariationKey = "on", Shards = [] }] }], + }; + var root = new Flag + { + Key = "root", + Enabled = true, + VariationType = FeatureFlagsValueType.String, + Variations = new Dictionary { ["selected"] = new("selected", "value") }, + Allocations = + [ + new Allocation + { + Key = "root-allocation", + DoLog = false, + Rules = [new Rule([new ConditionConfiguration { FlagEvaluation = new FlagEvaluationConfiguration { Key = "dependency" }, Operator = ConditionOperator.ONE_OF, Value = new[] { "on" } }])], + Splits = [new Split { VariationKey = "selected", Shards = [] }], + }, + ], + }; + var flags = new FlagCollection { ["dependency"] = dependency, ["root"] = root }; + module.ApplyConfiguration(new ServerConfiguration { Flags = flags }).Should().BeTrue(); + + module.Evaluate("root", FeatureFlagsValueType.String, "fallback", "subject", null).Variant.Should().Be("selected"); + await module.FlushAsync(); + + received.Should().ContainSingle(); + var observations = JObject.Parse(received.Single().BodyInJson)["flagEvaluations"]!; + observations.Should().ContainSingle(); + observations[0]!["flag"]!["key"]!.Value().Should().Be("dependency"); + } + finally + { + await module.DisposeAsync(); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task DependencyErrorTelemetryCannotBeBrokenByContextEnumeration(bool consent) + { + using var agent = MockTracerAgent.Create(_output); + var received = new ConcurrentQueue(); + agent.EventPlatformProxyPayloadReceived += (_, args) => received.Enqueue(args.Value); + var settings = Settings(agent.Port, "remote_config"); + var module = FeatureFlagsModule.Create(settings, new MockRcmSubscriptionManager(), discoveryService: FlagEvaluationTestDiscovery.Available(settings))!; + try + { + module.Activate(); + var root = new Flag + { + Key = "root", + Enabled = true, + VariationType = FeatureFlagsValueType.String, + Variations = new Dictionary { ["selected"] = new("selected", "value") }, + Allocations = + [ + new Allocation + { + Key = "root-allocation", + DoLog = false, + Rules = [new Rule([new ConditionConfiguration { FlagEvaluation = new FlagEvaluationConfiguration { Key = "missing" }, Operator = ConditionOperator.ONE_OF, Value = new[] { "on" } }])], + Splits = [new Split { VariationKey = "selected", Shards = [] }], + }, + ], + }; + module.ApplyConfiguration(new ServerConfiguration { ObserveFullEvaluationData = consent, Flags = new FlagCollection { ["root"] = root } }).Should().BeTrue(); + + module.Evaluate("root", FeatureFlagsValueType.String, "fallback", "subject", new ThrowOnEnumerationDictionary()).Reason.Should().Be(EvaluationReason.Error); + await module.FlushAsync(); + + var observation = JObject.Parse(received.Should().ContainSingle().Subject.BodyInJson)["flagEvaluations"]!.Should().ContainSingle().Subject; + observation["flag"]!["key"]!.Value().Should().Be("missing"); + observation["error"]!["message"]!.Value().Should().Be("FLAG_NOT_FOUND"); + observation["context"].Should().BeNull(); + } + finally + { + await module.DisposeAsync(); + } + } + [Theory] [InlineData("remote_config", "evp_proxy/v2")] public async Task DiscoveryStartsOneWriterAfterActivation(string source, string endpoint) @@ -502,6 +602,59 @@ private static ExporterSettings UpdateSettings(TracerSettings settings, int port private static FlagEvalEvent Observation() => new("flag", "variant", null, "private-subject", 1790000000000, null); + private sealed class ThrowOnEnumerationDictionary : IDictionary + { + public ICollection Keys => []; + + public ICollection Values => []; + + public int Count => 0; + + public bool IsReadOnly => false; + + public object? this[string key] + { + get => null; + set + { + } + } + + public void Add(string key, object? value) + { + } + + public void Add(KeyValuePair item) + { + } + + public void Clear() + { + } + + public bool Contains(KeyValuePair item) => false; + + public bool ContainsKey(string key) => false; + + public void CopyTo(KeyValuePair[] array, int arrayIndex) + { + } + + public IEnumerator> GetEnumerator() => throw new InvalidOperationException("attributes must not be copied without consent"); + + public bool Remove(string key) => false; + + public bool Remove(KeyValuePair item) => false; + + public bool TryGetValue(string key, out object? value) + { + value = null; + return false; + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + private sealed class FakeDeliverySource : IFeatureFlagsDeliverySource { public int Started { get; private set; } diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/UfcConfigurationParserTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/UfcConfigurationParserTests.cs index ad058384e428..cac4e36ccaff 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/UfcConfigurationParserTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/UfcConfigurationParserTests.cs @@ -134,6 +134,64 @@ public void ParsesFlagsFromEnvelope() configuration!.Flags!["test-flag"].Enabled.Should().BeTrue(); } + [Fact] + public void ParsesOptionalEvaluatorParamsAndFlagEvaluationCondition() + { + var body = """ + { "data": { "type": "universal-flag-configuration", + "attributes": { "format": "SERVER", "createdAt": "2025-01-01T00:00:00Z", + "environment": { "name": "production" }, + "evaluatorParams": {}, + "flags": { "root": { "key": "root", "enabled": true, "variationType": "STRING", + "allocations": [{ "key": "allocation", "rules": [{ "conditions": [{ + "flagEvaluation": { "key": "prerequisite" }, "operator": "ONE_OF", "value": ["on"] + }] }], "splits": [] }] } } } } } + """; + + Parse(body, out var configuration, out var error).Should().BeTrue(); + + error.Should().BeNull(); + configuration!.EvaluatorParams.Should().NotBeNull(); + configuration.EvaluatorParams!.MaxDependencyDepth.Should().BeNull(); + var condition = configuration.Flags!["root"].Allocations![0].Rules![0].Conditions![0]; + condition.Attribute.Should().BeNull(); + condition.FlagEvaluation!.Key.Should().Be("prerequisite"); + } + + [Fact] + public void PreservesExplicitNullMaximumDependencyDepth() + { + var body = ValidEnvelope.Replace("\"flags\": {}", "\"evaluatorParams\": { \"maxDependencyDepth\": null }, \"flags\": {}"); + + Parse(body, out var configuration, out var error).Should().BeTrue(); + + error.Should().BeNull(); + configuration!.EvaluatorParams.Should().NotBeNull(); + configuration.EvaluatorParams!.MaxDependencyDepthPresent.Should().BeTrue(); + configuration.EvaluatorParams.MaxDependencyDepth.Should().BeNull(); + } + + [Theory] + [InlineData("{ \"key\": 123 }")] + [InlineData("{ \"key\": \"dependency\", \"extra\": true }")] + public void RejectsMalformedFlagEvaluationDescriptor(string descriptor) + { + var body = $$""" + { "data": { "type": "universal-flag-configuration", + "attributes": { "format": "SERVER", "createdAt": "2025-01-01T00:00:00Z", + "environment": { "name": "production" }, + "flags": { "root": { "key": "root", "enabled": true, "variationType": "STRING", + "allocations": [{ "key": "allocation", "rules": [{ "conditions": [{ + "flagEvaluation": {{descriptor}}, "operator": "ONE_OF", "value": ["on"] + }] }], "splits": [] }] } } } } } + """; + + Parse(body, out var configuration, out var error).Should().BeTrue(); + + error.Should().BeNull(); + configuration!.Flags.Should().NotContainKey("root"); + } + [Fact] public void AcceptsANumericEnvironmentName() { From 9d64cf85007d516259417f0635bc09acabd9369c Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Tue, 3 Nov 2026 19:11:57 -0600 Subject: [PATCH 2/5] Address dependent flag review feedback Bound and flatten nested dependency context telemetry, preserve condition-source presence, and enforce the context node budget before enumeration. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 --- .../FeatureFlags/DependencyEvaluation.cs | 79 +++++++++++- .../FeatureFlags/FeatureFlagsModule.cs | 2 +- .../Rcm/Model/ConditionConfiguration.cs | 4 + .../FeatureFlagsDependentFlagTests.cs | 114 ++++++++++++++++++ 4 files changed, 197 insertions(+), 2 deletions(-) diff --git a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs index 0db7fab7ad72..e5b23480f195 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs @@ -19,6 +19,8 @@ internal sealed class DependencyEvaluation( string? targetingKey, IReadOnlyDictionary? contextAttributes) { + private const int MaxContextKeyLength = 256; + public Evaluation Evaluation { get; } = evaluation; public DateTime EvaluationTime { get; } = evaluationTime; @@ -109,9 +111,84 @@ void CopyMetadata(string key) } } + internal static IReadOnlyDictionary? FlattenContext(IReadOnlyDictionary? attributes) + { + if (attributes is null) + { + return null; + } + + var flattened = new Dictionary(); + var remainingNodes = 1_280; + try + { + VisitDictionary(attributes, string.Empty, 0); + } + catch + { + // The context was already detached, but telemetry construction must remain best effort. + } + + return flattened; + + void VisitDictionary(IEnumerable> dictionary, string prefix, int depth) + { + var count = 0; + foreach (var pair in dictionary) + { + if (count++ >= 256 || remainingNodes-- <= 0 || flattened.Count >= 256) + { + break; + } + + var separatorLength = prefix.Length == 0 ? 0 : 1; + if (pair.Key.Length > MaxContextKeyLength + || prefix.Length > MaxContextKeyLength - separatorLength - pair.Key.Length) + { + continue; + } + + var key = prefix.Length == 0 ? pair.Key : prefix + "." + pair.Key; + VisitValue(key, pair.Value, depth + 1); + } + } + + void VisitValue(string key, object? value, int depth) + { + if (depth > 4 || flattened.Count >= 256) + { + return; + } + + switch (value) + { + case IReadOnlyDictionary dictionary: + VisitDictionary(dictionary, key, depth); + break; + case object?[] array: + var count = Math.Min(array.Length, 256); + for (var i = 0; i < count && remainingNodes-- > 0 && flattened.Count < 256; i++) + { + var index = i.ToString(CultureInfo.InvariantCulture); + if (key.Length > MaxContextKeyLength - 1 - index.Length) + { + continue; + } + + VisitValue(key + "." + index, array[i], depth + 1); + } + + break; + default: + flattened[key] = value; + break; + } + } + } + private static object? Detach(object? value, int depth, ref int remainingNodes) { - if (depth >= 32 || remainingNodes-- <= 0) + if (remainingNodes-- <= 0 || depth >= 32) { return null; } diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs index 95b04f811279..017f3d93c90f 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs @@ -944,7 +944,7 @@ private void ReportDependencyEvaluation(DependencyEvaluation dependency) var evaluation = dependency.Evaluation; var consent = evaluation.FlagMetadata?.TryGetValue(FeatureFlagMetadataKeys.ObserveFullEvaluationData, out var value) == true && value == "true"; - var attributes = consent ? dependency.ContextAttributes : null; + var attributes = consent ? DependencyEvaluation.FlattenContext(dependency.ContextAttributes) : null; writer.TryEnqueue( new FlagEvalEvent( evaluation.FlagKey, diff --git a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs index 6e3dbeee6cac..4c4b9d48dc50 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/Rcm/Model/ConditionConfiguration.cs @@ -46,6 +46,10 @@ public FlagEvaluationConfiguration? FlagEvaluation public object? Value { get; set; } + public bool ShouldSerializeAttribute() => AttributePresent; + + public bool ShouldSerializeFlagEvaluation() => FlagEvaluationPresent; + internal bool MatchesRegex(object attributeValue) { if (_regex == null) diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs index 9b158bdbecb0..7a5a6be6439f 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs @@ -5,11 +5,15 @@ #nullable enable +using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Text; using System.Threading.Tasks; using Datadog.Trace.FeatureFlags; +using Datadog.Trace.FeatureFlags.FlagEvaluation; using Datadog.Trace.FeatureFlags.Rcm.Model; +using Datadog.Trace.Vendors.Newtonsoft.Json; using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; using FluentAssertions; using Xunit; @@ -185,6 +189,102 @@ public void ReturnedBatchBoundsJsonTokenContextSnapshots() nested.Should().BeNull(); } + [Fact] + public void ContextNodeBudgetIncludesChildrenRejectedAtMaximumDepth() + { + var terminal = new CountingEnumerable(2_000); + object? value = terminal; + for (var depth = 0; depth < 31; depth++) + { + value = new object?[] { value }; + } + + DependencyEvaluation.SnapshotContext( + new EvaluationContext("subject", new Dictionary { ["deep"] = value }), + includeContext: true); + + terminal.Visited.Should().BeLessOrEqualTo(1_280); + } + + [Fact] + public void DependencyContextFlattensNestedFieldsForEvaluationTelemetry() + { + var snapshot = new Dictionary + { + ["user"] = new Dictionary { ["country"] = "US" }, + ["tags"] = new object?[] { "paid", false }, + }; + + var flattened = DependencyEvaluation.FlattenContext(snapshot); + var observation = new FlagEvalEvent("dependency", "on", null, "subject", 1, flattened, observeFullEvaluationData: true); + var aggregator = new FlagEvaluationAggregator(10, 10, 10); + aggregator.Add(observation); + var payload = FlagEvaluationPayload.Encode( + aggregator.Drain(), + new Dictionary(), + flushTimeMs: 1, + payloadLimitBytes: 1_000_000).Payloads.Single(); + var evaluationContext = JObject.Parse(Encoding.UTF8.GetString(payload))["flagEvaluations"]![0]!["context"]!["evaluation"]!; + + observation.ContextAttrs.Should().BeEquivalentTo(new Dictionary + { + ["user.country"] = "US", + ["tags.0"] = "paid", + ["tags.1"] = false, + }); + evaluationContext["user.country"]!.Value().Should().Be("US"); + evaluationContext["tags.0"]!.Value().Should().Be("paid"); + evaluationContext["tags.1"]!.Value().Should().BeFalse(); + } + + [Fact] + public void DependencyContextRejectsOversizedPathsBeforeTraversingChildren() + { + var oversizedKey = new string('x', 257); + var snapshot = new Dictionary + { + [oversizedKey] = Enumerable.Range(0, 256).Cast().ToArray(), + [new string('y', 255)] = new object?[] { "too-long" }, + ["valid"] = "retained", + }; + + DependencyEvaluation.FlattenContext(snapshot).Should().BeEquivalentTo(new Dictionary + { + ["valid"] = "retained", + }); + } + + [Fact] + public void ConditionSerializationPreservesTheSelectedSourcePresence() + { + var attribute = new ConditionConfiguration { Attribute = "plan", Operator = ConditionOperator.ONE_OF, Value = new[] { "paid" } }; + var dependency = Dependency("prerequisite", "on"); + + var attributeJson = JObject.Parse(JsonConvert.SerializeObject(attribute)); + var dependencyJson = JObject.Parse(JsonConvert.SerializeObject(dependency)); + + attributeJson.Should().ContainKey(nameof(ConditionConfiguration.Attribute)); + attributeJson.Should().NotContainKey(nameof(ConditionConfiguration.FlagEvaluation)); + dependencyJson.Should().ContainKey(nameof(ConditionConfiguration.FlagEvaluation)); + dependencyJson.Should().NotContainKey(nameof(ConditionConfiguration.Attribute)); + JsonConvert.DeserializeObject(attributeJson.ToString())!.FlagEvaluationPresent.Should().BeFalse(); + JsonConvert.DeserializeObject(dependencyJson.ToString())!.AttributePresent.Should().BeFalse(); + } + + [Fact] + public void ConditionSerializationRetainsExplicitNullForValidation() + { + var condition = new ConditionConfiguration { Attribute = null, Operator = ConditionOperator.ONE_OF, Value = new[] { "paid" } }; + + var json = JObject.Parse(JsonConvert.SerializeObject(condition)); + var roundTrip = JsonConvert.DeserializeObject(json.ToString())!; + + json.Should().ContainKey(nameof(ConditionConfiguration.Attribute)); + json[nameof(ConditionConfiguration.Attribute)]!.Type.Should().Be(JTokenType.Null); + roundTrip.AttributePresent.Should().BeTrue(); + roundTrip.FlagEvaluationPresent.Should().BeFalse(); + } + [Fact] public async Task ParallelRootEvaluationsHaveIndependentBatches() { @@ -377,4 +477,18 @@ private static ConditionConfiguration Dependency(string key, string variant) private static ConditionConfiguration Attribute(string key, string value) => new() { Attribute = key, Operator = ConditionOperator.ONE_OF, Value = new[] { value } }; + + private sealed class CountingEnumerable(int count) : IEnumerable + { + public int Visited { get; private set; } + + public IEnumerator GetEnumerator() + { + for (var i = 0; i < count; i++) + { + Visited++; + yield return i; + } + } + } } From 1bc9adaa45833f31728aa27c79b0007cc65e0060 Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Tue, 3 Nov 2026 19:59:15 -0600 Subject: [PATCH 3/5] Address dependent flag review findings Environment: Datadog workspace Co-Authored-By: Codex GPT-5 --- .../Sdk/FeatureFlagsSdk.cs | 65 +++++- .../FeatureFlags/DependencyEvaluation.cs | 201 +++++++++++++++--- .../FeatureFlags/FeatureFlagsEvaluator.cs | 99 ++++++--- .../FeatureFlags/FeatureFlagsModule.cs | 6 +- .../CanonicalFixtureTests.cs | 83 ++++++++ .../FeatureFlagsSdkTests.cs | 106 +++++++++ .../FeatureFlagsDependentFlagTests.cs | 92 ++++++++ 7 files changed, 581 insertions(+), 71 deletions(-) diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs index 642f6d99e3d6..059828473ffb 100644 --- a/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs @@ -24,6 +24,10 @@ namespace Datadog.FeatureFlags.OpenFeature; [Browsable(false)] internal static class FeatureFlagsSdk { + private const int MaxContextContainerItems = 256; + private const int MaxContextDepth = 4; + private const int MaxContextNodes = 1_280; + internal delegate IEvaluation? EvaluationCallback(string flagKey, Trace.FeatureFlags.ValueType targetType, object? defaultValue, string? targetingKey, IDictionary? attributes); /// Gets a value indicating whether FeatureFlags framework is available or not @@ -121,17 +125,62 @@ public static ResolutionDetails Resolve(string flagKey, Trace.FeatureFlags return null; } - return context.AsDictionary().Select(p => new KeyValuePair(p.Key, ToObject(p.Value))).ToDictionary(p => p.Key, p => p.Value); + var result = new Dictionary(); + foreach (var pair in context.AsDictionary()) + { + // Targeting must see every top-level attribute. Bound only recursive expansion of each + // nested value; a large structure must not consume the budget of a later scalar. + var remainingNodes = MaxContextNodes; + result[pair.Key] = ToObject(pair.Value, depth: 1, ref remainingNodes); + } + + return result; } - private static object? ToObject(Value value) => value switch + private static object? ToObject(Value value, int depth, ref int remainingNodes) { - null => null, - { IsBoolean: true } => value.AsBoolean, - { IsString: true } => value.AsString, - { IsNumber: true } => value.AsDouble, - _ => value.AsObject, - }; + if (remainingNodes-- <= 0 || depth > MaxContextDepth) + { + return null; + } + + if (value is null) { return null; } + if (value.IsBoolean) { return value.AsBoolean; } + if (value.IsString) { return value.AsString; } + if (value.IsNumber) { return value.AsDouble; } + if (value.IsStructure) { return ConvertStructure(value.AsStructure!.AsDictionary(), depth, ref remainingNodes); } + if (value.IsList) { return ConvertList(value.AsList!, depth, ref remainingNodes); } + return null; + } + + private static Dictionary ConvertStructure(IEnumerable> structure, int depth, ref int remainingNodes) + { + var result = new Dictionary(); + var count = 0; + foreach (var pair in structure) + { + if (count++ >= MaxContextContainerItems || remainingNodes <= 0) + { + break; + } + + result[pair.Key] = ToObject(pair.Value, depth + 1, ref remainingNodes); + } + + return result; + } + + private static object?[] ConvertList(IReadOnlyList list, int depth, ref int remainingNodes) + { + var count = Math.Min(list.Count, MaxContextContainerItems); + var result = new List(count); + for (var i = 0; i < count && remainingNodes > 0; i++) + { + result.Add(ToObject(list[i], depth + 1, ref remainingNodes)); + } + + return result.ToArray(); + } internal static ResolutionDetails GetResolutionDetails(string flagKey, T defaultValue, Datadog.Trace.FeatureFlags.IEvaluation? evaluation, long? evalTimeMs) { diff --git a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs index e5b23480f195..74e01acb9e9a 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs @@ -9,6 +9,7 @@ using System.Collections; using System.Collections.Generic; using System.Globalization; +using Datadog.Trace.FeatureFlags.FlagEvaluation; using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; namespace Datadog.Trace.FeatureFlags; @@ -17,7 +18,8 @@ internal sealed class DependencyEvaluation( Evaluation evaluation, DateTime evaluationTime, string? targetingKey, - IReadOnlyDictionary? contextAttributes) + IReadOnlyDictionary? contextAttributes, + ContextOmissionReason contextOmissions) { private const int MaxContextKeyLength = 256; @@ -29,11 +31,14 @@ internal sealed class DependencyEvaluation( public IReadOnlyDictionary? ContextAttributes { get; } = contextAttributes; + public ContextOmissionReason ContextOmissions { get; } = contextOmissions; + public static DependencyEvaluation Snapshot( Evaluation evaluation, DateTime evaluationTime, string? targetingKey, - IReadOnlyDictionary? contextAttributes) + IReadOnlyDictionary? contextAttributes, + ContextOmissionReason contextOmissions) { try { @@ -45,7 +50,7 @@ public static DependencyEvaluation Snapshot( evaluation.Variant, evaluation.Error, metadata); - return new DependencyEvaluation(detached, evaluationTime, targetingKey, contextAttributes); + return new DependencyEvaluation(detached, evaluationTime, targetingKey, contextAttributes, contextOmissions); } catch { @@ -62,7 +67,7 @@ public static DependencyEvaluation Snapshot( evaluation.Variant, evaluation.Error, metadata); - return new DependencyEvaluation(detached, evaluationTime, targetingKey, null); + return new DependencyEvaluation(detached, evaluationTime, targetingKey, null, contextOmissions | ContextOmissionReason.SnapshotError); void CopyMetadata(string key) { @@ -83,7 +88,11 @@ void CopyMetadata(string key) } public static IReadOnlyDictionary? SnapshotContext(EvaluationContext? context, bool includeContext) + => SnapshotContext(context, includeContext, out _); + + public static IReadOnlyDictionary? SnapshotContext(EvaluationContext? context, bool includeContext, out ContextOmissionReason omissions) { + omissions = ContextOmissionReason.None; if (!includeContext || context is null) { return null; @@ -93,28 +102,49 @@ void CopyMetadata(string key) { var remainingNodes = 1_280; var attributes = new Dictionary(); + var count = 0; foreach (var pair in context.Attributes) { + if (count++ >= 256) + { + omissions |= ContextOmissionReason.MaxContextFields; + break; + } + if (remainingNodes <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; break; } - attributes[pair.Key] = Detach(pair.Value, 0, ref remainingNodes); + if (pair.Key.Length > MaxContextKeyLength) + { + omissions |= ContextOmissionReason.MaxKeyLength; + remainingNodes--; + continue; + } + + attributes[pair.Key] = Detach(pair.Value, 0, ref remainingNodes, ref omissions); } return attributes; } catch { + omissions |= ContextOmissionReason.SnapshotError; return null; } } internal static IReadOnlyDictionary? FlattenContext(IReadOnlyDictionary? attributes) + => FlattenContext(attributes, out _); + + internal static IReadOnlyDictionary? FlattenContext(IReadOnlyDictionary? attributes, out ContextOmissionReason omissions) { + var reasons = ContextOmissionReason.None; if (attributes is null) { + omissions = reasons; return null; } @@ -126,9 +156,11 @@ void CopyMetadata(string key) } catch { + reasons |= ContextOmissionReason.SnapshotError; // The context was already detached, but telemetry construction must remain best effort. } + omissions = reasons; return flattened; void VisitDictionary(IEnumerable> dictionary, string prefix, int depth) @@ -136,8 +168,21 @@ void VisitDictionary(IEnumerable> dictionary, stri var count = 0; foreach (var pair in dictionary) { - if (count++ >= 256 || remainingNodes-- <= 0 || flattened.Count >= 256) + if (count++ >= 256) + { + reasons |= depth == 0 ? ContextOmissionReason.MaxContextFields : ContextOmissionReason.MaxStructureProperties; + break; + } + + if (remainingNodes-- <= 0) + { + reasons |= ContextOmissionReason.MaxVisitedNodes; + break; + } + + if (flattened.Count >= 256) { + reasons |= ContextOmissionReason.MaxContextFields; break; } @@ -145,6 +190,7 @@ void VisitDictionary(IEnumerable> dictionary, stri if (pair.Key.Length > MaxContextKeyLength || prefix.Length > MaxContextKeyLength - separatorLength - pair.Key.Length) { + reasons |= ContextOmissionReason.MaxKeyLength; continue; } @@ -157,6 +203,7 @@ void VisitValue(string key, object? value, int depth) { if (depth > 4 || flattened.Count >= 256) { + reasons |= depth > 4 ? ContextOmissionReason.MaxSnapshotDepth : ContextOmissionReason.MaxContextFields; return; } @@ -166,12 +213,30 @@ void VisitValue(string key, object? value, int depth) VisitDictionary(dictionary, key, depth); break; case object?[] array: + if (array.Length > 256) + { + reasons |= ContextOmissionReason.MaxListElements; + } + var count = Math.Min(array.Length, 256); - for (var i = 0; i < count && remainingNodes-- > 0 && flattened.Count < 256; i++) + for (var i = 0; i < count; i++) { + if (remainingNodes-- <= 0) + { + reasons |= ContextOmissionReason.MaxVisitedNodes; + break; + } + + if (flattened.Count >= 256) + { + reasons |= ContextOmissionReason.MaxContextFields; + break; + } + var index = i.ToString(CultureInfo.InvariantCulture); if (key.Length > MaxContextKeyLength - 1 - index.Length) { + reasons |= ContextOmissionReason.MaxKeyLength; continue; } @@ -186,51 +251,87 @@ void VisitValue(string key, object? value, int depth) } } - private static object? Detach(object? value, int depth, ref int remainingNodes) + private static object? Detach(object? value, int depth, ref int remainingNodes, ref ContextOmissionReason omissions) { - if (remainingNodes-- <= 0 || depth >= 32) + if (remainingNodes-- <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; + return null; + } + + if (depth >= 32) + { + omissions |= ContextOmissionReason.MaxSnapshotDepth; return null; } return value switch { null or string or bool or byte or sbyte or short or ushort or int or uint or long or ulong or float or double or decimal or char or DateTime or DateTimeOffset or Guid => value, - JValue valueToken => Detach(valueToken.Value, depth + 1, ref remainingNodes), - JObject objectToken => DetachObject(objectToken, depth, ref remainingNodes), - JArray arrayToken => DetachArray(arrayToken, depth, ref remainingNodes), - JToken => null, - IDictionary dictionary => DetachDictionary(dictionary, depth, ref remainingNodes), - IDictionary dictionary => DetachDictionary(dictionary, depth, ref remainingNodes), - IEnumerable enumerable => DetachEnumerable(enumerable, depth, ref remainingNodes), - _ => Convert.ToString(value, CultureInfo.InvariantCulture), + JValue valueToken => Detach(valueToken.Value, depth + 1, ref remainingNodes, ref omissions), + JObject objectToken => DetachObject(objectToken, depth, ref remainingNodes, ref omissions), + JArray arrayToken => DetachArray(arrayToken, depth, ref remainingNodes, ref omissions), + JToken => Unsupported(ref omissions), + IDictionary dictionary => DetachDictionary(dictionary, depth, ref remainingNodes, ref omissions), + IDictionary dictionary => DetachDictionary(dictionary, depth, ref remainingNodes, ref omissions), + IEnumerable enumerable => DetachEnumerable(enumerable, depth, ref remainingNodes, ref omissions), + _ => Unsupported(ref omissions), }; } + private static object? Unsupported(ref ContextOmissionReason omissions) + { + omissions |= ContextOmissionReason.UnsupportedValue; + return null; + } + private static Dictionary DetachDictionary( IEnumerable> dictionary, int depth, - ref int remainingNodes) + ref int remainingNodes, + ref ContextOmissionReason omissions) { var detached = new Dictionary(); + var count = 0; foreach (var pair in dictionary) { + if (count++ >= 256) + { + omissions |= ContextOmissionReason.MaxStructureProperties; + break; + } + if (remainingNodes <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; break; } - detached[pair.Key] = Detach(pair.Value, depth + 1, ref remainingNodes); + if (pair.Key.Length > MaxContextKeyLength) + { + omissions |= ContextOmissionReason.MaxKeyLength; + remainingNodes--; + continue; + } + + detached[pair.Key] = Detach(pair.Value, depth + 1, ref remainingNodes, ref omissions); } return detached; } - private static Dictionary DetachDictionary(IDictionary dictionary, int depth, ref int remainingNodes) + private static Dictionary DetachDictionary(IDictionary dictionary, int depth, ref int remainingNodes, ref ContextOmissionReason omissions) { var detached = new Dictionary(); + var count = 0; foreach (var item in dictionary) { + if (count++ >= 256) + { + omissions |= ContextOmissionReason.MaxStructureProperties; + break; + } + if (item is not DictionaryEntry entry) { continue; @@ -238,58 +339,104 @@ void VisitValue(string key, object? value, int depth) if (remainingNodes <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; break; } - detached[Convert.ToString(entry.Key) ?? string.Empty] = Detach(entry.Value, depth + 1, ref remainingNodes); + if (entry.Key is not string key) + { + omissions |= ContextOmissionReason.UnsupportedValue; + remainingNodes--; + continue; + } + + if (key.Length > MaxContextKeyLength) + { + omissions |= ContextOmissionReason.MaxKeyLength; + remainingNodes--; + continue; + } + + detached[key] = Detach(entry.Value, depth + 1, ref remainingNodes, ref omissions); } return detached; } - private static object?[] DetachEnumerable(IEnumerable enumerable, int depth, ref int remainingNodes) + private static object?[] DetachEnumerable(IEnumerable enumerable, int depth, ref int remainingNodes, ref ContextOmissionReason omissions) { var detached = new List(); + var count = 0; foreach (var item in enumerable) { + if (count++ >= 256) + { + omissions |= ContextOmissionReason.MaxListElements; + break; + } + if (remainingNodes <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; break; } - detached.Add(Detach(item, depth + 1, ref remainingNodes)); + detached.Add(Detach(item, depth + 1, ref remainingNodes, ref omissions)); } return detached.ToArray(); } - private static Dictionary DetachObject(JObject value, int depth, ref int remainingNodes) + private static Dictionary DetachObject(JObject value, int depth, ref int remainingNodes, ref ContextOmissionReason omissions) { var detached = new Dictionary(); + var count = 0; foreach (var property in value.Properties()) { + if (count++ >= 256) + { + omissions |= ContextOmissionReason.MaxStructureProperties; + break; + } + if (remainingNodes <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; break; } - detached[property.Name] = Detach(property.Value, depth + 1, ref remainingNodes); + if (property.Name.Length > MaxContextKeyLength) + { + omissions |= ContextOmissionReason.MaxKeyLength; + remainingNodes--; + continue; + } + + detached[property.Name] = Detach(property.Value, depth + 1, ref remainingNodes, ref omissions); } return detached; } - private static object?[] DetachArray(JArray value, int depth, ref int remainingNodes) + private static object?[] DetachArray(JArray value, int depth, ref int remainingNodes, ref ContextOmissionReason omissions) { var detached = new List(); + var count = 0; foreach (var item in value) { + if (count++ >= 256) + { + omissions |= ContextOmissionReason.MaxListElements; + break; + } + if (remainingNodes <= 0) { + omissions |= ContextOmissionReason.MaxVisitedNodes; break; } - detached.Add(Detach(item, depth + 1, ref remainingNodes)); + detached.Add(Detach(item, depth + 1, ref remainingNodes, ref omissions)); } return detached.ToArray(); diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs index 370e71841ae5..6e9f7ab567a0 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsEvaluator.cs @@ -13,6 +13,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.RegularExpressions; +using Datadog.Trace.FeatureFlags.FlagEvaluation; using Datadog.Trace.FeatureFlags.Rcm.Model; using Datadog.Trace.Logging; using Datadog.Trace.SourceGenerators; @@ -910,15 +911,18 @@ private sealed class DependencyEvaluationSession private readonly DateTime _evaluationTime; private readonly string _consent; private readonly bool _collectDependencyEvaluations; + private readonly string _rootFlagKey; private readonly string? _targetingKey; private readonly bool _includeContext; - private readonly Dictionary _memo = new(); - private readonly HashSet _evaluated = []; - private readonly HashSet _active; - private readonly List _frames = []; - private readonly List _exposures = []; - private readonly List _evaluations = []; + private HashSet? _exposureKeys; + private List? _exposures; + private Dictionary? _memo; + private HashSet? _evaluated; + private HashSet? _active; + private List? _frames; + private List? _evaluations; private IReadOnlyDictionary? _contextAttributes; + private ContextOmissionReason _contextOmissions; private bool _contextSnapshotted; private int? _maxDepth; @@ -937,34 +941,41 @@ public DependencyEvaluationSession( _evaluationTime = evaluationTime; _consent = consent; _collectDependencyEvaluations = collectDependencyEvaluations; + _rootFlagKey = rootFlagKey; _targetingKey = context?.TargetingKey; _includeContext = collectDependencyEvaluations && consent == "true"; - _active = new HashSet { rootFlagKey }; } public ExposureCheckpoint Checkpoint() { - var frame = _frames.Count == 0 ? null : _frames[_frames.Count - 1]; - return new ExposureCheckpoint(_exposures.Count, frame?.ExposureTrace.Count); + var frame = _frames is not { Count: > 0 } ? null : _frames[_frames.Count - 1]; + return new ExposureCheckpoint(_exposures?.Count ?? 0, frame?.ExposureTrace.Count); } public void Rollback(ExposureCheckpoint checkpoint) { - _exposures.RemoveRange(checkpoint.ExposureCount, _exposures.Count - checkpoint.ExposureCount); - if (checkpoint.FrameExposureCount is { } count && _frames.Count > 0) + if (_exposures is not null && _exposureKeys is not null) + { + RemoveTail(_exposures, _exposureKeys, checkpoint.ExposureCount); + } + + if (checkpoint.FrameExposureCount is { } count && _frames is { Count: > 0 }) { var trace = _frames[_frames.Count - 1].ExposureTrace; - trace.RemoveRange(count, trace.Count - count); + RemoveTail(trace, _frames[_frames.Count - 1].ExposureKeys, count); } } public string Resolve(string flagKey) { - var depth = _frames.Count + 1; + var memo = _memo ??= new Dictionary(); + var active = _active ??= new HashSet { _rootFlagKey }; + var frames = _frames ??= []; + var depth = frames.Count + 1; var maxDepth = _maxDepth ??= GetMaxDependencyDepth(_config); if (depth > maxDepth) { - if (!_memo.ContainsKey(flagKey) && !_active.Contains(flagKey)) + if (!memo.ContainsKey(flagKey) && !active.Contains(flagKey)) { EmitDepthError(flagKey); } @@ -972,12 +983,12 @@ public string Resolve(string flagKey) throw new DependencyGraphException("GENERAL", "maximum flag dependency depth exceeded"); } - if (_active.Contains(flagKey)) + if (active.Contains(flagKey)) { throw new DependencyGraphException("GENERAL", "flag dependency cycle detected"); } - if (_memo.TryGetValue(flagKey, out var memoized)) + if (memo.TryGetValue(flagKey, out var memoized)) { if (depth + memoized.DescendantDepth > maxDepth) { @@ -1007,9 +1018,9 @@ public string Resolve(string flagKey) var type = flag.VariationType.Value; var defaultValue = DefaultValue(type); - _active.Add(flagKey); + active.Add(flagKey); var frame = new DependencyFrame(depth); - _frames.Add(frame); + frames.Add(frame); MemoizedDependency? resolved = null; try { @@ -1026,7 +1037,7 @@ public string Resolve(string flagKey) var trace = new List(frame.ExposureTrace); var result = new MemoizedDependency(evaluation, frame.MaxReachedDepth - depth, trace); - _memo.Add(flagKey, result); + memo.Add(flagKey, result); resolved = result; EmitEvaluation(evaluation); return evaluation.Variant; @@ -1056,8 +1067,8 @@ public string Resolve(string flagKey) } finally { - _frames.RemoveAt(_frames.Count - 1); - _active.Remove(flagKey); + frames.RemoveAt(frames.Count - 1); + active.Remove(flagKey); if (resolved is not null) { RecordReachedDepth(depth + resolved.DescendantDepth); @@ -1069,10 +1080,13 @@ public string Resolve(string flagKey) public void RecordExposure(string flagKey, Exposure.Model.ExposureEvent exposure) { var candidate = new ExposureCandidate(flagKey, exposure); - AppendUnique(_exposures, candidate); - if (_frames.Count > 0) + _exposures ??= []; + _exposureKeys ??= []; + AppendUnique(_exposures, _exposureKeys, candidate); + if (_frames is { Count: > 0 }) { - AppendUnique(_frames[_frames.Count - 1].ExposureTrace, candidate); + var frame = _frames[_frames.Count - 1]; + AppendUnique(frame.ExposureTrace, frame.ExposureKeys, candidate); } } @@ -1080,8 +1094,8 @@ public EvaluationTrace Complete(Evaluation details) { var exposures = details.Reason == EvaluationReason.Error ? Array.Empty() - : _exposures.Select(candidate => candidate.Exposure).ToArray(); - return new EvaluationTrace(details, _evaluations.ToArray(), exposures); + : _exposures?.Select(candidate => candidate.Exposure).ToArray() ?? []; + return new EvaluationTrace(details, _evaluations?.ToArray() ?? [], exposures); } private static object DefaultValue(ValueType type) => type switch @@ -1094,29 +1108,42 @@ public EvaluationTrace Complete(Evaluation details) _ => string.Empty, }; - private static void AppendUnique(List target, ExposureCandidate candidate) + private static void AppendUnique(List target, HashSet keys, ExposureCandidate candidate) { - if (!target.Any(existing => existing.FlagKey == candidate.FlagKey)) + if (keys.Add(candidate.FlagKey)) { target.Add(candidate); } } + private static void RemoveTail(List target, HashSet keys, int count) + { + for (var i = target.Count - 1; i >= count; i--) + { + keys.Remove(target[i].FlagKey); + } + + target.RemoveRange(count, target.Count - count); + } + private void RecordExposureTrace(List trace) { foreach (var candidate in trace) { - AppendUnique(_exposures, candidate); - if (_frames.Count > 0) + _exposures ??= []; + _exposureKeys ??= []; + AppendUnique(_exposures, _exposureKeys, candidate); + if (_frames is { Count: > 0 }) { - AppendUnique(_frames[_frames.Count - 1].ExposureTrace, candidate); + var frame = _frames[_frames.Count - 1]; + AppendUnique(frame.ExposureTrace, frame.ExposureKeys, candidate); } } } private void RecordReachedDepth(int depth) { - if (_frames.Count > 0) + if (_frames is { Count: > 0 }) { var frame = _frames[_frames.Count - 1]; frame.MaxReachedDepth = Math.Max(frame.MaxReachedDepth, depth); @@ -1130,6 +1157,7 @@ private void EmitEvaluation(Evaluation evaluation) return; } + _evaluated ??= []; if (!_evaluated.Add(evaluation.FlagKey)) { return; @@ -1137,11 +1165,12 @@ private void EmitEvaluation(Evaluation evaluation) if (!_contextSnapshotted) { - _contextAttributes = DependencyEvaluation.SnapshotContext(_context, _includeContext); + _contextAttributes = DependencyEvaluation.SnapshotContext(_context, _includeContext, out _contextOmissions); _contextSnapshotted = true; } - _evaluations.Add(DependencyEvaluation.Snapshot(evaluation, _evaluationTime, _targetingKey, _contextAttributes)); + _evaluations ??= []; + _evaluations.Add(DependencyEvaluation.Snapshot(evaluation, _evaluationTime, _targetingKey, _contextAttributes, _contextOmissions)); } private void EmitDepthError(string flagKey) @@ -1157,6 +1186,8 @@ private sealed class DependencyFrame(int maxReachedDepth) public int MaxReachedDepth { get; set; } = maxReachedDepth; public List ExposureTrace { get; } = []; + + public HashSet ExposureKeys { get; } = []; } private sealed class MemoizedDependency(Evaluation evaluation, int descendantDepth, List exposureTrace) diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs index 017f3d93c90f..4952e570b374 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs @@ -944,7 +944,8 @@ private void ReportDependencyEvaluation(DependencyEvaluation dependency) var evaluation = dependency.Evaluation; var consent = evaluation.FlagMetadata?.TryGetValue(FeatureFlagMetadataKeys.ObserveFullEvaluationData, out var value) == true && value == "true"; - var attributes = consent ? DependencyEvaluation.FlattenContext(dependency.ContextAttributes) : null; + var flattenOmissions = ContextOmissionReason.None; + var attributes = consent ? DependencyEvaluation.FlattenContext(dependency.ContextAttributes, out flattenOmissions) : null; writer.TryEnqueue( new FlagEvalEvent( evaluation.FlagKey, @@ -954,7 +955,8 @@ private void ReportDependencyEvaluation(DependencyEvaluation dependency) new DateTimeOffset(dependency.EvaluationTime).ToUnixTimeMilliseconds(), attributes, FeatureFlagsEvaluator.ErrorCode(evaluation), - consent)); + consent), + (int)(dependency.ContextOmissions | flattenOmissions)); } // Created on first use because most applications never evaluate a flag, and under the lock diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs index 6de5986aeb31..9ad465e6af43 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/CanonicalFixtureTests.cs @@ -121,6 +121,89 @@ public async Task PublicProviderMethodsMatchCanonicalFixtures(string description } } + [Fact] + public async Task ProviderTargetingPreservesScalarsAfterLargeAndBudgetHeavyAttributes() + { + var flags = new TracerModel.FlagCollection + { + ["flag"] = new TracerModel.Flag + { + Key = "flag", + Enabled = true, + VariationType = TracerFlags.ValueType.String, + Variations = new Dictionary + { + ["matched"] = new("matched", "matched"), + ["fallback"] = new("fallback", "fallback"), + }, + Allocations = + [ + new TracerModel.Allocation + { + Key = "targeted", + Rules = + [ + new TracerModel.Rule( + [ + new TracerModel.ConditionConfiguration + { + Attribute = "match", + Operator = TracerModel.ConditionOperator.ONE_OF, + Value = new[] { "paid" }, + }, + ]), + ], + Splits = [new TracerModel.Split { VariationKey = "matched", Shards = [] }], + }, + new TracerModel.Allocation + { + Key = "default", + Splits = [new TracerModel.Split { VariationKey = "fallback", Shards = [] }], + }, + ], + }, + }; + var evaluator = new TracerFlags.FeatureFlagsEvaluator(null, new TracerModel.ServerConfiguration { Flags = flags }); + IDictionary? captured = null; + using var provider = new DatadogProvider((key, type, fallback, targetingKey, attributes) => + { + captured = attributes; + return new EvaluationAdapter(evaluator.Evaluate(key, (TracerFlags.ValueType)type, fallback, new TracerFlags.EvaluationContext(targetingKey, attributes))); + }); + + foreach (var (context, expectedCount) in new[] + { + (CreateContext(includeBudgetHeavyNestedValue: false), 301), + (CreateContext(includeBudgetHeavyNestedValue: true), 302), + }) + { + var resolution = await provider.ResolveStringValueAsync("flag", "caller-default", context); + + Assert.Equal("matched", resolution.Value); + Assert.Equal("paid", captured!["match"]); + Assert.Equal(expectedCount, captured.Count); + } + + static EvaluationContext CreateContext(bool includeBudgetHeavyNestedValue) + { + var builder = EvaluationContext.Builder(); + for (var i = 0; i < 300; i++) + { + builder.Set("field" + i, i); + } + + if (includeBudgetHeavyNestedValue) + { + var branches = Enumerable.Range(0, 256).ToDictionary( + i => i.ToString(), + _ => new Value(Enumerable.Repeat(new Value("item"), 256).ToList())); + builder.Set("nested", new Value(new Structure(branches))); + } + + return builder.Set("match", "paid").Build(); + } + } + private static void AssertResolution(JsonLinq.JObject testCase, ResolutionDetails actual) { var expected = testCase["result"]!; diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs index c511091177c9..43ff524b2e58 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs @@ -7,9 +7,11 @@ using System; using System.Collections.Generic; +using System.Linq; using Datadog.Trace.FeatureFlags; using FluentAssertions; using OpenFeature.Constant; +using OpenFeature.Model; using Xunit; namespace Datadog.FeatureFlags.OpenFeature.Tests; @@ -133,6 +135,110 @@ public void DisabledEventsDoNotInventMetadataForAnOldTracer() result.FlagMetadata.Should().BeNull(); } + [Fact] + public void NestedContextValuesAreNormalizedBeforeTracerEvaluation() + { + IDictionary? captured = null; + var context = global::OpenFeature.Model.EvaluationContext.Builder() + .Set("user", new Value(new Structure(new Dictionary + { + ["country"] = new("US"), + ["tags"] = new(new List { new("paid"), new(false) }), + }))) + .Build(); + + FeatureFlagsSdk.Resolve( + "flag", + Trace.FeatureFlags.ValueType.Boolean, + false, + context, + evaluate: (_, _, _, _, attributes) => + { + captured = attributes; + return new TestEvaluation(null); + }); + + captured.Should().NotBeNull(); + captured!["user"].Should().BeEquivalentTo(new Dictionary + { + ["country"] = "US", + ["tags"] = new object?[] { "paid", false }, + }); + } + + [Fact] + public void NestedContextNormalizationBoundsDepthContainersAndNodes() + { + var deep = new Value("leaf"); + for (var i = 0; i < 10; i++) + { + deep = new Value(new Structure(new Dictionary { ["child"] = deep })); + } + + var wide = new Value(Enumerable.Repeat(new Value("item"), 300).ToList()); + var branches = new Dictionary(); + for (var i = 0; i < 256; i++) + { + branches[i.ToString()] = new Value(Enumerable.Repeat(new Value("item"), 256).ToList()); + } + + var capturedDeep = Capture(deep); + var capturedWide = Capture(wide); + var capturedBranches = Capture(new Value(new Structure(branches))); + + MaximumDepth(capturedDeep["value"]).Should().BeLessOrEqualTo(5, "the root attribute is depth one"); + ((object?[])capturedWide["value"]!).Should().HaveCountLessOrEqualTo(256); + CountNodes(capturedBranches).Should().BeLessOrEqualTo(1_281, "the root dictionary is not an OpenFeature Value node"); + + static IDictionary Capture(Value value) + { + IDictionary? captured = null; + var context = global::OpenFeature.Model.EvaluationContext.Builder().Set("value", value).Build(); + FeatureFlagsSdk.Resolve( + "flag", + Trace.FeatureFlags.ValueType.Boolean, + false, + context, + evaluate: (_, _, _, _, attributes) => + { + captured = attributes; + return new TestEvaluation(null); + }); + + return captured!; + } + + static int CountNodes(object? value) + { + if (value is IDictionary dictionary) + { + return 1 + dictionary.Values.Sum(CountNodes); + } + + if (value is object?[] array) + { + return 1 + array.Sum(CountNodes); + } + + return 1; + } + + static int MaximumDepth(object? value) + { + if (value is IDictionary dictionary && dictionary.Count > 0) + { + return 1 + dictionary.Values.Max(MaximumDepth); + } + + if (value is object?[] array && array.Length > 0) + { + return 1 + array.Max(MaximumDepth); + } + + return 1; + } + } + private sealed class TestEvaluation(IDictionary? metadata, string? error = null, EvaluationReason? reason = null) : IEvaluation { public string FlagKey => "flag"; diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs index 7a5a6be6439f..703251210006 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs @@ -5,6 +5,7 @@ #nullable enable +using System; using System.Collections; using System.Collections.Generic; using System.Linq; @@ -254,6 +255,75 @@ public void DependencyContextRejectsOversizedPathsBeforeTraversingChildren() }); } + [Fact] + public void DependencyContextSkipsOversizedKeysBeforeSnapshottingValuesAndReportsOmissions() + { + var throwing = new ThrowingEnumerable(); + var attributes = new Dictionary + { + [new string('x', 257)] = throwing, + ["valid"] = "retained", + }; + + var snapshot = DependencyEvaluation.SnapshotContext( + new EvaluationContext("subject", attributes), + includeContext: true, + out var snapshotOmissions); + var flattened = DependencyEvaluation.FlattenContext(snapshot, out var flattenOmissions); + + throwing.Enumerated.Should().BeFalse(); + snapshot.Should().BeEquivalentTo(new Dictionary { ["valid"] = "retained" }); + flattened.Should().BeEquivalentTo(snapshot); + (snapshotOmissions | flattenOmissions).Should().HaveFlag(ContextOmissionReason.MaxKeyLength); + } + + [Fact] + public void DependencyContextReportsSnapshotAndFlatteningLimits() + { + var throwingContext = new EvaluationContext( + "subject", + new Dictionary { ["throwing"] = new ThrowingEnumerable() }); + + DependencyEvaluation.SnapshotContext(throwingContext, includeContext: true, out var snapshotOmissions).Should().BeNull(); + snapshotOmissions.Should().HaveFlag(ContextOmissionReason.SnapshotError); + + var oversized = new Dictionary + { + ["items"] = Enumerable.Range(0, 300).Cast().ToArray(), + }; + DependencyEvaluation.FlattenContext(oversized, out var flattenOmissions).Should().HaveCount(256); + flattenOmissions.Should().HaveFlag(ContextOmissionReason.MaxListElements); + } + + [Fact] + public void DependencyContextDoesNotInvokeUnsupportedValuesAndReportsThem() + { + var arbitrary = new ThrowingToString(); + var arbitraryKey = new ThrowingToString(); + var attributes = new Dictionary + { + ["arbitrary"] = arbitrary, + ["token"] = new JConstructor("unsupported", 1), + ["dictionary"] = new Hashtable { [arbitraryKey] = "value" }, + }; + + var snapshot = DependencyEvaluation.SnapshotContext( + new EvaluationContext("subject", attributes), + includeContext: true, + out var omissions); + + arbitrary.Called.Should().BeFalse(); + arbitraryKey.Called.Should().BeFalse(); + snapshot.Should().BeEquivalentTo(new Dictionary + { + ["arbitrary"] = null, + ["token"] = null, + ["dictionary"] = new Dictionary(), + }); + omissions.Should().HaveFlag(ContextOmissionReason.UnsupportedValue); + omissions.Should().NotHaveFlag(ContextOmissionReason.SnapshotError); + } + [Fact] public void ConditionSerializationPreservesTheSelectedSourcePresence() { @@ -491,4 +561,26 @@ public IEnumerator GetEnumerator() } } } + + private sealed class ThrowingEnumerable : IEnumerable + { + public bool Enumerated { get; private set; } + + public IEnumerator GetEnumerator() + { + Enumerated = true; + throw new InvalidOperationException("must not enumerate"); + } + } + + private sealed class ThrowingToString + { + public bool Called { get; private set; } + + public override string ToString() + { + Called = true; + throw new InvalidOperationException("must not stringify"); + } + } } From 12d8d99adf1c7cbc2521f94cc10c0374a2d53f07 Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Tue, 3 Nov 2026 20:26:05 -0600 Subject: [PATCH 4/5] Preserve dependency context normalization metadata Environment: Datadog workspace Co-Authored-By: Codex GPT-5 --- .../Sdk/FeatureFlagsSdk.cs | 58 +++++++++++++++---- .../OpenFeatureSdkEvaluateIntegration.cs | 29 +++++++++- .../FeatureFlags/DependencyEvaluation.cs | 20 ++++++- .../FeatureFlags/EvaluationContext.cs | 7 ++- .../FeatureFlags/FeatureFlagsModule.cs | 4 +- .../FeatureFlagsSdkTests.cs | 17 ++++++ .../FeatureFlagsDependentFlagTests.cs | 18 ++++++ 7 files changed, 133 insertions(+), 20 deletions(-) diff --git a/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs b/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs index 059828473ffb..dff64c1d1454 100644 --- a/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs +++ b/tracer/src/Datadog.FeatureFlags.OpenFeature/Sdk/FeatureFlagsSdk.cs @@ -12,6 +12,7 @@ using System.Threading; using System.Threading.Tasks; using Datadog.Trace.FeatureFlags; +using Datadog.Trace.FeatureFlags.FlagEvaluation; using OpenFeature.Constant; using OpenFeature.Model; @@ -126,57 +127,85 @@ public static ResolutionDetails Resolve(string flagKey, Trace.FeatureFlags } var result = new Dictionary(); + var omissionReasons = ContextOmissionReason.None; foreach (var pair in context.AsDictionary()) { // Targeting must see every top-level attribute. Bound only recursive expansion of each // nested value; a large structure must not consume the budget of a later scalar. var remainingNodes = MaxContextNodes; - result[pair.Key] = ToObject(pair.Value, depth: 1, ref remainingNodes); + result[pair.Key] = ToObject(pair.Value, depth: 1, ref remainingNodes, ref omissionReasons); } - return result; + return new ContextAttributes(result, omissionReasons); } - private static object? ToObject(Value value, int depth, ref int remainingNodes) + private static object? ToObject(Value value, int depth, ref int remainingNodes, ref ContextOmissionReason omissionReasons) { - if (remainingNodes-- <= 0 || depth > MaxContextDepth) + if (remainingNodes-- <= 0) + { + omissionReasons |= ContextOmissionReason.MaxVisitedNodes; + return null; + } + + if (depth > MaxContextDepth) { + omissionReasons |= ContextOmissionReason.MaxSnapshotDepth; return null; } if (value is null) { return null; } + if (value.IsNull) { return null; } if (value.IsBoolean) { return value.AsBoolean; } if (value.IsString) { return value.AsString; } if (value.IsNumber) { return value.AsDouble; } - if (value.IsStructure) { return ConvertStructure(value.AsStructure!.AsDictionary(), depth, ref remainingNodes); } - if (value.IsList) { return ConvertList(value.AsList!, depth, ref remainingNodes); } + if (value.AsDateTime is { } dateTime) { return dateTime; } + if (value.IsStructure) { return ConvertStructure(value.AsStructure!.AsDictionary(), depth, ref remainingNodes, ref omissionReasons); } + if (value.IsList) { return ConvertList(value.AsList!, depth, ref remainingNodes, ref omissionReasons); } + omissionReasons |= ContextOmissionReason.UnsupportedValue; return null; } - private static Dictionary ConvertStructure(IEnumerable> structure, int depth, ref int remainingNodes) + private static Dictionary ConvertStructure(IEnumerable> structure, int depth, ref int remainingNodes, ref ContextOmissionReason omissionReasons) { var result = new Dictionary(); var count = 0; foreach (var pair in structure) { - if (count++ >= MaxContextContainerItems || remainingNodes <= 0) + if (count++ >= MaxContextContainerItems) + { + omissionReasons |= ContextOmissionReason.MaxStructureProperties; + break; + } + + if (remainingNodes <= 0) { + omissionReasons |= ContextOmissionReason.MaxVisitedNodes; break; } - result[pair.Key] = ToObject(pair.Value, depth + 1, ref remainingNodes); + result[pair.Key] = ToObject(pair.Value, depth + 1, ref remainingNodes, ref omissionReasons); } return result; } - private static object?[] ConvertList(IReadOnlyList list, int depth, ref int remainingNodes) + private static object?[] ConvertList(IReadOnlyList list, int depth, ref int remainingNodes, ref ContextOmissionReason omissionReasons) { + if (list.Count > MaxContextContainerItems) + { + omissionReasons |= ContextOmissionReason.MaxListElements; + } + var count = Math.Min(list.Count, MaxContextContainerItems); var result = new List(count); for (var i = 0; i < count && remainingNodes > 0; i++) { - result.Add(ToObject(list[i], depth + 1, ref remainingNodes)); + result.Add(ToObject(list[i], depth + 1, ref remainingNodes, ref omissionReasons)); + } + + if (result.Count < count) + { + omissionReasons |= ContextOmissionReason.MaxVisitedNodes; } return result.ToArray(); @@ -326,4 +355,11 @@ private static Value ConvertArray(object?[] array) { return new Value(array.Select(ConvertObject).ToList()); } + + internal sealed class ContextAttributes(Dictionary attributes, ContextOmissionReason omissionReasons) : Dictionary(attributes) + { + // Read by the tracer integration through duck typing. Keeping this on the dictionary avoids + // changing the instrumented Evaluate signature and preserves compatibility with older tracers. + public int ContextOmissionReasons { get; } = (int)omissionReasons; + } } diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs index 203540260c87..0aafdfd6c245 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/OpenFeature/OpenFeatureSdkEvaluateIntegration.cs @@ -29,9 +29,31 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Op [EditorBrowsable(EditorBrowsableState.Never)] public sealed class OpenFeatureSdkEvaluateIntegration { + private interface IContextAttributesWithOmissions + { + int ContextOmissionReasons { get; } + } + internal static CallTargetState OnMethodBegin(ref string flagKey, FeatureFlags.ValueType targetType, ref object? defaultValue, ref string? targetingKey, ref IDictionary? attributes) { - return new CallTargetState(null, new State(flagKey, targetType, defaultValue, targetingKey, attributes)); + return new CallTargetState(null, new State(flagKey, targetType, defaultValue, targetingKey, attributes, GetContextOmissionReasons(attributes))); + } + + internal static int GetContextOmissionReasons(IDictionary? attributes) + { + try + { + if (attributes is not null && attributes.TryDuckCast(out var contextAttributes)) + { + return contextAttributes.ContextOmissionReasons; + } + } + catch + { + // Context omission telemetry is best effort and must not affect flag resolution. + } + + return 0; } internal static CallTargetReturn OnMethodEnd(TReturn? returnValue, Exception? exception, in CallTargetState state) @@ -55,7 +77,8 @@ internal static CallTargetState OnMethodBegin(ref string flagKey, Featu parameters.DefaultValue, parameters.TargetingKey, parameters.Attributes, - SupportsDeferredExposures.Value); + SupportsDeferredExposures.Value, + parameters.ContextOmissionReasons); return new CallTargetReturn(res.DuckCast()); } @@ -66,7 +89,7 @@ private static class SupportsDeferredExposures public static readonly bool Value = typeof(TReturn).Assembly.GetType("Datadog.FeatureFlags.OpenFeature.ExposureCommitHook", throwOnError: false) is not null; } - private sealed record State(string FlagKey, FeatureFlags.ValueType TargetType, object? DefaultValue, string? TargetingKey, IDictionary? Attributes) + private sealed record State(string FlagKey, FeatureFlags.ValueType TargetType, object? DefaultValue, string? TargetingKey, IDictionary? Attributes, int ContextOmissionReasons) { } } diff --git a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs index 74e01acb9e9a..045535d5844c 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/DependencyEvaluation.cs @@ -100,14 +100,28 @@ void CopyMetadata(string key) try { + omissions = context.ContextOmissions; var remainingNodes = 1_280; var attributes = new Dictionary(); - var count = 0; + const string targetingKey = "targetingKey"; + var attributeCount = context.Attributes.Count - (context.Attributes.ContainsKey(targetingKey) ? 1 : 0); + if (attributeCount > 256) + { + omissions |= ContextOmissionReason.MaxContextFields; + } + + var remainingAttributes = Math.Min(attributeCount, 256); foreach (var pair in context.Attributes) { - if (count++ >= 256) + // OpenFeature exposes the root targeting key in AsDictionary(), but dependency + // telemetry reports it separately and it must not consume the attribute budget. + if (pair.Key == targetingKey) + { + continue; + } + + if (remainingAttributes-- <= 0) { - omissions |= ContextOmissionReason.MaxContextFields; break; } diff --git a/tracer/src/Datadog.Trace/FeatureFlags/EvaluationContext.cs b/tracer/src/Datadog.Trace/FeatureFlags/EvaluationContext.cs index d154f7eb130b..b72e0be2f0a4 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/EvaluationContext.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/EvaluationContext.cs @@ -8,13 +8,15 @@ using System; using System.Collections.Generic; using System.ComponentModel; +using Datadog.Trace.FeatureFlags.FlagEvaluation; namespace Datadog.Trace.FeatureFlags { /// Standard implementation of a EvaluationContext /// Targeting Key /// Context optional attributes - internal sealed class EvaluationContext(string? key, IDictionary? attributes = null) + /// Omissions recorded while adapting the caller's context + internal sealed class EvaluationContext(string? key, IDictionary? attributes = null, ContextOmissionReason contextOmissions = ContextOmissionReason.None) { /// Gets the Context Targeting Key public string? TargetingKey { get; } = key; @@ -22,6 +24,9 @@ internal sealed class EvaluationContext(string? key, IDictionary Gets the Context optional Values public IDictionary Attributes { get; } = attributes ?? new Dictionary(); + /// Gets omissions already recorded while adapting the caller's context. + public ContextOmissionReason ContextOmissions { get; } = contextOmissions; + /// Get the Context value if existent /// Value key /// Returns Context Value or null diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs index 4952e570b374..7b22ae9c2bca 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs @@ -422,7 +422,7 @@ internal void RegisterOnNewConfigEventHandler(Action? onNewConfig) internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defaultValue, string? targetingKey, IDictionary? attributes) => Evaluate(flagKey, resultType, defaultValue, targetingKey, attributes, deferExposures: false); - internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defaultValue, string? targetingKey, IDictionary? attributes, bool deferExposures) + internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defaultValue, string? targetingKey, IDictionary? attributes, bool deferExposures, int contextOmissionReasons = 0) { var evaluator = Volatile.Read(ref _evaluator); if (evaluator is null) @@ -447,7 +447,7 @@ internal Evaluation Evaluate(string flagKey, ValueType resultType, object? defau flagKey, resultType, defaultValue, - new EvaluationContext(targetingKey, attributes), + new EvaluationContext(targetingKey, attributes, (ContextOmissionReason)contextOmissionReasons), collectDependencyEvaluations); foreach (var dependency in trace.DependencyEvaluations) { diff --git a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs index 43ff524b2e58..93cf809b2386 100644 --- a/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs +++ b/tracer/test/Datadog.FeatureFlags.OpenFeature.Tests/FeatureFlagsSdkTests.cs @@ -5,10 +5,13 @@ #nullable enable +extern alias Tracer; + using System; using System.Collections.Generic; using System.Linq; using Datadog.Trace.FeatureFlags; +using Datadog.Trace.FeatureFlags.FlagEvaluation; using FluentAssertions; using OpenFeature.Constant; using OpenFeature.Model; @@ -139,7 +142,9 @@ public void DisabledEventsDoNotInventMetadataForAnOldTracer() public void NestedContextValuesAreNormalizedBeforeTracerEvaluation() { IDictionary? captured = null; + var createdAt = new DateTime(2026, 11, 4, 1, 2, 3, DateTimeKind.Utc); var context = global::OpenFeature.Model.EvaluationContext.Builder() + .Set("createdAt", new Value(createdAt)) .Set("user", new Value(new Structure(new Dictionary { ["country"] = new("US"), @@ -159,6 +164,7 @@ public void NestedContextValuesAreNormalizedBeforeTracerEvaluation() }); captured.Should().NotBeNull(); + captured!["createdAt"].Should().Be(createdAt); captured!["user"].Should().BeEquivalentTo(new Dictionary { ["country"] = "US", @@ -189,6 +195,9 @@ public void NestedContextNormalizationBoundsDepthContainersAndNodes() MaximumDepth(capturedDeep["value"]).Should().BeLessOrEqualTo(5, "the root attribute is depth one"); ((object?[])capturedWide["value"]!).Should().HaveCountLessOrEqualTo(256); CountNodes(capturedBranches).Should().BeLessOrEqualTo(1_281, "the root dictionary is not an OpenFeature Value node"); + OmissionReasons(capturedDeep).Should().HaveFlag(ContextOmissionReason.MaxSnapshotDepth); + OmissionReasons(capturedWide).Should().HaveFlag(ContextOmissionReason.MaxListElements); + OmissionReasons(capturedBranches).Should().HaveFlag(ContextOmissionReason.MaxVisitedNodes); static IDictionary Capture(Value value) { @@ -208,6 +217,14 @@ public void NestedContextNormalizationBoundsDepthContainersAndNodes() return captured!; } + static ContextOmissionReason OmissionReasons(IDictionary attributes) + { + var sdkReasons = ((FeatureFlagsSdk.ContextAttributes)attributes).ContextOmissionReasons; + var tracerReasons = Tracer::Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.OpenFeature.OpenFeatureSdkEvaluateIntegration.GetContextOmissionReasons(attributes); + tracerReasons.Should().Be(sdkReasons, "the profiler bridge must preserve SDK normalization omissions"); + return (ContextOmissionReason)sdkReasons; + } + static int CountNodes(object? value) { if (value is IDictionary dictionary) diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs index 703251210006..43a740f328e0 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsDependentFlagTests.cs @@ -161,6 +161,24 @@ public void ReturnedBatchOwnsTelemetrySnapshot() prerequisite!.Variations!.Values.Should().OnlyContain(variant => variant.Value!.ToString() == "on"); } + [Fact] + public void DependencyTelemetryPreservesSdkOmissionsAndDoesNotCountRootTargetingKey() + { + var attributes = Enumerable.Range(0, 256).ToDictionary(i => "field" + i, i => (object?)i); + attributes["targetingKey"] = "subject"; + var config = Configuration( + StaticFlag("prerequisite", "on"), + DependentFlag("root", Allocation("root", [Dependency("prerequisite", "on")], "on"))); + config.ObserveFullEvaluationData = true; + + var context = new EvaluationContext("subject", attributes, ContextOmissionReason.MaxListElements); + var trace = Evaluator(config).EvaluateWithTrace("root", ValueType.String, "fallback", context); + var dependency = trace.DependencyEvaluations.Should().ContainSingle().Subject; + + dependency.ContextAttributes.Should().HaveCount(256).And.NotContainKey("targetingKey"); + dependency.ContextOmissions.Should().Be(ContextOmissionReason.MaxListElements); + } + [Fact] public void ReturnedBatchBoundsJsonTokenContextSnapshots() { From 7a3dea53e48d491bd1f79599acb51a393bb79e75 Mon Sep 17 00:00:00 2001 From: Helen Han Date: Wed, 4 Nov 2026 14:00:12 -0500 Subject: [PATCH 5/5] Advertise dependent flag support to Remote Configuration Advertise bit 50 with FFE_FLAGS on the agent-based configuration path. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 --- .../FeatureFlags/FeatureFlagsModule.cs | 1 + .../RcmCapabilitiesIndices.cs | 2 ++ .../FeatureFlags/FeatureFlagsModuleTests.cs | 16 ++++++++++++++++ 3 files changed, 19 insertions(+) diff --git a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs index 7b22ae9c2bca..fee4c868e6e2 100644 --- a/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs +++ b/tracer/src/Datadog.Trace/FeatureFlags/FeatureFlagsModule.cs @@ -857,6 +857,7 @@ private void SubscribeToRemoteConfiguration() _rcmSubscription = new Subscription(ffeProduct.UpdateFromRcm, RcmProducts.FfeFlags); _rcmSubscriptionManager.SubscribeToChanges(_rcmSubscription); _rcmSubscriptionManager.SetCapability(RcmCapabilitiesIndices.FfeFlagConfigurationRules, true); + _rcmSubscriptionManager.SetCapability(RcmCapabilitiesIndices.FfeFlagDependentFlagVariantKeyV1, true); _deliveryStarted = true; _log.Debug("FeatureFlagsModule::SubscribeToRemoteConfiguration -> Starting remote_config configuration source (Agent Remote Configuration)"); } diff --git a/tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapabilitiesIndices.cs b/tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapabilitiesIndices.cs index 74da98c0d098..622502cbb89e 100644 --- a/tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapabilitiesIndices.cs +++ b/tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapabilitiesIndices.cs @@ -103,6 +103,8 @@ internal static class RcmCapabilitiesIndices public static readonly BigInteger FfeFlagConfigurationRules = Create(46); + public static readonly BigInteger FfeFlagDependentFlagVariantKeyV1 = Create(50); + private static BigInteger Create(int index) => new(1UL << index); } } diff --git a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsModuleTests.cs b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsModuleTests.cs index 11477fc54b77..9d3fd9728cfe 100644 --- a/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsModuleTests.cs +++ b/tracer/test/Datadog.Trace.Tests/FeatureFlags/FeatureFlagsModuleTests.cs @@ -13,6 +13,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Numerics; using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -360,6 +361,21 @@ public void Create_WithRemoteConfigSource_SubscribesImmediately() rcmManager.ProductKeys.Should().Contain(RcmProducts.FfeFlags); } + [Theory] + [InlineData("remote_config", true)] + [InlineData("agentless", false)] + public void Create_AdvertisesDependentFlagsOnlyForRemoteConfig(string source, bool advertises) + { + var rcmManager = new Mock(); + var settings = CreateSettings((ConfigurationKeys.FeatureFlags.FeatureFlagsConfigurationSource, source)); + + using var module = CreateModule(settings, rcmManager.Object); + + var times = advertises ? Times.Once() : Times.Never(); + rcmManager.Verify(manager => manager.SetCapability(new BigInteger(1UL << 46), true), times); + rcmManager.Verify(manager => manager.SetCapability(new BigInteger(1UL << 50), true), times); + } + [Fact] public void Create_WhenDisabled_ReturnsNull() {