Update config-in-code manual instrumentation to use an `IConfigurationSource` #6397

andrewlock · 2024-12-04T09:23:16Z

Change how we "apply" settings from manual configuration to the automatic side to use IConfigurationSource

Reason for change

The previous design of how we "apply" settings made in code using manual instrumentation required mutating a TracerSettings object after it was already built. In fact, this is pretty much the only place that we mutate the settings.

By switching to using a "configuration source" approach, so that the settings are built once with the correct values opens up the option to make these immutable (and therefore delete all of the Immutable* settings we currently have. This reduces both code duplication and work we do on startup, and opens the path to further refactoring improvements.

Note that the public API does not change, so consumers of Datadog.Trace.Manual are still working with a mutable object initially.

Implementation details

Currently we pass a Dictionary<string, object> between the manual and automatic side. Previously, we then iterated the keys, compared against known values, and modified TracerSettings as required.

With the changes, we have a ManualInstrumentationConfigurationSource which just "wraps" the Dictionary<>, and returns the correct value as required for a given key. Overall, I think this is much cleaner.

Where things get messy is how we handle disabling specific integrations. The existing dictionary is optimised for looping through the provided values, fetching the setting that needs to be modified, and changing all the required properties. Unfortunately, the IConfigurationSource approach where we're looking up by a key like DD_TRACE_NPGSQL_ENABLED works horribly for this pattern 🙁. So I introduced an additional approach which explicitly additionally transfers the settings using these values, making them just "standard" lookups.

Note that due to backwards + forwards compatibility requirements

We still need to send the "old" version of integration settings from the manual side, in case it's used with an old version of the auto instrumentation

We still need to handle the "old" version of integration settings in the auto side, in case it's used with an old version of the manual instrumentation.

At least in this case we can use the more efficient IConfigurationSource reader, so we don't pay the expense of retrieving the settings. The only downside is a couple of extra allocations when they do disable integrations in code.

Minor other changes:

Add helper ctor to CompositeConfigurationSource for creating the internal list from a collection of IConfigurationSource
Tweak DictionaryObjectConfigurationSource so we can derive from it
Create a separate integration for <3.7.0 manual instrumentation that uses the legacy settings, otherwise use the new settings objects

Test coverage

Mostly covered by existing unit tests (and indirectly by integration tests). Tweaked the test to test both the new and legacy configuration source.

Other details

Requires #6393 to fix how we read integration settings

Part of stack

andrewlock · 2024-12-04T10:13:45Z

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.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6397) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (70ms)  : 64, 76
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6397) - mean (977ms)  : 947, 1008
     .   : milestone, 977,
    master - mean (983ms)  : 951, 1016
     .   : milestone, 983,

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

    section CallTarget+Inlining+NGEN
    This PR (6397) - mean (675ms)  : 661, 688
     .   : milestone, 675,
    master - mean (682ms)  : 665, 699
     .   : milestone, 682,

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

    section CallTarget+Inlining+NGEN
    This PR (6397) - mean (628ms)  : 612, 644
     .   : milestone, 628,
    master - mean (633ms)  : 613, 654
     .   : milestone, 633,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6397) - mean (190ms)  : 186, 194
     .   : milestone, 190,
    master - mean (190ms)  : 186, 194
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6397) - mean (1,090ms)  : 1059, 1122
     .   : milestone, 1090,
    master - mean (1,090ms)  : 1065, 1115
     .   : milestone, 1090,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6397) - mean (276ms)  : 269, 282
     .   : milestone, 276,
    master - mean (276ms)  : 271, 282
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (6397) - mean (865ms)  : 839, 892
     .   : milestone, 865,
    master - mean (867ms)  : 833, 901
     .   : milestone, 867,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6397) - mean (265ms)  : 260, 270
     .   : milestone, 265,
    master - mean (265ms)  : 261, 270
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6397) - mean (844ms)  : 814, 874
     .   : milestone, 844,
    master - mean (849ms)  : 811, 887
     .   : milestone, 849,

andrewlock · 2024-12-04T10:19:03Z

Benchmarks Report for appsec 🐌

Benchmarks for #6397 compared to master:

