Stop using Tracer.Instance if we don't need to

[andrewlock]

Summary of changes

Updates a couple of places where we're calling Tracer.Instance where we don't need to

Reason for change

In one of my other PRs I accidentally broke something that should only have affected integration tests, but a bunch of unit tests broke. It highlighted where they were using Tracer.Instance and setting the global tracer for tests. In other places, it revealed that code that tests that looked independent of other tests really wasn't...

Calling Tracer.Instance potentially does a lot of work, as it initializes the tracer. It's also hard to follow if we're making static calls out in places we don't need to. So just pass through the settings we want instead.

Implementation details

Instead of calling Tracer.Instance.Settings, use the value of Tracer or TracerSettings that's already available wherever possible. It makes the tests cleaner too.

Test coverage

Same coverage, just a bit cleaner

Other details

Included as part of the config stack, just because I already refactored some of this code, and can't be bothered to faff with merge conflicts:

https://datadoghq.atlassian.net/browse/LANGPLAT-819

• Extract MutableSettings from TracerSettings #7522
• Rebuild MutableSettings on dynamic config changes #7525
• Move DefaultServiceName to MutableSettings #7530
• Simplify PerTraceSettings.GetServiceName() #7532
• Fix ad-hoc use of TracerSettings to use MutableSettings where appropriate #7543
• Fix calls to IsIntegrationEnabled(), IsErrorStatusCode(), and GetIntegrationAnalyticsSampleRate() #7544
• Enforce that you can't update exporter settings to point to UDS on Windows #7721
• Add helper DictionaryExtensions.SequenceEqual #7722
• Create SettingsManager for managing mutable settings and ExporterSettings #7695
• Remove properties from TracerSettings which can change at runtime #7723
• Handle changes to MutableSettings and ExporterSettings without rebuilding #7724
• Stop using Tracer.Instance if we don't need to #7744 👈

[andrewlock]

This one is actually kind of annoying. As it's in a static cctor, we can't pass it in anywhere, which means there's one DbScopeFactoryTests test which will use this and end up initializing the global tracer. Oh well.

[bouwkast]

:sorry: pretty sure this was me

[zacharycmontoya]

Hmm. I understand that we want to minimize work but I feel like we should calculate all DbCommands regardless of config (i.e. no dependency here) and then we do the config check on the hot path, which allows this setting to be hot-reloadable or "config at runtime" ready

[zacharycmontoya]

Concretely, I'm suggesting that we remove the TryGetIntegrationDetails call in the static constructor of Cache<TCommand> and refactor the other call in CreateDbCommandScope a bit. Perhaps in a separate PR

[andrewlock]

I can see arguments both ways tbh.

The only downside to @zacharycmontoya's proposal would be more work to do for custom DbCommands (ones that we don't explicitly know about), which are also marked as disabled initially, and which customers have marked as disabled. The question I would have is whether that's ok - e.g. if they're disabled because we're erroring out literally running TryGetIntegrationDetails, and that's why they disabled it, then that could be a problem. If it's just an optimization, then yes, it probably is fine.

That said, this actually is technically already hot-reloadable. The DisabledAdoNetCommandTypes setting passed in here is used to decide whether to populate the cache for custom types, but if we don't populate the cache, then we run TryGetIntegrationDetails() and check DisabledAdoNetCommandTypes at runtime anyway. So if this changes, then the result changes, it will just never be cached in that scenario.

