[Tracing] fix precedence of remote vs local sampling rules (#5720)

* merge settings

* switch between local vs remote schemas

* update config log

* update tests

* clarify comment

* assert remote over local precedence

* use TracerSettings.Create() helper

* rename things

tracer/src/Datadog.Trace/Configuration/ImmutableTracerSettings.cs

@@ -38,6 +38,7 @@ public partial record ImmutableTracerSettings
  private readonly IReadOnlyDictionary<string, string> _globalTags;
  private readonly double? _globalSamplingRate;
  private readonly bool _runtimeMetricsEnabled;
+ private readonly string? _customSamplingRules;
 
  /// <summary>
  /// Initializes a new instance of the <see cref="ImmutableTracerSettings"/> class
@@ -98,7 +99,7 @@ internal ImmutableTracerSettings(TracerSettings settings, bool unusedParamNotToU
  AnalyticsEnabledInternal = settings.AnalyticsEnabledInternal;
 #pragma warning restore 618
  MaxTracesSubmittedPerSecondInternal = settings.MaxTracesSubmittedPerSecondInternal;
- CustomSamplingRulesInternal = settings.CustomSamplingRulesInternal;
+ _customSamplingRules = settings.CustomSamplingRulesInternal;
  CustomSamplingRulesFormat = settings.CustomSamplingRulesFormat;
  SpanSamplingRules = settings.SpanSamplingRules;
  _globalSamplingRate = settings.GlobalSamplingRateInternal;
@@ -298,14 +299,9 @@ internal ImmutableTracerSettings(TracerSettings settings, bool unusedParamNotToU
  /// </summary>
  /// <seealso cref="ConfigurationKeys.CustomSamplingRules"/>
  [GeneratePublicApi(PublicApiUsage.ImmutableTracerSettings_CustomSamplingRules_Get)]
- internal string? CustomSamplingRulesInternal { get; }
+ internal string? CustomSamplingRulesInternal => DynamicSettings.SamplingRules ?? _customSamplingRules;
 
- /// <summary>
- /// Gets the trace sampling rules from remote config.
- /// These contain custom remote rules and dynamic (aka adaptive) rules.
- /// They will be merged with local sampling rules.
- /// </summary>
- internal string? RemoteSamplingRules => DynamicSettings.SamplingRules;
+ internal bool CustomSamplingRulesIsRemote => DynamicSettings.SamplingRules != null;
 
  /// <summary>
  /// Gets a value indicating the format for custom sampling rules ("regex" or "glob").

tracer/src/Datadog.Trace/TracerManager.cs

@@ -407,9 +407,6 @@ void WriteDictionary(IReadOnlyDictionary<string, string> dictionary)
  writer.WritePropertyName("sampling_rules");
  writer.WriteValue(instanceSettings.CustomSamplingRulesInternal);
 
- writer.WritePropertyName("remote_sampling_rules");
- writer.WriteRawValue(instanceSettings.RemoteSamplingRules);
-
  writer.WritePropertyName("tags");
  WriteDictionary(instanceSettings.GlobalTagsInternal);
 

tracer/src/Datadog.Trace/TracerManagerFactory.cs

@@ -262,40 +262,40 @@ protected virtual ITraceSampler GetSampler(ImmutableTracerSettings settings)
 
  // Note: the order that rules are registered is important, as they are evaluated in order.
  // The first rule that matches will be used to determine the sampling rate.
-
- // Remote and local "custom" sampling rules:
- // Unlike most settings, local custom sampling rules configuration (DD_TRACE_SAMPLING_RULES) is not
- // simply overriden with the remote configuration. Instead, remote rules are merged with local rules,
- // with remote rules taking precedence.
-
  var sampler = new TraceSampler(new TracerRateLimiter(settings.MaxTracesSubmittedPerSecondInternal));
 
- // remote sampling rules
- var remoteSamplingRulesJson = settings.RemoteSamplingRules;
+ // sampling rules (remote value overrides local value)
+ var samplingRulesJson = settings.CustomSamplingRulesInternal;
 
- if (!string.IsNullOrWhiteSpace(remoteSamplingRulesJson))
+ // check if the rules are remote or local because they have different JSON schemas
+ if (settings.CustomSamplingRulesIsRemote)
  {
- var remoteSamplingRules =
- RemoteCustomSamplingRule.BuildFromConfigurationString(
- remoteSamplingRulesJson,
- RegexBuilder.DefaultTimeout);
+ // remote sampling rules
+ if (!string.IsNullOrWhiteSpace(samplingRulesJson))
+ {
+ var remoteSamplingRules =
+ RemoteCustomSamplingRule.BuildFromConfigurationString(
+ samplingRulesJson,
+ RegexBuilder.DefaultTimeout);
 
- sampler.RegisterRules(remoteSamplingRules);
+ sampler.RegisterRules(remoteSamplingRules);
+ }
  }
-
- // local sampling rules
- var patternFormatIsValid = SamplingRulesFormat.IsValid(settings.CustomSamplingRulesFormat, out var samplingRulesFormat);
- var localSamplingRulesJson = settings.CustomSamplingRulesInternal;
-
- if (patternFormatIsValid && !string.IsNullOrWhiteSpace(localSamplingRulesJson))
+ else
  {
- var localSamplingRules =
- LocalCustomSamplingRule.BuildFromConfigurationString(
- localSamplingRulesJson,
- samplingRulesFormat,
- RegexBuilder.DefaultTimeout);
+ // local sampling rules
+ var patternFormatIsValid = SamplingRulesFormat.IsValid(settings.CustomSamplingRulesFormat, out var samplingRulesFormat);
 
- sampler.RegisterRules(localSamplingRules);
+ if (patternFormatIsValid && !string.IsNullOrWhiteSpace(samplingRulesJson))
+ {
+ var localSamplingRules =
+ LocalCustomSamplingRule.BuildFromConfigurationString(
+ samplingRulesJson,
+ samplingRulesFormat,
+ RegexBuilder.DefaultTimeout);
+
+ sampler.RegisterRules(localSamplingRules);
+ }
  }
 
  // global sampling rate (remote value overrides local value)

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/DynamicConfigurationTests.cs

@@ -213,7 +213,7 @@ static string FlattenJsonArray(JToken json)
  // json["debug"]?.Value<bool>().Should().Be(expectedConfig.DebugLogsEnabled);
  json["log_injection_enabled"]?.Value<bool>().Should().Be(expectedConfig.LogInjectionEnabled);
  json["sample_rate"]?.Value<double?>().Should().Be(expectedConfig.TraceSampleRate);
- JsonConfigurationSource.JTokenToString(json["remote_sampling_rules"]).Should().Be(expectedConfig.TraceSamplingRules);
+ JsonConfigurationSource.JTokenToString(json["sampling_rules"]).Should().Be(expectedConfig.TraceSamplingRules);
  // json["span_sampling_rules"]?.Value<string>().Should().Be(expectedConfig.SpanSamplingRules);
  // json["data_streams_enabled"]?.Value<bool>().Should().Be(expectedConfig.DataStreamsEnabled);
  FlattenJsonArray(json["header_tags"]).Should().Be(expectedConfig.TraceHeaderTags ?? string.Empty);

tracer/test/Datadog.Trace.Tests/Configuration/DynamicConfigurationTests.cs

@@ -4,8 +4,8 @@
 // </copyright>
 
 using System;
+using System.Collections.Generic;
 using System.IO;
-using System.Threading;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.Configuration.ConfigurationSources;
 using Datadog.Trace.Configuration.Telemetry;
@@ -110,28 +110,58 @@ public void EnableTracing()
  [Fact]
  public void SetSamplingRules()
  {
- var tracerSettings = new TracerSettings();
+ // start with local sampling rules only
+ var localSamplingRulesConfig = new[]
+ {
+ new { sample_rate = 0.5, service = "Service3", resource = "Resource3", },
+ };
+
+ var localSamplingRulesJson = JsonConvert.SerializeObject(localSamplingRulesConfig);
+
+ var tracerSettings = TracerSettings.Create(new()
+ {
+ { "DD_TRACE_SAMPLING_RULES", localSamplingRulesJson }
+ });
+
  TracerManager.ReplaceGlobalManager(new ImmutableTracerSettings(tracerSettings), TracerManagerFactory.Instance);
 
- // sampling rules is null by default
- TracerManager.Instance.Settings.RemoteSamplingRules.Should().BeNull();
+ TracerManager.Instance.Settings.CustomSamplingRulesInternal.Should().Be(localSamplingRulesJson);
+ TracerManager.Instance.Settings.CustomSamplingRulesIsRemote.Should().BeFalse();
 
- var singleAgentRuleOnly = ((TraceSampler)TracerManager.Instance.PerTraceSettings.TraceSampler)!.GetRules();
- singleAgentRuleOnly.Should().ContainSingle().And.AllBeOfType<AgentSamplingRule>();
+ var rules = ((TraceSampler)TracerManager.Instance.PerTraceSettings.TraceSampler)!.GetRules();
 
- var samplingRulesConfig = new[]
+ rules.Should()
+ .BeEquivalentTo(
+ new ISamplingRule[]
+ {
+ new LocalCustomSamplingRule(
+ rate: 0.1f,
+ serviceNamePattern: "Service3",
+ operationNamePattern: null,
+ resourceNamePattern: "Resource3",
+ tagPatterns: null,
+ timeout: TimeSpan.FromSeconds(1),
+ patternFormat: "glob"),
+ new AgentSamplingRule()
+ });
+
+ // set sampling rules "remotely"
+ var remoteSamplingRulesConfig = new[]
  {
  new { sample_rate = 0.5, provenance = "customer", service = "Service1", resource = "Resource1", },
  new { sample_rate = 0.1, provenance = "dynamic", service = "Service2", resource = "Resource2", }
  };
 
- var samplingRulesJson = JsonConvert.SerializeObject(samplingRulesConfig);
+ var configBuilder = CreateConfig(("tracing_sampling_rules", remoteSamplingRulesConfig));
+ DynamicConfigurationManager.OnlyForTests_ApplyConfiguration(configBuilder);
 
- // set sampling rules "remotely"
- DynamicConfigurationManager.OnlyForTests_ApplyConfiguration(CreateConfig(("tracing_sampling_rules", samplingRulesConfig)));
- TracerManager.Instance.Settings.RemoteSamplingRules.Should().Be(samplingRulesJson);
- var rules = ((TraceSampler)TracerManager.Instance.PerTraceSettings.TraceSampler)!.GetRules();
+ var remoteSamplingRulesJson = JsonConvert.SerializeObject(remoteSamplingRulesConfig);
+ TracerManager.Instance.Settings.CustomSamplingRulesInternal.Should().Be(remoteSamplingRulesJson);
+ TracerManager.Instance.Settings.CustomSamplingRulesIsRemote.Should().BeTrue();
+
+ rules = ((TraceSampler)TracerManager.Instance.PerTraceSettings.TraceSampler)!.GetRules();
 
+ // new list should include the remote rules, not the local rules
  rules.Should()
  .BeEquivalentTo(
  new ISamplingRule[]