All benchmarks have the same speed
1 benchmarks have fewer allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.Asm.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	215μs	109ns	406ns	2.9	0.108	205.48 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	314μs	102ns	381ns	2.83	0	212.92 KB
master	`AllCycleSimpleBody`	net472	281μs	147ns	551ns	38.1	2.94	240.1 KB
master	`AllCycleMoreComplexBody`	net6.0	221μs	95.1ns	356ns	2.86	0	208.98 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	326μs	124ns	478ns	2.96	0	216.34 KB
master	`AllCycleMoreComplexBody`	net472	286μs	677ns	2.62μs	38.6	2.85	243.61 KB
master	`ObjectExtractorSimpleBody`	net6.0	137ns	0.0673ns	0.252ns	0.00394	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	225ns	0.121ns	0.452ns	0.00374	0	272 B
master	`ObjectExtractorSimpleBody`	net472	211ns	0.187ns	0.724ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	3.04μs	1.1ns	3.82ns	0.0535	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.77μs	1.81ns	6.52ns	0.0509	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	4.28μs	3.94ns	15.3ns	0.602	0.00641	3.8 KB
#6397	`AllCycleSimpleBody`	net6.0	218μs	336ns	1.3μs	2.82	0	205.48 KB
#6397	`AllCycleSimpleBody`	netcoreapp3.1	318μs	161ns	601ns	2.88	0	212.92 KB
#6397	`AllCycleSimpleBody`	net472	285μs	299ns	1.12μs	38.1	2.83	240.1 KB
#6397	`AllCycleMoreComplexBody`	net6.0	224μs	125ns	468ns	2.91	0	208.98 KB
#6397	`AllCycleMoreComplexBody`	netcoreapp3.1	324μs	134ns	465ns	2.99	0	216.34 KB
#6397	`AllCycleMoreComplexBody`	net472	291μs	300ns	1.16μs	38.7	2.89	243.61 KB
#6397	`ObjectExtractorSimpleBody`	net6.0	147ns	0.0645ns	0.241ns	0.00395	0	280 B
#6397	`ObjectExtractorSimpleBody`	netcoreapp3.1	225ns	1.19ns	6.17ns	0.00365	0	272 B
#6397	`ObjectExtractorSimpleBody`	net472	232ns	0.101ns	0.364ns	0.0446	0	281 B
#6397	`ObjectExtractorMoreComplexBody`	net6.0	2.96μs	0.977ns	3.66ns	0.0535	0	3.78 KB
#6397	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.86μs	2.08ns	8.05ns	0.05	0	3.69 KB
#6397	`ObjectExtractorMoreComplexBody`	net472	4.26μs	3.54ns	13.7ns	0.603	0.00632	3.8 KB

Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	37.2μs	11.2ns	43.6ns	0.446	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	54.2μs	16.6ns	64.5ns	0.435	0	32.4 KB
master	`EncodeArgs`	net472	67.1μs	42.1ns	163ns	5.15	0.0673	32.5 KB
master	`EncodeLegacyArgs`	net6.0	77.8μs	26ns	101ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	106μs	346ns	1.34μs	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	157μs	165ns	640ns	0.312	0	2.15 KB
#6397	`EncodeArgs`	net6.0	37.3μs	22.6ns	84.5ns	0.449	0	32.4 KB
#6397	`EncodeArgs`	netcoreapp3.1	54.5μs	31.7ns	123ns	0.433	0	32.4 KB
#6397	`EncodeArgs`	net472	66.9μs	59.1ns	229ns	5.15	0.0665	32.5 KB
#6397	`EncodeLegacyArgs`	net6.0	71.9μs	119ns	462ns	0	0	2.14 KB
#6397	`EncodeLegacyArgs`	netcoreapp3.1	106μs	420ns	1.63μs	0	0	2.14 KB
#6397	`EncodeLegacyArgs`	net472	155μs	159ns	615ns	0.308	0	2.15 KB

Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunWafRealisticBenchmark`	net6.0	180μs	128ns	477ns	0	2.44 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	195μs	214ns	827ns	0	2.39 KB
master	`RunWafRealisticBenchmark`	net472	207μs	210ns	814ns	0.309	2.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	49.2ns	191ns	0	1.47 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	131μs	109ns	409ns	0	1.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	139μs	35.9ns	129ns	0.209	1.49 KB
#6397	`RunWafRealisticBenchmark`	net6.0	185μs	102ns	394ns	0	2.44 KB
#6397	`RunWafRealisticBenchmark`	netcoreapp3.1	193μs	152ns	569ns	0	2.39 KB
#6397	`RunWafRealisticBenchmark`	net472	210μs	325ns	1.21μs	0.312	2.46 KB
#6397	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	219ns	848ns	0	1.47 KB
#6397	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	131μs	237ns	919ns	0	1.46 KB
#6397	`RunWafRealisticBenchmarkWithAttack`	net472	142μs	58.8ns	212ns	0.217	1.48 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #6397

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	61.77 KB	57.8 KB	-3.97 KB	-6.42%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	52.4μs	203ns	813ns	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	53.9μs	115ns	430ns	42.64 KB
master	`StringConcatBenchmark`	net472	37.3μs	91.7ns	318ns	61.77 KB
master	`StringConcatAspectBenchmark`	net6.0	295μs	5.69μs	56μs	254.71 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	351μs	1.81μs	11.9μs	253.84 KB
master	`StringConcatAspectBenchmark`	net472	282μs	4.91μs	47.3μs	278.53 KB
#6397	`StringConcatBenchmark`	net6.0	53.6μs	255ns	988ns	43.44 KB
#6397	`StringConcatBenchmark`	netcoreapp3.1	53.5μs	103ns	355ns	42.64 KB
#6397	`StringConcatBenchmark`	net472	38μs	112ns	402ns	57.8 KB
#6397	`StringConcatAspectBenchmark`	net6.0	300μs	5.49μs	54.1μs	255.04 KB
#6397	`StringConcatAspectBenchmark`	netcoreapp3.1	345μs	1.64μs	6.76μs	253.09 KB
#6397	`StringConcatAspectBenchmark`	net472	287μs	4.8μs	46.3μs	278.53 KB

andrewlock · 2024-12-04T10:30:13Z

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6397) (11.176M)   : 0, 11175593
    master (11.201M)   : 0, 11200575
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6397) (7.353M)   : 0, 7353182
    master (7.193M)   : 0, 7192592
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.637M)   : 0, 7636936

    section Manual
    master (11.152M)   : 0, 11151831

    section Manual + Automatic
    This PR (6397) (6.887M)   : 0, 6886656
    master (6.647M)   : 0, 6647097

    section DD_TRACE_ENABLED=0
    master (10.391M)   : 0, 10390637

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6397) (9.458M)   : 0, 9458419
    master (9.756M)   : 0, 9756093
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6397) (6.414M)   : 0, 6413803
    master (6.416M)   : 0, 6416022

    section Trace stats
    master (6.677M)   : 0, 6677436

    section Manual
    master (9.406M)   : 0, 9405878

    section Manual + Automatic
    This PR (6397) (5.946M)   : 0, 5946493
    master (5.969M)   : 0, 5969446

    section DD_TRACE_ENABLED=0
    master (8.683M)   : 0, 8683346

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6397) (9.955M)   : 0, 9954855
    master (9.691M)   : 0, 9690567
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6397) (6.408M)   : 0, 6407816
    master (6.374M)   : 0, 6373753
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (7.051M)   : 0, 7051311

    section Manual
    master (9.664M)   : 0, 9664005

    section Manual + Automatic
    This PR (6397) (5.991M)   : 0, 5990959
    master (6.028M)   : 0, 6028279

    section DD_TRACE_ENABLED=0
    master (9.015M)   : 0, 9015445

datadog-ddstaging · 2024-12-04T10:59:54Z

Datadog Report

Branch report: andrew/manual-instrumentation-config-source
Commit report: 219e031
Test service: dd-trace-dotnet

✅ 0 Failed, 452765 Passed, 2824 Skipped, 19h 8m 46.1s Total Time

andrewlock · 2024-12-04T14:35:16Z

Benchmarks Report for tracer 🐌

Benchmarks for #6397 compared to master:

2 benchmarks are slower, with geometric mean 1.135
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartStopWithChild`	net6.0	7.97μs	42.9ns	235ns	0.00799	0	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10.3μs	55.8ns	382ns	0.0203	0.0101	0	5.81 KB
master	`StartStopWithChild`	net472	16.3μs	47.8ns	179ns	1.04	0.298	0.0886	6.21 KB
#6397	`StartStopWithChild`	net6.0	7.82μs	43.5ns	272ns	0.0197	0.00786	0	5.61 KB
#6397	`StartStopWithChild`	netcoreapp3.1	10.1μs	54.3ns	287ns	0.0192	0.0096	0	5.8 KB
#6397	`StartStopWithChild`	net472	16.4μs	55ns	213ns	1.04	0.308	0.0972	6.21 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	476μs	498ns	1.93μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	652μs	387ns	1.5μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	844μs	346ns	1.34μs	0.419	3.3 KB
#6397	`WriteAndFlushEnrichedTraces`	net6.0	490μs	160ns	600ns	0	2.7 KB
#6397	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	649μs	388ns	1.5μs	0	2.7 KB
#6397	`WriteAndFlushEnrichedTraces`	net472	854μs	556ns	2.08μs	0.425	3.3 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	151μs	882ns	8.18μs	0.163	14.47 KB
master	`SendRequest`	netcoreapp3.1	170μs	1.06μs	10.4μs	0.176	17.27 KB
master	`SendRequest`	net472	0.00274ns	0.000976ns	0.00378ns	0	0 b
#6397	`SendRequest`	net6.0	151μs	879ns	7.51μs	0.14	14.47 KB
#6397	`SendRequest`	netcoreapp3.1	169μs	978ns	8.58μs	0.174	17.27 KB
#6397	`SendRequest`	net472	0.00318ns	0.00122ns	0.00471ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	582μs	2.98μs	14.3μs	0.558	0	0	41.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	681μs	3.98μs	35.6μs	0.327	0	0	41.81 KB
master	`WriteAndFlushEnrichedTraces`	net472	829μs	3.84μs	14.9μs	8.12	2.44	0.406	53.3 KB
#6397	`WriteAndFlushEnrichedTraces`	net6.0	605μs	3.17μs	15.8μs	0.551	0	0	41.89 KB
#6397	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	655μs	2.93μs	16μs	0.326	0	0	41.66 KB
#6397	`WriteAndFlushEnrichedTraces`	net472	859μs	4.11μs	16.4μs	8.13	2.57	0.428	53.29 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.25μs	1ns	3.89ns	0.0143	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.73μs	1.41ns	5.29ns	0.013	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.09μs	2.33ns	9.03ns	0.156	0.00104	987 B
#6397	`ExecuteNonQuery`	net6.0	1.24μs	1.02ns	3.97ns	0.0143	0	1.02 KB
#6397	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.19ns	4.61ns	0.0134	0	1.02 KB
#6397	`ExecuteNonQuery`	net472	2.1μs	1.32ns	5.1ns	0.156	0.00105	987 B