So on that basis, I think removing the TryGetIntegrationDetails has a small amount of risk (if customer disabled adonet due to errors - not sure if that's really possible, but I can believe it), means a small bit more work (calculating values which will never be used), and doesn't change how hot-reloadable we are in general (though if we made it hot reloadable, we'd have to rethink how caching works). The plus side is it removes the static access to Tracer.Instance which is better for tests.

So yeah, I'm torn 🤷‍♂️ I'd say separate PR either way, given the questions, as the current behaviour in this PR is identical to the existing. But it's worth thinking about

[bouwkast]

:sorry: pretty sure this was me

[bouwkast]

😃

[dd-trace-dotnet-ci-bot]

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (7744) and master.

✅ No regressions detected - check the details below

Full Metrics Comparison

FakeDbCommand

Metric	Master (Mean ± 95% CI)	Current (Mean ± 95% CI)	Change	Status
.NET Framework 4.8 - Baseline
duration	74.35 ± (74.28 - 75.12) ms	73.72 ± (73.82 - 74.38) ms	-0.8%	✅
.NET Framework 4.8 - Bailout
duration	80.22 ± (79.93 - 80.66) ms	80.30 ± (80.15 - 80.80) ms	+0.1%	✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration	1117.66 ± (1119.26 - 1129.74) ms	1113.91 ± (1115.09 - 1123.06) ms	-0.3%	✅
.NET Core 3.1 - Baseline
process.internal_duration_ms	22.89 ± (22.82 - 22.97) ms	22.85 ± (22.79 - 22.91) ms	-0.2%	✅
process.time_to_main_ms	86.97 ± (86.66 - 87.28) ms	87.53 ± (87.19 - 87.88) ms	+0.6%	✅⬆️
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	10.91 ± (10.91 - 10.91) MB	10.90 ± (10.90 - 10.91) MB	-0.1%	✅
runtime.dotnet.threads.count	12 ± (12 - 12)	12 ± (12 - 12)	+0.0%	✅
.NET Core 3.1 - Bailout
process.internal_duration_ms	22.61 ± (22.56 - 22.67) ms	22.83 ± (22.76 - 22.90) ms	+1.0%	✅⬆️
process.time_to_main_ms	87.33 ± (87.02 - 87.64) ms	88.56 ± (88.21 - 88.91) ms	+1.4%	✅⬆️
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	10.95 ± (10.95 - 10.95) MB	10.94 ± (10.93 - 10.94) MB	-0.1%	✅
runtime.dotnet.threads.count	13 ± (13 - 13)	13 ± (13 - 13)	+0.0%	✅
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms	218.42 ± (216.98 - 219.87) ms	220.50 ± (219.29 - 221.71) ms	+1.0%	✅⬆️
process.time_to_main_ms	548.34 ± (547.21 - 549.46) ms	546.67 ± (545.48 - 547.85) ms	-0.3%	✅
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	52.48 ± (52.46 - 52.50) MB	52.52 ± (52.50 - 52.54) MB	+0.1%	✅⬆️
runtime.dotnet.threads.count	28 ± (28 - 28)	28 ± (28 - 28)	+0.6%	✅⬆️
.NET 6 - Baseline
process.internal_duration_ms	21.65 ± (21.59 - 21.72) ms	21.58 ± (21.52 - 21.65) ms	-0.3%	✅
process.time_to_main_ms	75.20 ± (74.86 - 75.53) ms	74.92 ± (74.61 - 75.23) ms	-0.4%	✅
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	10.60 ± (10.60 - 10.60) MB	10.61 ± (10.61 - 10.61) MB	+0.1%	✅⬆️
runtime.dotnet.threads.count	10 ± (10 - 10)	10 ± (10 - 10)	+0.0%	✅
.NET 6 - Bailout
process.internal_duration_ms	21.70 ± (21.62 - 21.77) ms	21.56 ± (21.49 - 21.62) ms	-0.6%	✅
process.time_to_main_ms	77.97 ± (77.53 - 78.40) ms	76.50 ± (76.22 - 76.78) ms	-1.9%	✅
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	10.68 ± (10.67 - 10.68) MB	10.65 ± (10.65 - 10.66) MB	-0.2%	✅
runtime.dotnet.threads.count	11 ± (11 - 11)	11 ± (11 - 11)	+0.0%	✅
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms	208.08 ± (206.52 - 209.65) ms	207.47 ± (206.34 - 208.59) ms	-0.3%	✅
process.time_to_main_ms	517.01 ± (515.90 - 518.11) ms	508.41 ± (507.36 - 509.46) ms	-1.7%	✅
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	51.60 ± (51.57 - 51.63) MB	51.57 ± (51.54 - 51.59) MB	-0.1%	✅
runtime.dotnet.threads.count	28 ± (28 - 28)	28 ± (28 - 28)	+0.0%	✅⬆️
.NET 8 - Baseline
process.internal_duration_ms	19.66 ± (19.60 - 19.72) ms	19.85 ± (19.79 - 19.91) ms	+1.0%	✅⬆️
process.time_to_main_ms	74.09 ± (73.76 - 74.42) ms	74.63 ± (74.33 - 74.93) ms	+0.7%	✅⬆️
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	7.65 ± (7.65 - 7.66) MB	7.65 ± (7.64 - 7.65) MB	-0.0%	✅
runtime.dotnet.threads.count	10 ± (10 - 10)	10 ± (10 - 10)	+0.0%	✅
.NET 8 - Bailout
process.internal_duration_ms	19.74 ± (19.68 - 19.80) ms	19.82 ± (19.76 - 19.88) ms	+0.4%	✅⬆️
process.time_to_main_ms	76.38 ± (76.06 - 76.69) ms	75.22 ± (74.94 - 75.49) ms	-1.5%	✅
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	7.71 ± (7.71 - 7.72) MB	7.70 ± (7.69 - 7.71) MB	-0.2%	✅
runtime.dotnet.threads.count	11 ± (11 - 11)	11 ± (11 - 11)	+0.0%	✅
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms	190.34 ± (189.38 - 191.29) ms	191.89 ± (190.98 - 192.81) ms	+0.8%	✅⬆️
process.time_to_main_ms	489.56 ± (488.57 - 490.54) ms	484.31 ± (483.33 - 485.29) ms	-1.1%	✅
runtime.dotnet.exceptions.count	0 ± (0 - 0)	0 ± (0 - 0)	+0.0%	✅
runtime.dotnet.mem.committed	38.85 ± (38.81 - 38.88) MB	38.87 ± (38.83 - 38.91) MB	+0.1%	✅⬆️
runtime.dotnet.threads.count	27 ± (27 - 27)	27 ± (27 - 27)	+0.0%	✅⬆️

HttpMessageHandler

Metric	Master (Mean ± 95% CI)	Current (Mean ± 95% CI)	Change	Status
.NET Framework 4.8 - Baseline
duration	192.93 ± (193.00 - 193.85) ms	191.85 ± (191.82 - 192.54) ms	-0.6%	✅
.NET Framework 4.8 - Bailout
duration	195.89 ± (195.70 - 196.15) ms	195.77 ± (195.82 - 196.58) ms	-0.1%	✅
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration	1167.84 ± (1170.49 - 1180.26) ms	1163.78 ± (1163.56 - 1169.33) ms	-0.3%	✅
.NET Core 3.1 - Baseline
process.internal_duration_ms	187.65 ± (187.23 - 188.07) ms	188.15 ± (187.68 - 188.62) ms	+0.3%	✅⬆️
process.time_to_main_ms	80.39 ± (80.20 - 80.59) ms	80.58 ± (80.35 - 80.81) ms	+0.2%	✅⬆️
runtime.dotnet.exceptions.count	3 ± (3 - 3)	3 ± (3 - 3)	+0.0%	✅
runtime.dotnet.mem.committed	16.07 ± (16.04 - 16.10) MB	16.13 ± (16.10 - 16.15) MB	+0.4%	✅⬆️
runtime.dotnet.threads.count	20 ± (20 - 20)	20 ± (20 - 20)	-0.1%	✅
.NET Core 3.1 - Bailout
process.internal_duration_ms	187.30 ± (186.99 - 187.61) ms	186.99 ± (186.70 - 187.28) ms	-0.2%	✅
process.time_to_main_ms	81.56 ± (81.44 - 81.68) ms	81.47 ± (81.35 - 81.59) ms	-0.1%	✅
runtime.dotnet.exceptions.count	3 ± (3 - 3)	3 ± (3 - 3)	+0.0%	✅
runtime.dotnet.mem.committed	16.15 ± (16.12 - 16.17) MB	16.19 ± (16.16 - 16.22) MB	+0.3%	✅⬆️
runtime.dotnet.threads.count	21 ± (20 - 21)	21 ± (20 - 21)	+0.0%	✅⬆️
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms	391.93 ± (389.73 - 394.14) ms	393.49 ± (391.08 - 395.89) ms	+0.4%	✅⬆️
process.time_to_main_ms	514.76 ± (514.10 - 515.42) ms	517.69 ± (517.07 - 518.30) ms	+0.6%	✅⬆️
runtime.dotnet.exceptions.count	3 ± (3 - 3)	3 ± (3 - 3)	+0.0%	✅
runtime.dotnet.mem.committed	62.85 ± (62.70 - 63.01) MB	62.88 ± (62.72 - 63.03) MB	+0.0%	✅⬆️
runtime.dotnet.threads.count	29 ± (29 - 29)	29 ± (29 - 30)	+0.0%	✅⬆️
.NET 6 - Baseline
process.internal_duration_ms	191.69 ± (191.29 - 192.08) ms	191.99 ± (191.65 - 192.34) ms	+0.2%	✅⬆️
process.time_to_main_ms	69.58 ± (69.41 - 69.75) ms	69.78 ± (69.60 - 69.96) ms	+0.3%	✅⬆️
runtime.dotnet.exceptions.count	4 ± (4 - 4)	4 ± (4 - 4)	+0.0%	✅
runtime.dotnet.mem.committed	16.08 ± (15.94 - 16.22) MB	16.17 ± (16.04 - 16.30) MB	+0.6%	✅⬆️
runtime.dotnet.threads.count	18 ± (18 - 18)	18 ± (18 - 18)	+0.4%	✅⬆️
.NET 6 - Bailout
process.internal_duration_ms	190.48 ± (190.15 - 190.81) ms	190.82 ± (190.51 - 191.12) ms	+0.2%	✅⬆️
process.time_to_main_ms	70.42 ± (70.31 - 70.53) ms	70.59 ± (70.50 - 70.68) ms	+0.2%	✅⬆️
runtime.dotnet.exceptions.count	4 ± (4 - 4)	4 ± (4 - 4)	+0.0%	✅
runtime.dotnet.mem.committed	15.98 ± (15.82 - 16.13) MB	16.10 ± (15.97 - 16.24) MB	+0.8%	✅⬆️
runtime.dotnet.threads.count	19 ± (19 - 19)	20 ± (19 - 20)	+3.2%	✅⬆️
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms	413.82 ± (410.59 - 417.04) ms	405.07 ± (402.76 - 407.38) ms	-2.1%	✅
process.time_to_main_ms	485.74 ± (485.15 - 486.32) ms	484.38 ± (483.67 - 485.08) ms	-0.3%	✅
runtime.dotnet.exceptions.count	4 ± (4 - 4)	4 ± (4 - 4)	+0.0%	✅
runtime.dotnet.mem.committed	62.09 ± (61.95 - 62.23) MB	62.38 ± (62.25 - 62.51) MB	+0.5%	✅⬆️
runtime.dotnet.threads.count	29 ± (29 - 29)	29 ± (29 - 29)	-0.0%	✅
.NET 8 - Baseline
process.internal_duration_ms	189.57 ± (189.19 - 189.95) ms	189.24 ± (188.91 - 189.56) ms	-0.2%	✅
process.time_to_main_ms	69.10 ± (68.89 - 69.31) ms	68.73 ± (68.56 - 68.90) ms	-0.5%	✅
runtime.dotnet.exceptions.count	4 ± (4 - 4)	4 ± (4 - 4)	+0.0%	✅
runtime.dotnet.mem.committed	11.77 ± (11.73 - 11.82) MB	11.77 ± (11.74 - 11.80) MB	-0.0%	✅
runtime.dotnet.threads.count	18 ± (18 - 18)	18 ± (18 - 18)	-0.8%	✅
.NET 8 - Bailout
process.internal_duration_ms	189.00 ± (188.71 - 189.29) ms	189.49 ± (189.13 - 189.84) ms	+0.3%	✅⬆️
process.time_to_main_ms	69.85 ± (69.75 - 69.95) ms	70.16 ± (70.03 - 70.28) ms	+0.4%	✅⬆️
runtime.dotnet.exceptions.count	4 ± (4 - 4)	4 ± (4 - 4)	+0.0%	✅
runtime.dotnet.mem.committed	11.85 ± (11.81 - 11.88) MB	11.67 ± (11.58 - 11.75) MB	-1.5%	✅
runtime.dotnet.threads.count	19 ± (19 - 19)	19 ± (18 - 19)	-2.7%	✅
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms	361.34 ± (359.79 - 362.89) ms	361.83 ± (360.22 - 363.45) ms	+0.1%	✅⬆️
process.time_to_main_ms	463.68 ± (463.03 - 464.32) ms	458.47 ± (457.76 - 459.18) ms	-1.1%	✅
runtime.dotnet.exceptions.count	4 ± (4 - 4)	4 ± (4 - 4)	+0.0%	✅
runtime.dotnet.mem.committed	50.34 ± (50.30 - 50.38) MB	50.25 ± (50.22 - 50.29) MB	-0.2%	✅
runtime.dotnet.threads.count	29 ± (29 - 29)	29 ± (29 - 29)	+0.2%	✅⬆️

Comparison explanation

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 highlighted 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).

Duration charts

FakeDbCommand (.NET Framework 4.8)

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (74ms)  : 70, 78
    master - mean (75ms)  : 69, 80

    section Bailout
    This PR (7744) - mean (80ms)  : 76, 85
    master - mean (80ms)  : 75, 86

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (1,119ms)  : 1062, 1177
    master - mean (1,125ms)  : 1046, 1203

Loading

FakeDbCommand (.NET Core 3.1)

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (118ms)  : 111, 124
    master - mean (117ms)  : 112, 123

    section Bailout
    This PR (7744) - mean (119ms)  : 113, 124
    master - mean (117ms)  : 111, 124

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (800ms)  : 762, 837
    master - mean (799ms)  : 749, 848

Loading

FakeDbCommand (.NET 6)

gantt
    title Execution time (ms) FakeDbCommand (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (103ms)  : 96, 111
    master - mean (104ms)  : 97, 111

    section Bailout
    This PR (7744) - mean (105ms)  : 100, 109
    master - mean (106ms)  : 98, 115

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (744ms)  : 714, 774
    master - mean (757ms)  : 715, 800

Loading

FakeDbCommand (.NET 8)

gantt
    title Execution time (ms) FakeDbCommand (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (103ms)  : 98, 108
    master - mean (102ms)  : 95, 110

    section Bailout
    This PR (7744) - mean (103ms)  : 97, 109
    master - mean (105ms)  : 98, 112

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (706ms)  : 676, 736
    master - mean (709ms)  : 681, 737

Loading

HttpMessageHandler (.NET Framework 4.8)

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (192ms)  : 189, 196
    master - mean (193ms)  : 188, 199

    section Bailout
    This PR (7744) - mean (196ms)  : 193, 200
    master - mean (196ms)  : 194, 198

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (1,166ms)  : 1125, 1208
    master - mean (1,175ms)  : 1101, 1250

Loading

HttpMessageHandler (.NET Core 3.1)

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (277ms)  : 270, 284
    master - mean (277ms)  : 272, 282

    section Bailout
    This PR (7744) - mean (276ms)  : 272, 281
    master - mean (277ms)  : 273, 281

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (943ms)  : 899, 987
    master - mean (939ms)  : 900, 978

Loading

HttpMessageHandler (.NET 6)

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (270ms)  : 264, 276
    master - mean (270ms)  : 264, 275

    section Bailout
    This PR (7744) - mean (269ms)  : 265, 273
    master - mean (269ms)  : 265, 273

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (920ms)  : 882, 958
    master - mean (929ms)  : 873, 986

Loading

HttpMessageHandler (.NET 8)

gantt
    title Execution time (ms) HttpMessageHandler (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7744) - mean (268ms)  : 263, 272
    master - mean (269ms)  : 262, 276

    section Bailout
    This PR (7744) - mean (269ms)  : 265, 273
    master - mean (268ms)  : 264, 272

    section CallTarget+Inlining+NGEN
    This PR (7744) - mean (851ms)  : 832, 870
    master - mean (858ms)  : 837, 879

Loading