[Debugger Default-On] DEBUG-4406 Support multi-config merging with priorities #7536

dudikeleti · 2025-09-18T18:56:41Z

Summary of changes

Implements priority-based merging for APM_TRACING config with determenistic order:
Priority Order: Service+Env (4) > Service (3) > Env (2) > Wildcard (1) > Org (0)

Reason for change

When multiple configs are sent, the "last one wins." This causes non-deterministic behavior across tracers and inconsistent feature enablement. internal RFC

Test coverage

ApmTracingConfigMergerTests.cs

dd-trace-dotnet-ci-bot · 2025-09-18T19:49:40Z

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

Welch test with statistical test for significance of 5%
Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (72ms)  : 71, 73
     .   : milestone, 72,
    master - mean (72ms)  : 71, 73
     .   : milestone, 72,

    section Baseline
    This PR (7536) - mean (68ms)  : 67, 70
     .   : milestone, 68,
    master - mean (69ms)  : 64, 73
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (1,045ms)  : 1003, 1086
     .   : milestone, 1045,
    master - mean (1,048ms)  : 1015, 1081
     .   : milestone, 1048,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (106ms)  : 105, 108
     .   : milestone, 106,
    master - mean (107ms)  : 105, 108
     .   : milestone, 107,

    section Baseline
    This PR (7536) - mean (106ms)  : 103, 109
     .   : milestone, 106,
    master - mean (106ms)  : 104, 108
     .   : milestone, 106,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (741ms)  : 720, 762
     .   : milestone, 741,
    master - mean (748ms)  : 724, 773
     .   : milestone, 748,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (94ms)  : 93, 95
     .   : milestone, 94,
    master - mean (94ms)  : 93, 95
     .   : milestone, 94,

    section Baseline
    This PR (7536) - mean (94ms)  : 91, 96
     .   : milestone, 94,
    master - mean (94ms)  : 91, 96
     .   : milestone, 94,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (704ms)  : 685, 724
     .   : milestone, 704,
    master - mean (708ms)  : 684, 733
     .   : milestone, 708,

gantt
    title Execution time (ms) FakeDbCommand (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (92ms)  : 91, 94
     .   : milestone, 92,
    master - mean (93ms)  : 92, 94
     .   : milestone, 93,

    section Baseline
    This PR (7536) - mean (92ms)  : 89, 95
     .   : milestone, 92,
    master - mean (92ms)  : 90, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (659ms)  : 646, 672
     .   : milestone, 659,
    master - mean (664ms)  : 648, 680
     .   : milestone, 664,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (200ms)  : 198, 203
     .   : milestone, 200,
    master - mean (201ms)  : 197, 206
     .   : milestone, 201,

    section Baseline
    This PR (7536) - mean (197ms)  : 193, 202
     .   : milestone, 197,
    master - mean (197ms)  : 192, 201
     .   : milestone, 197,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (1,171ms)  : 1125, 1217
     .   : milestone, 1171,
    master - mean (1,186ms)  : 1102, 1271
     .   : milestone, 1186,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (283ms)  : 277, 289
     .   : milestone, 283,
    master - mean (288ms)  : 279, 298
     .   : milestone, 288,

    section Baseline
    This PR (7536) - mean (282ms)  : 275, 289
     .   : milestone, 282,
    master - mean (288ms)  : 277, 298
     .   : milestone, 288,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (957ms)  : 915, 999
     .   : milestone, 957,
    master - mean (961ms)  : 914, 1008
     .   : milestone, 961,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (276ms)  : 271, 282
     .   : milestone, 276,
    master - mean (278ms)  : 273, 284
     .   : milestone, 278,

    section Baseline
    This PR (7536) - mean (276ms)  : 269, 283
     .   : milestone, 276,
    master - mean (280ms)  : 270, 289
     .   : milestone, 280,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (937ms)  : 895, 979
     .   : milestone, 937,
    master - mean (947ms)  : 901, 993
     .   : milestone, 947,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7536) - mean (274ms)  : 269, 279
     .   : milestone, 274,
    master - mean (278ms)  : 270, 285
     .   : milestone, 278,

    section Baseline
    This PR (7536) - mean (274ms)  : 268, 281
     .   : milestone, 274,
    master - mean (276ms)  : 269, 283
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (7536) - mean (861ms)  : 839, 883
     .   : milestone, 861,
    master - mean (871ms)  : 837, 905
     .   : milestone, 871,

anna-git · 2025-09-24T10:46:38Z

tracer/src/Datadog.Trace/RemoteConfigurationManagement/RcmCapabilitiesIndices.cs


        public static readonly BigInteger AsmTraceTaggingRules = Create(43);