Benchmarks.Trace.ElasticsearchBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6397

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync‑net6.0	1.133	1,209.77	1,371.03

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.23μs	0.529ns	1.98ns	0.0135	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.65μs	0.639ns	2.39ns	0.0132	976 B
master	`CallElasticsearch`	net472	2.58μs	2.46ns	9.52ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.21μs	0.969ns	3.75ns	0.0133	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.57μs	0.575ns	2.15ns	0.0133	1.02 KB
master	`CallElasticsearchAsync`	net472	2.66μs	1.72ns	6.67ns	0.166	1.05 KB
#6397	`CallElasticsearch`	net6.0	1.28μs	0.715ns	2.68ns	0.0132	976 B
#6397	`CallElasticsearch`	netcoreapp3.1	1.55μs	0.449ns	1.68ns	0.0132	976 B
#6397	`CallElasticsearch`	net472	2.69μs	2.38ns	9.2ns	0.157	995 B
#6397	`CallElasticsearchAsync`	net6.0	1.37μs	0.707ns	2.65ns	0.0131	952 B
#6397	`CallElasticsearchAsync`	netcoreapp3.1	1.66μs	1.2ns	4.64ns	0.0142	1.02 KB
#6397	`CallElasticsearchAsync`	net472	2.62μs	2.48ns	9.26ns	0.166	1.05 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.37μs	1.37ns	5.13ns	0.0131	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.72μs	1.13ns	4.21ns	0.0129	952 B
master	`ExecuteAsync`	net472	1.81μs	0.823ns	3.19ns	0.145	915 B
#6397	`ExecuteAsync`	net6.0	1.35μs	0.468ns	1.69ns	0.013	952 B
#6397	`ExecuteAsync`	netcoreapp3.1	1.69μs	0.568ns	2.12ns	0.0121	952 B
#6397	`ExecuteAsync`	net472	1.83μs	0.642ns	2.49ns	0.144	915 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.39μs	2.34ns	8.75ns	0.0328	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.31μs	1.6ns	6.18ns	0.037	2.85 KB
master	`SendAsync`	net472	7.48μs	1.86ns	7.21ns	0.493	3.12 KB
#6397	`SendAsync`	net6.0	4.41μs	1.79ns	6.7ns	0.0329	2.31 KB
#6397	`SendAsync`	netcoreapp3.1	5.34μs	2.74ns	10.6ns	0.0376	2.85 KB
#6397	`SendAsync`	net472	7.36μs	2.19ns	8.18ns	0.493	3.12 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.43μs	0.522ns	1.88ns	0.0229	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.26μs	0.877ns	3.28ns	0.0216	1.64 KB
master	`EnrichedLog`	net472	2.73μs	2.1ns	8.12ns	0.249	1.57 KB
#6397	`EnrichedLog`	net6.0	1.46μs	1.16ns	4.35ns	0.0226	1.64 KB
#6397	`EnrichedLog`	netcoreapp3.1	2.2μs	1.31ns	5.07ns	0.022	1.64 KB
#6397	`EnrichedLog`	net472	2.56μs	1.93ns	7.49ns	0.249	1.57 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	119μs	123ns	477ns	0.0599	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	123μs	262ns	1.01μs	0.0615	0	4.28 KB
master	`EnrichedLog`	net472	151μs	180ns	699ns	0.683	0.228	4.46 KB
#6397	`EnrichedLog`	net6.0	119μs	186ns	719ns	0.0604	0	4.28 KB
#6397	`EnrichedLog`	netcoreapp3.1	123μs	94.1ns	339ns	0	0	4.28 KB
#6397	`EnrichedLog`	net472	151μs	121ns	468ns	0.684	0.228	4.46 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3.02μs	1.13ns	4.23ns	0.0302	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.19μs	1.25ns	4.68ns	0.0294	2.2 KB
master	`EnrichedLog`	net472	4.86μs	1.26ns	4.87ns	0.319	2.02 KB
#6397	`EnrichedLog`	net6.0	3.07μs	1.52ns	5.9ns	0.0305	2.2 KB
#6397	`EnrichedLog`	netcoreapp3.1	4.25μs	1.45ns	5.63ns	0.03	2.2 KB
#6397	`EnrichedLog`	net472	4.86μs	2.22ns	8.62ns	0.319	2.02 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.32μs	0.675ns	2.53ns	0.016	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.72μs	0.739ns	2.86ns	0.0154	1.14 KB
master	`SendReceive`	net472	2.06μs	2.04ns	7.89ns	0.183	1.16 KB
#6397	`SendReceive`	net6.0	1.36μs	0.787ns	2.95ns	0.0162	1.14 KB
#6397	`SendReceive`	netcoreapp3.1	1.79μs	1.18ns	4.42ns	0.0152	1.14 KB
#6397	`SendReceive`	net472	2.05μs	1.14ns	4.43ns	0.183	1.16 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.85μs	1.37ns	5.14ns	0.0228	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.97μs	1.99ns	7.72ns	0.0219	1.65 KB
master	`EnrichedLog`	net472	4.37μs	2.06ns	7.13ns	0.323	2.04 KB
#6397	`EnrichedLog`	net6.0	2.74μs	0.762ns	2.95ns	0.022	1.6 KB
#6397	`EnrichedLog`	netcoreapp3.1	3.84μs	1.34ns	5.02ns	0.0211	1.65 KB
#6397	`EnrichedLog`	net472	4.4μs	3.04ns	11.8ns	0.322	2.04 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6397

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0	1.136	483.30	549.23

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	456ns	0.117ns	0.439ns	0.00819	576 B
master	`StartFinishSpan`	netcoreapp3.1	612ns	0.431ns	1.67ns	0.00783	576 B
master	`StartFinishSpan`	net472	701ns	0.35ns	1.35ns	0.0916	578 B
master	`StartFinishScope`	net6.0	483ns	0.207ns	0.803ns	0.00974	696 B
master	`StartFinishScope`	netcoreapp3.1	795ns	0.468ns	1.75ns	0.00919	696 B
master	`StartFinishScope`	net472	869ns	0.484ns	1.88ns	0.104	658 B
#6397	`StartFinishSpan`	net6.0	479ns	0.278ns	1.08ns	0.00796	576 B
#6397	`StartFinishSpan`	netcoreapp3.1	612ns	0.369ns	1.33ns	0.0079	576 B
#6397	`StartFinishSpan`	net472	687ns	0.438ns	1.7ns	0.0917	578 B
#6397	`StartFinishScope`	net6.0	549ns	0.369ns	1.43ns	0.00987	696 B
#6397	`StartFinishScope`	netcoreapp3.1	793ns	0.249ns	0.898ns	0.00917	696 B
#6397	`StartFinishScope`	net472	864ns	0.466ns	1.74ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	685ns	0.359ns	1.39ns	0.00954	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	955ns	0.461ns	1.79ns	0.00954	696 B
master	`RunOnMethodBegin`	net472	1.11μs	0.797ns	2.87ns	0.105	658 B
#6397	`RunOnMethodBegin`	net6.0	666ns	0.329ns	1.28ns	0.00972	696 B
#6397	`RunOnMethodBegin`	netcoreapp3.1	1μs	0.611ns	2.37ns	0.00938	696 B
#6397	`RunOnMethodBegin`	net472	1.15μs	0.478ns	1.85ns	0.105	658 B