+        public static readonly BigInteger CapabilityAsmExtendedDataCollection = Create(44);


how is this one related with APM_TRACING?

It's not. I had to do this to make test pass. The bit number is taken from another tracer implementation. If this is wrong, I'll handle it differently.

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

anna-git · 2025-09-24T10:59:22Z

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

+            var higherPriority = this.Priority >= other.Priority ? this : other;
+            var lowerPriority = this.Priority >= other.Priority ? other : this;


nit

Suggested change

var higherPriority = this.Priority >= other.Priority ? this : other;

var lowerPriority = this.Priority >= other.Priority ? other : this;

var (higherPriority, lowerPriority) = this.Priority >= other.Priority ? (this, other) : (other, this);

I like it too but does not work in .net461 (am I missing something?)

I think this syntax uses ValueTuple which was added in net47.

anna-git · 2025-09-24T11:10:00Z

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

+        {
+            // Get current service/environment for filtering
+            var currentSettings = Tracer.Instance.Settings;
+            var serviceName = currentSettings.ServiceName ?? "unknown";


do we want the DefaultServiceName here or the provided service name through DD_SERVICE? cc @andrewlock

anna-git · 2025-09-24T11:45:01Z

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


-                return result;
-            }
+            var mergedConfigJson = ApmTracingConfigMerger.MergeConfigurations(


in MergeConfigurations we deserialize and reserialize something that's gonna be deserialized again within DynamicConfigConfigurationSource. Could we not integrate the merge logic inside DynamicConfigConfiguratioSource directly? like around here

dd-trace-dotnet/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/DynamicConfigConfigurationSource.cs

Line 43 in 7e24fb2

if (jobject != null)

this way we could deserialize once and treat the configurations directly

Please see updated solution 66af9cf
I Can integrate it into DynamicConfigConfigurationSource but I'm afraid that would be too much for configuratioin sousrce. Anyway let me know WDYT

bouwkast

There are discrepancies between the implementations

Can we get those looked at (or maybe I'm missing something here as I'm not the best at going through other code)?

bouwkast · 2025-09-24T13:36:04Z

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

+                TracingEnabled = higher.TracingEnabled ?? lower.TracingEnabled,
+                LogInjectionEnabled = higher.LogInjectionEnabled ?? lower.LogInjectionEnabled,
+                TracingSamplingRate = higher.TracingSamplingRate ?? lower.TracingSamplingRate,
+                TracingSamplingRules = higher.TracingSamplingRules ?? lower.TracingSamplingRules,
+                TracingHeaderTags = higher.TracingHeaderTags ?? lower.TracingHeaderTags,
+                TracingTags = higher.TracingTags ?? lower.TracingTags,


It isn't clear to me why we don't also merge the other properties: DebugEnabled, RuntimeMetricsEnabled, ServiceMapping, DataStreamsEnabled, SpanSamplingRules

The Java implementation merges all of them from what I can tell https://github.com/DataDog/dd-trace-java/blob/274d1448c251f13a88b72e7438dfb8a2c8474213/dd-trace-core/src/main/java/datadog/trace/core/TracingConfigPoller.java#L415

Good catch. I've added those properties later and forgot to update that code. Fixed in 20fe7257ed99147aea95c407163a544803573a81

bouwkast · 2025-09-24T13:51:33Z

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

+            applicableConfigs.Sort((a, b) =>
+            {
+                var priorityComparison = b.Priority.CompareTo(a.Priority); // Descending
+                return priorityComparison != 0 ? priorityComparison : string.Compare(a.ConfigId, b.ConfigId, StringComparison.Ordinal); // Ascending
+            });
+
+            var mergedConfig = applicableConfigs.Aggregate((current, next) => current.MergeWith(next));


I find this is a bit difficult to follow in my opinion and would be beneficial to just show that it is expected to do in a comment

Fixed in d50b9de

bouwkast · 2025-09-24T13:56:33Z

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

+                    var configData = ParseConfiguration(config.Path.Id, jsonContent);
+
+                    // Filter immediately during parsing
+                    if (configData?.Matches(serviceName, environment) == true)


I don't see this in the Java implementation? DataDog/dd-trace-java#9360

I do see it in the Python implementation- https://github.com/DataDog/dd-trace-py/pull/14364/files

Maybe I'm not seeing that logic in the Java implementation

I changed it to the current behaviour (as Java does) until we have an agreement on what what we should do. Fixed in 1599052

bouwkast · 2025-09-24T14:00:49Z

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

+                return new ConfigurationTarget(hasService, hasEnv, hasCluster) switch
+                {
+                    (true, true, _) => 5, // Service+env (highest priority)
+                    (true, false, _) => 4, // Service only
+                    (false, true, _) => 3, // Env only
+                    (false, false, true) => 2, // Cluster
+                    (false, false, false) => 1 // Org level
+                };


It appears the Python implementation has more granularity than this

https://github.com/DataDog/dd-trace-py/blob/8ed7a3e93e4b892566b1c80f44346665890681a4/ddtrace/internal/remoteconfig/products/apm_tracing.py#L34-L40

# Precedence ordering goes from most specific to least specific: # 1. Service, 2. Env, 3. Cluster target, 4. Wildcard `*` return ( ((service is not None and service != "*") << 2) | ((env is not None and env != "*") << 1) | (cluster_target is not None) << 0 )

It seems they are taking into account each possible combination, should we do that too?

Sure, thanks for pointing out. Fixed in a6023fe

…dd more tests

…igurationSource

dudikeleti force-pushed the dudik/multi-config branch from 690a914 to 416e8ac Compare September 20, 2025 00:43

This comment has been minimized.

Sign in to view

dudikeleti marked this pull request as ready for review September 22, 2025 12:34

dudikeleti requested a review from a team as a code owner September 22, 2025 12:34

dudikeleti requested review from anna-git and removed request for anna-git September 23, 2025 14:06

dudikeleti force-pushed the dudik/multi-config branch from 19c9898 to 10a9ba8 Compare September 23, 2025 18:09

dudikeleti changed the title ~~[ER Default-On] DEBUG-4406 Support multi config mering with priorities~~ [Debugger Default-On] DEBUG-4406 Support multi config mering with priorities Sep 24, 2025

anna-git reviewed Sep 24, 2025

View reviewed changes

tracer/src/Datadog.Trace/Configuration/DynamicConfigurationManager.cs Outdated Show resolved Hide resolved

anna-git reviewed Sep 24, 2025

View reviewed changes

bouwkast requested changes Sep 24, 2025

View reviewed changes

lucaspimentel changed the title ~~[Debugger Default-On] DEBUG-4406 Support multi config mering with priorities~~ [Debugger Default-On] DEBUG-4406 Support multi config merging with priorities Sep 24, 2025

lucaspimentel changed the title ~~[Debugger Default-On] DEBUG-4406 Support multi config merging with priorities~~ [Debugger Default-On] DEBUG-4406 Support multi-config merging with priorities Sep 24, 2025

github-actions bot added the area:debugger label Sep 24, 2025

dudikeleti added 12 commits September 24, 2025 21:25

support multi config with priorities

5811676

update capabilities indices

885837b

handle configuration removal, add cluster, fix multi config history a…

60fd117

…dd more tests

add missing capability

8a643d4

Fix DynamicConfigurationTests

354fe88

Fix UpdateConfiguration test

69d4a02

Add the rest of LibConfig properties to the merge logic

799cb55

Handle more priorities combinations

bc75850

Do not filter un-matched service name or environment name

e7ebaad

make the sorting more understandable and add early exit

1f2e185

Fix comment

9ba8b1b

Add a way for providing jtoken instead of string to DynamicConfigConf…

66af9cf

…igurationSource

Update tests

aaca066

dudikeleti force-pushed the dudik/multi-config branch from c459981 to aaca066 Compare September 24, 2025 19:26


		public static readonly BigInteger AsmTraceTaggingRules = Create(43);

		public static readonly BigInteger CapabilityAsmExtendedDataCollection = Create(44);

		var higherPriority = this.Priority >= other.Priority ? this : other;
		var lowerPriority = this.Priority >= other.Priority ? other : this;

	var higherPriority = this.Priority >= other.Priority ? this : other;
	var lowerPriority = this.Priority >= other.Priority ? other : this;
	var (higherPriority, lowerPriority) = this.Priority >= other.Priority ? (this, other) : (other, this);

[Debugger Default-On] DEBUG-4406 Support multi-config merging with priorities #7536

Are you sure you want to change the base?

[Debugger Default-On] DEBUG-4406 Support multi-config merging with priorities #7536

Uh oh!

Conversation

dudikeleti commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Test coverage

Uh oh!

dd-trace-dotnet-ci-bot bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Execution-Time Benchmarks Report ⏱️

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

dudikeleti Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anna-git Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bouwkast left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dudikeleti Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dudikeleti commented Sep 18, 2025 •

edited

Loading

dd-trace-dotnet-ci-bot bot commented Sep 18, 2025 •

edited

Loading

dudikeleti Sep 24, 2025 •

edited

Loading

anna-git Sep 24, 2025 •

edited

Loading

dudikeleti Sep 24, 2025 •

edited

Loading