## Summary of changes - Delete the tracer settings snapshot source generator - Delete the snapshot objects ## Reason for change We used these for tracking when customers use config in code so we know which properties they're changing. However, in v3 we explicitly set the values ourselves in the `ConfigureIntegration` so we may as well do all the work there ## Implementation details - Delete the source generator - Delete usages of `[GenerateSnapshot]`, `[IgnoreForSnapshot]`, and `[ConfigKey]` - Delete `TracerSettingsSnapshot` and associated files and usages - Record config changes in `ConfigureIntegration` ## Test coverage Added a simple basic assertion to the manual instrumentation tests to assert that we're still recording the `code` origin ## Other details This is step 1 of a whole load of post-v3 code cleanup 😄 - #6370 👈 This PR - #6376 - #6385 - #6386 - #6397 - #6399

...Datadog.Trace/Configuration/ConfigurationSources/ManualInstrumentationConfigurationSource.cs

...g.Trace/Configuration/ConfigurationSources/ManualInstrumentationLegacyConfigurationSource.cs

## Summary of changes Delete the Public API Generator ## Reason for change We added this generator to reduce the amount of boilerplate you need to write to correctly record telemetry for public APIs. However, with the move to Datadog.Trace.Manual, these APIs are completely unused, so we may as well remove the complexity and duplication. ## Implementation details - Delete the `[GeneratePublicApi]` generator - Keeping the `[PublicApi]` attribute for now - that will be cleared up in a separate PR - Rename the previously-decorated properties that _were_ called `SomePropInternal` to `SomeProp` - We still have some "leftover" `Internal` suffixes, will tidy those up separately - Main changes are in `TracerSettings`, `ExporterSettings`, `IntegrationSettings` + their immutable counterparts ## Test coverage Covered by existing tests ## Other details I noticed that there were some properties that we were getting public API telemetry for which we kind of lost in the move to Datadog.Trace.Manual: `SpanContext.Parent`, `SpanContext.ParentId`, `SpanContext.ServiceName`. I don't _think_ that actually matters, as these can't actually be accessed any more for technical reasons... Part of stack - #6370 - #6376 👈 This PR - #6385 - #6386 - #6397 - #6399 - #6405

## Summary of changes - Delete legacy `IConfigurationSource` - Rename `ITelmeteredConfigurationSource` => `IConfigurationSource` - Remove some "public" API workarounds - Make some APIs `public` instead of `internal` (just to satisfy compiler) - Fix tests ## Reason for change `ITelmeteredConfigurationSource` was introduced when `IConfigurationSource` was public, and so could not be changed. We have since removed that interface from the public API (it's not in `Datadog.trace.Manual`) so now it simply adds confusion and complexity (as it should _not_ be used internally in Datadog.Trace). ## Implementation details This PR looks more complex than it is, because in order to replace all the usages of the old `IConfigurationSource` with the new one, I had to mark some other APIs public (and document them to satisfy the analyzers). It is mostly - Delete legacy `IConfigurationSource` - Rename `ITelmeteredConfigurationSource` => `IConfigurationSource` - Fix any errors I took the opportunity to also remove some of the `*Internal` methods which were used to avoid calling the "public" APIs; seeing as these aren't _actually_ public any more, that's just unnecessary duplication. ## Test coverage The testing was a pain. The `ConfigurationBuilder` tests were all designed to check that `ITelmeteredConfigurationSource` gave the same results as `IConfigurationSource` (while never _explicitly_ specifying what the "expected" behaviour was. I took some time to enumerate the _actual_ expected values for the `NameValueCollection` source, but the `Json` source has very different behaviour that is more of a pain to test, so I chose to simplify a lot there. We could do a _lot_ to clean up those tests, but I didn't want to add even more complexity in this PR. ## Other details To help "fix" the ASM tests I introduced a `DictionaryObjectConfigurationSource`, in which you pass in a `Dictionary<string, object>`. This is useful for testing (as it was previously used) but is actually going to be useful for a future clean-up PR too, so I kept it in Datadog.Trace instead of in the test project. Part of stack - #6370 - #6376 - #6385 - #6386 👈 This PR - #6397 - #6399 - #6400 - #6405 - #6408

…manual -> automatic

…ings

…from existing

@tonyredondo

…tead of mutating (#6399) ## Summary of changes Change how CI Visibility creates its `TracerSettings` object to avoid mutating settings afterwards ## Reason for change This stack of changes is about removing duplication (among other things) in the `TracerSettings` etc related objects. This is _partially_ required in order to remove the "snapshot generator" complexity that was removed in #6370. Given `TracerSettings` are not exposed publicly in Datadog.Trace, we want to move to doing a "one shot" configuring of them, ultimately so that we can make the object immutable (and remove `ImmutableTracerSettings` and friends). ## Implementation details CI Visibility is one of the few places where we mutate settings _after_ creating the `TracerSettings`. This is mostly because there's additional logic that CI Visibility wants to perform. Ultimately, the "solution" to that issue in this PR is to move that logic into the `TracerSettings` constructor. I'm not entirely happy about it, but it's the only approach I could find that works. - Add a "dummy" configuration value to a configuration source when creating the `TracerSettings`. This is used purely as a "switch" to say "we're in CI Visibility mode". - We absolutely could just pass this in as a constructor parameter. I went that route first and then backed away, but can totally be swayed. - Add an additional configuration source to update settings that we want to change in CI Vis (e.g. logs injection). - In the `TracerSettings` ctor, add an additional ignored URL when in CI Vis mode, modify the default service name (if required) and add an additional GlobalTag. ## Test coverage I'd love to have some, but the CI Visibility configuration is kinda all over the place, so if you have any pointers @tonyredondo I'm all ears... ## Other details Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 👈 This PR - #6400 - #6405 - #6408

## Summary of changes - Fixes the `verify_source_generated_changes_are_persisted.yml` workflow - Adds the missing changes ## Reason for change This workflow was broken when we stopped using a source generator to generate the call target integrations, so it was no longer flagging cases where the call target files have _not_ been correctly updated and need to be rebuilt using Nuke ## Implementation details Add the additional paths to include in the diff comparison ## Test coverage Did a manual test locally. Also committing this in 2 parts, to confirm it flags the currently broken master. ## Other details This missing file updates should have been added in #6397 but were missed because this was broken

…utableDirectLogSubmissionSettings` (#6400) ## Summary of changes Merge `DirectLogSubmissionSettings` with `ImmutableDirectLogSubmissionSettings` ## Reason for change There was never really a good reason for having these as separate types. It was primarily to make testing a little easier and to mirror the `TracerSettings`/`ImmutableTracerSettings` dichotomy, but as that's going away, this is just unnecessary complexity ## Implementation details - Moved the additional logic that was previously inside `ImmutableDirectLogSubmissionSettings` into `DirectLogSubmissionSettings` - Renamed the `DirectLogSubmissionSettings` properties to match the "Immutable" version (and remove the unnecessary prefix) - Replace all usages of `Immutable*` with `DirectLogSubmissionSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization (using `IConfigurationSource`) ## Test coverage Essentially the same. I removed/tweaked some tests that are no longer relevant ## Other details Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 👈 This PR - #6405 - #6408 - #6415

…tegrationSettings` (#6405) ## Summary of changes Merge `IntegrationSettings` with `ImmutableIntegrationSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Moved additional logic (handling `DisabledNames` that was previously inside `ImmutableIntegrationSettings` into `IntegrationSettings` - Replace all usages of `Immutable*` with `IntegrationSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization - Reorder initialization of `DisabledIntegrationNames` in `TracerSettings` so that it can be used in the `IntegrationSettings` constructor ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 👈 This PR - #6408 - #6415

…terSettings` (#6408) ## Summary of changes Merge `ExporterSettings` with `ImmutableExporterSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Made the properties in `ExporterSettings` get-only. This required quite a lot of work because we were doing a lot of mutating of the settings in the "helper" functions. - I only _lightly_ refactored those methods (as much as possible) to avoid setting the properties in the functions and instead returning the details to set later. - These are prime candidates for some _much_ heavier refactoring later, but I didn't want to get bogged down with that in this PR - Replace all usages of `Immutable*` with `ExporterSettings` - Replace usages of `AgentUriInternal` with `AgentUri` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 👈 This PR - #6415

…ettings` (#6415) ## Summary of changes Merge `TracerSettings` with `ImmutableTracerSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Make the properties in `TracerSettings` get-only. - Make the collections in `TracerSettings` readonly. - Move logic that used to be in the constructor of `ImmutableTracerSettings` into `TracerSettings` - e.g. Service/Version/Env were being changed based on DD_TAGS values. Moved that to TracerSettings and (importantly) added missing telemetry recording of these values. - Added missing recording of _effective_ `DisabledInstegrations` - Moving this logic caused some _tests_ to be broken (checking default values). Updated the expected values of those tests in a single - Replace all usages of `ImmutableTracerSettings` with `TracerSettings` - Move `ITracer` to Datadog.Trace.Manual - It's only used there, and references the manual-version of `ImmutableTracerSettings` which we _want_ to keep. - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization in tests ## Test coverage All covered by existing tests (I hope) 🤞 ## Other details There's still a _lot_ of scope to improve this Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 - #6415 👈 This PR

## Summary of changes - Delete the tracer settings snapshot source generator - Delete the snapshot objects ## Reason for change We used these for tracking when customers use config in code so we know which properties they're changing. However, in v3 we explicitly set the values ourselves in the `ConfigureIntegration` so we may as well do all the work there ## Implementation details - Delete the source generator - Delete usages of `[GenerateSnapshot]`, `[IgnoreForSnapshot]`, and `[ConfigKey]` - Delete `TracerSettingsSnapshot` and associated files and usages - Record config changes in `ConfigureIntegration` ## Test coverage Added a simple basic assertion to the manual instrumentation tests to assert that we're still recording the `code` origin ## Other details This is step 1 of a whole load of post-v3 code cleanup 😄 - #6370 👈 This PR - #6376 - #6385 - #6386 - #6397 - #6399

## Summary of changes Delete the Public API Generator ## Reason for change We added this generator to reduce the amount of boilerplate you need to write to correctly record telemetry for public APIs. However, with the move to Datadog.Trace.Manual, these APIs are completely unused, so we may as well remove the complexity and duplication. ## Implementation details - Delete the `[GeneratePublicApi]` generator - Keeping the `[PublicApi]` attribute for now - that will be cleared up in a separate PR - Rename the previously-decorated properties that _were_ called `SomePropInternal` to `SomeProp` - We still have some "leftover" `Internal` suffixes, will tidy those up separately - Main changes are in `TracerSettings`, `ExporterSettings`, `IntegrationSettings` + their immutable counterparts ## Test coverage Covered by existing tests ## Other details I noticed that there were some properties that we were getting public API telemetry for which we kind of lost in the move to Datadog.Trace.Manual: `SpanContext.Parent`, `SpanContext.ParentId`, `SpanContext.ServiceName`. I don't _think_ that actually matters, as these can't actually be accessed any more for technical reasons... Part of stack - #6370 - #6376 👈 This PR - #6385 - #6386 - #6397 - #6399 - #6405

…6385) ## Summary of changes Remove `PublicApiTests.PublicApiHasNotChanged` for Datadog.Trace.dll ## Reason for change The public API surface of Datadog.Trace is not _really_ public any more, as it's not directly referenced, so these tests are largely unneccessary. As of right now, `public` vs `internal` is essentially irrelevant in this project. ## Implementation details Remove the `PublicApiHasNotChanged` test. The other public API tests e.g. `AssemblyReferencesHaveNotChanged` _are_ still relevant, so jumped through some hoops to keep them ## Test coverage Slightly less now. ## Other details Part of stack - #6370 - #6376 - #6385 👈 This PR - #6386 - #6397 - #6399 - #6400 - #6405

## Summary of changes - Delete legacy `IConfigurationSource` - Rename `ITelmeteredConfigurationSource` => `IConfigurationSource` - Remove some "public" API workarounds - Make some APIs `public` instead of `internal` (just to satisfy compiler) - Fix tests ## Reason for change `ITelmeteredConfigurationSource` was introduced when `IConfigurationSource` was public, and so could not be changed. We have since removed that interface from the public API (it's not in `Datadog.trace.Manual`) so now it simply adds confusion and complexity (as it should _not_ be used internally in Datadog.Trace). ## Implementation details This PR looks more complex than it is, because in order to replace all the usages of the old `IConfigurationSource` with the new one, I had to mark some other APIs public (and document them to satisfy the analyzers). It is mostly - Delete legacy `IConfigurationSource` - Rename `ITelmeteredConfigurationSource` => `IConfigurationSource` - Fix any errors I took the opportunity to also remove some of the `*Internal` methods which were used to avoid calling the "public" APIs; seeing as these aren't _actually_ public any more, that's just unnecessary duplication. ## Test coverage The testing was a pain. The `ConfigurationBuilder` tests were all designed to check that `ITelmeteredConfigurationSource` gave the same results as `IConfigurationSource` (while never _explicitly_ specifying what the "expected" behaviour was. I took some time to enumerate the _actual_ expected values for the `NameValueCollection` source, but the `Json` source has very different behaviour that is more of a pain to test, so I chose to simplify a lot there. We could do a _lot_ to clean up those tests, but I didn't want to add even more complexity in this PR. ## Other details To help "fix" the ASM tests I introduced a `DictionaryObjectConfigurationSource`, in which you pass in a `Dictionary<string, object>`. This is useful for testing (as it was previously used) but is actually going to be useful for a future clean-up PR too, so I kept it in Datadog.Trace instead of in the test project. Part of stack - #6370 - #6376 - #6385 - #6386 👈 This PR - #6397 - #6399 - #6400 - #6405 - #6408

…nSource` (#6397) Change how we "apply" settings from manual configuration to the automatic side to use `IConfigurationSource` ## Reason for change The previous design of how we "apply" settings made in code using manual instrumentation required mutating a `TracerSettings` object after it was already built. In fact, this is pretty much the _only_ place that we mutate the settings. By switching to using a "configuration source" approach, so that the settings are built _once_ with the correct values opens up the option to make these immutable (and therefore delete all of the `Immutable*` settings we currently have. This reduces both code duplication and work we do on startup, and opens the path to further refactoring improvements. Note that the public API does not change, so consumers of Datadog.Trace.Manual are still working with a mutable object initially. ## Implementation details Currently we pass a `Dictionary<string, object>` between the manual and automatic side. Previously, we then iterated the keys, compared against known values, and modified `TracerSettings` as required. With the changes, we have a `ManualInstrumentationConfigurationSource` which just "wraps" the `Dictionary<>`, and returns the correct value as required for a given key. Overall, I think this is much cleaner. Where things get messy is how we handle disabling specific integrations. The existing dictionary is optimised for looping through the provided values, fetching the setting that needs to be modified, and changing all the required properties. Unfortunately, the `IConfigurationSource` approach where we're looking up by a key like `DD_TRACE_NPGSQL_ENABLED` works _horribly_ for this pattern 🙁. So I introduced an additional approach which explicitly _additionally_ transfers the settings using these values, making them just "standard" lookups. > Note that due to backwards + forwards compatibility requirements > - We _still_ need to send the "old" version of integration settings from the manual side, in case it's used with an old version of the auto instrumentation > - We _still_ need to handle the "old" version of integration settings in the auto side, in case it's used with an old version of the manual instrumentation. > - At least in this case we can use the more efficient `IConfigurationSource` reader, so we don't pay the expense of retrieving the settings. The only downside is a couple of extra allocations when they _do_ disable integrations in code. Minor other changes: - Add helper ctor to `CompositeConfigurationSource` for creating the internal list from a collection of `IConfigurationSource` - Tweak `DictionaryObjectConfigurationSource` so we can derive from it - Create a separate integration for <3.7.0 manual instrumentation that uses the legacy settings, otherwise use the new settings objects ## Test coverage Mostly covered by existing unit tests (and indirectly by integration tests). Tweaked the test to test both the new and legacy configuration source. ## Other details Requires #6393 to fix how we read integration settings Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 👈 This PR - #6399 - #6400 - #6405 - #6408

@tonyredondo

…tead of mutating (#6399) ## Summary of changes Change how CI Visibility creates its `TracerSettings` object to avoid mutating settings afterwards ## Reason for change This stack of changes is about removing duplication (among other things) in the `TracerSettings` etc related objects. This is _partially_ required in order to remove the "snapshot generator" complexity that was removed in #6370. Given `TracerSettings` are not exposed publicly in Datadog.Trace, we want to move to doing a "one shot" configuring of them, ultimately so that we can make the object immutable (and remove `ImmutableTracerSettings` and friends). ## Implementation details CI Visibility is one of the few places where we mutate settings _after_ creating the `TracerSettings`. This is mostly because there's additional logic that CI Visibility wants to perform. Ultimately, the "solution" to that issue in this PR is to move that logic into the `TracerSettings` constructor. I'm not entirely happy about it, but it's the only approach I could find that works. - Add a "dummy" configuration value to a configuration source when creating the `TracerSettings`. This is used purely as a "switch" to say "we're in CI Visibility mode". - We absolutely could just pass this in as a constructor parameter. I went that route first and then backed away, but can totally be swayed. - Add an additional configuration source to update settings that we want to change in CI Vis (e.g. logs injection). - In the `TracerSettings` ctor, add an additional ignored URL when in CI Vis mode, modify the default service name (if required) and add an additional GlobalTag. ## Test coverage I'd love to have some, but the CI Visibility configuration is kinda all over the place, so if you have any pointers @tonyredondo I'm all ears... ## Other details Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 👈 This PR - #6400 - #6405 - #6408

## Summary of changes - Fixes the `verify_source_generated_changes_are_persisted.yml` workflow - Adds the missing changes ## Reason for change This workflow was broken when we stopped using a source generator to generate the call target integrations, so it was no longer flagging cases where the call target files have _not_ been correctly updated and need to be rebuilt using Nuke ## Implementation details Add the additional paths to include in the diff comparison ## Test coverage Did a manual test locally. Also committing this in 2 parts, to confirm it flags the currently broken master. ## Other details This missing file updates should have been added in #6397 but were missed because this was broken

…utableDirectLogSubmissionSettings` (#6400) ## Summary of changes Merge `DirectLogSubmissionSettings` with `ImmutableDirectLogSubmissionSettings` ## Reason for change There was never really a good reason for having these as separate types. It was primarily to make testing a little easier and to mirror the `TracerSettings`/`ImmutableTracerSettings` dichotomy, but as that's going away, this is just unnecessary complexity ## Implementation details - Moved the additional logic that was previously inside `ImmutableDirectLogSubmissionSettings` into `DirectLogSubmissionSettings` - Renamed the `DirectLogSubmissionSettings` properties to match the "Immutable" version (and remove the unnecessary prefix) - Replace all usages of `Immutable*` with `DirectLogSubmissionSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization (using `IConfigurationSource`) ## Test coverage Essentially the same. I removed/tweaked some tests that are no longer relevant ## Other details Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 👈 This PR - #6405 - #6408 - #6415

…tegrationSettings` (#6405) ## Summary of changes Merge `IntegrationSettings` with `ImmutableIntegrationSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Moved additional logic (handling `DisabledNames` that was previously inside `ImmutableIntegrationSettings` into `IntegrationSettings` - Replace all usages of `Immutable*` with `IntegrationSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization - Reorder initialization of `DisabledIntegrationNames` in `TracerSettings` so that it can be used in the `IntegrationSettings` constructor ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 👈 This PR - #6408 - #6415

…terSettings` (#6408) ## Summary of changes Merge `ExporterSettings` with `ImmutableExporterSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Made the properties in `ExporterSettings` get-only. This required quite a lot of work because we were doing a lot of mutating of the settings in the "helper" functions. - I only _lightly_ refactored those methods (as much as possible) to avoid setting the properties in the functions and instead returning the details to set later. - These are prime candidates for some _much_ heavier refactoring later, but I didn't want to get bogged down with that in this PR - Replace all usages of `Immutable*` with `ExporterSettings` - Replace usages of `AgentUriInternal` with `AgentUri` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 👈 This PR - #6415

…ettings` (#6415) Merge `TracerSettings` with `ImmutableTracerSettings` This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. - Make the properties in `TracerSettings` get-only. - Make the collections in `TracerSettings` readonly. - Move logic that used to be in the constructor of `ImmutableTracerSettings` into `TracerSettings` - e.g. Service/Version/Env were being changed based on DD_TAGS values. Moved that to TracerSettings and (importantly) added missing telemetry recording of these values. - Added missing recording of _effective_ `DisabledInstegrations` - Moving this logic caused some _tests_ to be broken (checking default values). Updated the expected values of those tests in a single - Replace all usages of `ImmutableTracerSettings` with `TracerSettings` - Move `ITracer` to Datadog.Trace.Manual - It's only used there, and references the manual-version of `ImmutableTracerSettings` which we _want_ to keep. - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization in tests All covered by existing tests (I hope) 🤞 There's still a _lot_ of scope to improve this Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 - #6415 👈 This PR

andrewlock added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:refactor labels Dec 4, 2024

andrewlock requested review from a team as code owners December 4, 2024 09:23

andrewlock force-pushed the andrew/simplify-configuration-sources branch from 6448274 to ce085e6 Compare December 5, 2024 09:53

andrewlock requested review from a team as code owners December 5, 2024 09:53

andrewlock force-pushed the andrew/manual-instrumentation-config-source branch from e5ebb65 to 6d25385 Compare December 5, 2024 09:53

andrewlock mentioned this pull request Dec 5, 2024

Make DirectLogSubmissionSettings properly immutable and remove ImmutableDirectLogSubmissionSettings #6400

Merged

kevingosse reviewed Dec 5, 2024

View reviewed changes

andrewlock force-pushed the andrew/simplify-configuration-sources branch from ce085e6 to 1cc4bb6 Compare December 5, 2024 12:52

andrewlock force-pushed the andrew/manual-instrumentation-config-source branch from 6d25385 to 0dcb6ec Compare December 5, 2024 12:52

kevingosse approved these changes Dec 5, 2024

View reviewed changes

andrewlock mentioned this pull request Dec 5, 2024

Make IntegrationSettings properly immutable and remove ImmutableIntegrationSettings #6405

Merged

andrewlock force-pushed the andrew/simplify-configuration-sources branch from 1cc4bb6 to c68e10c Compare December 6, 2024 11:49

andrewlock mentioned this pull request Dec 6, 2024

Make ExporterSettings properly immutable and remove ImmutableExporterSettings #6408

Merged

Base automatically changed from andrew/simplify-configuration-sources to master December 6, 2024 16:22

andrewlock added 5 commits December 6, 2024 16:30

To make later changes easier, update _how_ we transfer settings from …

a43951f

…manual -> automatic

Create a ManualInstrumentationConfigurationSource for populating sett…

634d230

…ings

Add constructor to configuration source to allow easier construction …

bf5196d

…from existing

Use the manual instrumentation override configuration source

cc031b3

Add tests for ManualInstrumentationConfigurationSource

219e031

andrewlock force-pushed the andrew/manual-instrumentation-config-source branch from dd8d03c to 219e031 Compare December 6, 2024 16:32

andrewlock merged commit 94fc277 into master Dec 9, 2024
106 checks passed

andrewlock deleted the andrew/manual-instrumentation-config-source branch December 9, 2024 09:07

github-actions bot added this to the vNext-v3 milestone Dec 9, 2024

andrewlock mentioned this pull request Dec 10, 2024

Make TracerSettings properly immutable and remove ImmutableTracerSettings #6415

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update config-in-code manual instrumentation to use an `IConfigurationSource` #6397

Update config-in-code manual instrumentation to use an `IConfigurationSource` #6397

andrewlock commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6397

Raw results

andrewlock commented Dec 4, 2024 •

edited

Loading

datadog-ddstaging bot commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6397

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6397

Raw results

Raw results

Update config-in-code manual instrumentation to use an IConfigurationSource #6397

Update config-in-code manual instrumentation to use an IConfigurationSource #6397

Conversation

andrewlock commented Dec 4, 2024 • edited Loading

Reason for change

Implementation details

Test coverage

Other details

andrewlock commented Dec 4, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Dec 4, 2024 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6397

Raw results

andrewlock commented Dec 4, 2024 • edited Loading

Throughput/Crank Report ⚡

datadog-ddstaging bot commented Dec 4, 2024 • edited Loading

Datadog Report

andrewlock commented Dec 4, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6397

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6397

Raw results

Raw results

Update config-in-code manual instrumentation to use an `IConfigurationSource` #6397

Update config-in-code manual instrumentation to use an `IConfigurationSource` #6397

andrewlock commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading

datadog-ddstaging bot commented Dec 4, 2024 •

edited

Loading

andrewlock commented Dec 4, 2024 •

edited

Loading