Skip to content

Conversation

@andrewlock
Copy link
Member

@andrewlock andrewlock commented Jan 8, 2026

Summary of changes

Fix incorrect nullable annotations on Activity duck types

Reason for change

While working on other performance things, noticed that the nullable annotations often declared non-nullability when they actually could be null.

A particularly confusing part are the TraceId and SpanId values in IW3CActivity. These were marked non-nullable because when you call Activity.Start() then these will always be non-null, but only if you're using W3C IDs. If you're using hierarchical IDs (the default in <.NET 5) then these values will be null.

As a side note, I suspect this explains the "we saw errors about these being null in error tracking but don't understand why" scenarios 😄

Also, I think we should rename IW3CActivity to IActivity3 instead. W3C implies that it's a W3C activity, but that's not necessarily the case, and is essentially the source of the above confusion I think. IActivity3 would then be consistently named with IActivity5 and IActivity6 we also currently have.

Implementation details

Add nullable annotations to values that can be null, and fix the fallout (mostly in ActivityKey)

Test coverage

Covered by existing tests sufficiently I think

Other details

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

Part of a stack working to improve OTel performance

@datadog-datadog-prod-us1

This comment has been minimized.

@dd-trace-dotnet-ci-bot
Copy link

dd-trace-dotnet-ci-bot bot commented Jan 8, 2026

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (8038) 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
duration68.47 ± (68.55 - 68.86) ms68.38 ± (68.37 - 68.58) ms-0.1%
.NET Framework 4.8 - Bailout
duration72.21 ± (72.14 - 72.43) ms72.18 ± (72.09 - 72.42) ms-0.0%
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1002.94 ± (1003.82 - 1008.80) ms1003.02 ± (1004.45 - 1010.17) ms+0.0%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms21.99 ± (21.96 - 22.02) ms22.00 ± (21.96 - 22.04) ms+0.1%✅⬆️
process.time_to_main_ms78.58 ± (78.46 - 78.70) ms78.56 ± (78.43 - 78.69) ms-0.0%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.92 ± (10.92 - 10.93) MB10.90 ± (10.90 - 10.91) MB-0.2%
runtime.dotnet.threads.count12 ± (12 - 12)12 ± (12 - 12)+0.0%
.NET Core 3.1 - Bailout
process.internal_duration_ms21.87 ± (21.86 - 21.89) ms22.04 ± (22.02 - 22.07) ms+0.8%✅⬆️
process.time_to_main_ms79.95 ± (79.84 - 80.05) ms79.96 ± (79.86 - 80.05) ms+0.0%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.97 ± (10.96 - 10.97) MB10.93 ± (10.93 - 10.94) MB-0.3%
runtime.dotnet.threads.count13 ± (13 - 13)13 ± (13 - 13)+0.0%
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms231.98 ± (227.92 - 236.04) ms233.90 ± (229.74 - 238.07) ms+0.8%✅⬆️
process.time_to_main_ms471.89 ± (471.34 - 472.45) ms470.63 ± (470.11 - 471.14) ms-0.3%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed48.15 ± (48.13 - 48.18) MB48.23 ± (48.20 - 48.25) MB+0.1%✅⬆️
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)-0.2%
.NET 6 - Baseline
process.internal_duration_ms20.73 ± (20.71 - 20.76) ms20.70 ± (20.66 - 20.73) ms-0.2%
process.time_to_main_ms68.02 ± (67.92 - 68.13) ms68.13 ± (68.00 - 68.26) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.64 ± (10.63 - 10.64) MB10.64 ± (10.64 - 10.64) MB+0.1%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 6 - Bailout
process.internal_duration_ms20.74 ± (20.71 - 20.76) ms20.51 ± (20.49 - 20.53) ms-1.1%
process.time_to_main_ms69.10 ± (69.05 - 69.16) ms68.90 ± (68.85 - 68.95) ms-0.3%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.69 ± (10.69 - 10.70) MB10.73 ± (10.72 - 10.73) MB+0.3%✅⬆️
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms244.98 ± (242.67 - 247.30) ms246.75 ± (245.48 - 248.02) ms+0.7%✅⬆️
process.time_to_main_ms442.27 ± (441.83 - 442.71) ms440.62 ± (440.11 - 441.13) ms-0.4%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed48.72 ± (48.69 - 48.75) MB48.66 ± (48.63 - 48.69) MB-0.1%
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.6%✅⬆️
.NET 8 - Baseline
process.internal_duration_ms18.82 ± (18.80 - 18.84) ms18.80 ± (18.78 - 18.83) ms-0.1%
process.time_to_main_ms67.42 ± (67.32 - 67.53) ms67.01 ± (66.91 - 67.11) ms-0.6%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.68 ± (7.67 - 7.68) MB7.67 ± (7.67 - 7.68) MB-0.0%
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 8 - Bailout
process.internal_duration_ms18.80 ± (18.77 - 18.82) ms18.92 ± (18.89 - 18.94) ms+0.6%✅⬆️
process.time_to_main_ms68.20 ± (68.15 - 68.26) ms68.30 ± (68.24 - 68.37) ms+0.1%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.75 ± (7.74 - 7.76) MB7.73 ± (7.72 - 7.74) MB-0.2%
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms178.56 ± (177.76 - 179.36) ms178.03 ± (177.21 - 178.85) ms-0.3%
process.time_to_main_ms425.68 ± (425.04 - 426.33) ms425.58 ± (425.01 - 426.15) ms-0.0%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed36.33 ± (36.30 - 36.36) MB36.38 ± (36.35 - 36.41) MB+0.1%✅⬆️
runtime.dotnet.threads.count27 ± (27 - 27)27 ± (27 - 27)-0.3%

HttpMessageHandler

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration192.64 ± (192.63 - 193.43) ms192.64 ± (192.73 - 193.63) ms+0.0%✅⬆️
.NET Framework 4.8 - Bailout
duration195.63 ± (195.45 - 195.97) ms195.97 ± (195.75 - 196.27) ms+0.2%✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1112.08 ± (1116.59 - 1125.39) ms1113.00 ± (1120.78 - 1131.81) ms+0.1%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms187.95 ± (187.59 - 188.32) ms188.01 ± (187.57 - 188.46) ms+0.0%✅⬆️
process.time_to_main_ms80.15 ± (79.95 - 80.35) ms80.27 ± (80.05 - 80.50) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.06 ± (16.03 - 16.09) MB16.16 ± (16.13 - 16.20) MB+0.6%✅⬆️
runtime.dotnet.threads.count20 ± (19 - 20)20 ± (19 - 20)-0.2%
.NET Core 3.1 - Bailout
process.internal_duration_ms186.96 ± (186.68 - 187.24) ms186.56 ± (186.26 - 186.87) ms-0.2%
process.time_to_main_ms81.33 ± (81.20 - 81.45) ms81.38 ± (81.24 - 81.52) ms+0.1%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.15 ± (16.12 - 16.18) MB16.20 ± (16.17 - 16.23) MB+0.3%✅⬆️
runtime.dotnet.threads.count21 ± (21 - 21)21 ± (20 - 21)-0.2%
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms413.35 ± (409.65 - 417.05) ms423.17 ± (419.83 - 426.50) ms+2.4%✅⬆️
process.time_to_main_ms473.20 ± (472.49 - 473.91) ms473.51 ± (472.89 - 474.13) ms+0.1%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed58.38 ± (58.25 - 58.51) MB58.72 ± (58.61 - 58.84) MB+0.6%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 30)29 ± (29 - 29)-0.1%
.NET 6 - Baseline
process.internal_duration_ms191.93 ± (191.55 - 192.31) ms192.75 ± (192.29 - 193.21) ms+0.4%✅⬆️
process.time_to_main_ms69.54 ± (69.30 - 69.79) ms69.48 ± (69.32 - 69.64) ms-0.1%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.19 ± (16.08 - 16.31) MB16.20 ± (16.09 - 16.32) MB+0.0%✅⬆️
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 19)-0.5%
.NET 6 - Bailout
process.internal_duration_ms190.02 ± (189.79 - 190.24) ms190.66 ± (190.31 - 191.00) ms+0.3%✅⬆️
process.time_to_main_ms70.18 ± (70.07 - 70.29) ms70.29 ± (70.19 - 70.40) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed15.87 ± (15.71 - 16.03) MB16.25 ± (16.12 - 16.38) MB+2.4%✅⬆️
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 20)+1.8%✅⬆️
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms451.37 ± (448.87 - 453.87) ms453.75 ± (451.59 - 455.91) ms+0.5%✅⬆️
process.time_to_main_ms445.42 ± (444.98 - 445.86) ms444.76 ± (444.26 - 445.27) ms-0.1%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed58.36 ± (58.22 - 58.50) MB58.40 ± (58.27 - 58.52) MB+0.1%✅⬆️
runtime.dotnet.threads.count30 ± (29 - 30)30 ± (29 - 30)-0.0%
.NET 8 - Baseline
process.internal_duration_ms190.10 ± (189.72 - 190.48) ms189.92 ± (189.55 - 190.29) ms-0.1%
process.time_to_main_ms68.89 ± (68.70 - 69.08) ms69.05 ± (68.83 - 69.28) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.73 ± (11.70 - 11.76) MB11.76 ± (11.73 - 11.79) MB+0.3%✅⬆️
runtime.dotnet.threads.count18 ± (18 - 18)18 ± (18 - 18)+0.0%✅⬆️
.NET 8 - Bailout
process.internal_duration_ms189.27 ± (189.01 - 189.53) ms189.01 ± (188.74 - 189.27) ms-0.1%
process.time_to_main_ms69.86 ± (69.74 - 69.98) ms69.94 ± (69.83 - 70.05) ms+0.1%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.77 ± (11.75 - 11.80) MB11.85 ± (11.82 - 11.89) MB+0.7%✅⬆️
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 19)+0.6%✅⬆️
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms363.95 ± (362.43 - 365.47) ms364.87 ± (363.25 - 366.49) ms+0.3%✅⬆️
process.time_to_main_ms426.89 ± (426.31 - 427.48) ms428.16 ± (427.53 - 428.78) ms+0.3%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed47.96 ± (47.93 - 47.99) MB48.00 ± (47.97 - 48.04) MB+0.1%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)-0.0%
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 (8038) - mean (68ms)  : 67, 70
    master - mean (69ms)  : 66, 71

    section Bailout
    This PR (8038) - mean (72ms)  : 71, 74
    master - mean (72ms)  : 71, 74

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (1,007ms)  : 965, 1049
    master - mean (1,006ms)  : 970, 1043

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 (8038) - mean (106ms)  : 104, 108
    master - mean (106ms)  : 103, 109

    section Bailout
    This PR (8038) - mean (107ms)  : 106, 109
    master - mean (107ms)  : 106, 108

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (730ms)  : 672, 787
    master - mean (732ms)  : 672, 793

Loading
FakeDbCommand (.NET 6)
gantt
    title Execution time (ms) FakeDbCommand (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8038) - mean (94ms)  : 91, 97
    master - mean (93ms)  : 92, 95

    section Bailout
    This PR (8038) - mean (94ms)  : 93, 95
    master - mean (95ms)  : 93, 96

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (711ms)  : 690, 732
    master - mean (713ms)  : 678, 748

Loading
FakeDbCommand (.NET 8)
gantt
    title Execution time (ms) FakeDbCommand (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8038) - mean (92ms)  : 89, 95
    master - mean (92ms)  : 90, 95

    section Bailout
    This PR (8038) - mean (93ms)  : 92, 94
    master - mean (93ms)  : 92, 94

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (631ms)  : 616, 647
    master - mean (632ms)  : 614, 650

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 (8038) - mean (193ms)  : 189, 198
    master - mean (193ms)  : 189, 197

    section Bailout
    This PR (8038) - mean (196ms)  : 193, 199
    master - mean (196ms)  : 193, 198

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (1,126ms)  : 1040, 1212
    master - mean (1,121ms)  : 1055, 1187

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 (8038) - mean (277ms)  : 271, 283
    master - mean (276ms)  : 272, 281

    section Bailout
    This PR (8038) - mean (276ms)  : 273, 280
    master - mean (276ms)  : 273, 280

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (927ms)  : 884, 970
    master - mean (918ms)  : 858, 978

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

    section Bailout
    This PR (8038) - mean (269ms)  : 264, 274
    master - mean (268ms)  : 265, 271

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (927ms)  : 893, 960
    master - mean (926ms)  : 887, 965

Loading
HttpMessageHandler (.NET 8)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8038) - mean (269ms)  : 263, 274
    master - mean (269ms)  : 263, 274

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

    section CallTarget+Inlining+NGEN
    This PR (8038) - mean (825ms)  : 806, 843
    master - mean (823ms)  : 799, 846

Loading

@andrewlock andrewlock marked this pull request as ready for review January 8, 2026 16:25
@andrewlock andrewlock requested review from a team as code owners January 8, 2026 16:25
@andrewlock andrewlock requested review from zacharycmontoya and removed request for a team January 8, 2026 16:25
Copy link
Collaborator

@bouwkast bouwkast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

As a side note, I suspect this explains the "we saw errors about these being null in error tracking but don't understand why" scenarios 😄

I think if I recall correctly this was actually the Activity.Id being null and not the SpanId/TraceId

But there were a lot of weirdness with handling the different versions and formats. I think that if SpanId/TraceId are null for Hierarchical the Id wouldn't, but looking at the current code that is marked nullable too so 🤷

https://github.com/dotnet/dotnet/blob/b0f34d51fccc69fd334253924abd8d6853fad7aa/src/runtime/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs#L96C9-L96C29

@andrewlock
Copy link
Member Author

I think if I recall correctly this was actually the Activity.Id being null and not the SpanId/TraceId

Ah, ok, I actually assumed it was related to the fix in the previous PR where we were assuming that IW3CActivity meant it had a SpanId/TraceId, which isn't the case.

I think that if SpanId/TraceId are null for Hierarchical the Id wouldn't, but looking at the current code that is marked nullable too so 🤷

Yeah, I think as long as you call Start() on the activity, then whichever ID type it is ID shouldn't be null. And given in practice we only see "started" activities, I think the existing nullable annotation should be ok.

That said, we should avoid calling it if possible when using W3C, because it allocates 🤔 I think we currently only use it as a fallback, but will add that as an extra note, just to flag it

andrewlock added a commit that referenced this pull request Jan 9, 2026
## Summary of changes

Update `Activity`-based benchmarks to reduce variability and make
comparisons easier

## Reason for change

The various `Activity`-related benchmarks call the global `Tracer`
instance, so we should make sure to configure it with our default
benchmarking settings (basically disabling background jobs like
telemetry/discovery/remote config) to reduce variation. Also added a
"baseline" job for comparison and a version that uses hiearachical IDs
instead of W3C IDs. Was a prerequisite for a bunch of other work.

## Implementation details

- Configure `Benchmarks.OpenTelemetry.InstrumentedApi` project and
`Benchmarks.Trace/ActivityBenchmark` to setup the global tracer with
Telemetry etc disabled
- Add two extra benchmarks to `Benchmarks.Trace/ActivityBenchmark`
- `StartStopWithChild_Baseline`, which is the same as
`StartStopWithChild` but without the DD integration
- `StartStopWithChild_Hierarchical`, which is the same as
`StartStopWithChild` but uses hierarchical ID format
- Don't run either of these in CI for now (to avoid extra load), just
for local comparisons
- Simplify the benchmark to just do explicit duck typing (which is
closer to what we do normally anyway, and removes a bunch of code)

## Test coverage

Running these benchmarks locally shows the improvements we need to make,
and highlights that we clearly have a bug with hierarchical IDs 😬


| Method | Runtime | Mean | Error | StdDev | Gen0 | Gen1 | Allocated |
| ------------------------------- | -------------------- | ----------: |
----------: | ----------: | -----: | -----: | --------: |
| StartStopWithChild_Baseline | .NET 6.0 | 672.9 ns | 23.23 ns | 66.66
ns | 0.0038 | - | 1.09 KB |
| StartStopWithChild_Hierarchical | .NET 6.0 | 35,478.1 ns | 1,035.96 ns
| 3,021.94 ns | - | - | 10.47 KB |
| StartStopWithChild | .NET 6.0 | 3,681.7 ns | 50.23 ns | 44.53 ns |
0.0153 | - | 4.87 KB |
| StartStopWithChild_Baseline | .NET 8.0 | 570.8 ns | 16.96 ns | 49.22
ns | 0.0029 | - | 1.09 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 32,478.5 ns | 1,215.04 ns
| 3,466.57 ns | - | - | 10.48 KB |
| StartStopWithChild | .NET 8.0 | 3,021.4 ns | 59.24 ns | 163.16 ns |
0.0153 | - | 4.77 KB |
| StartStopWithChild_Baseline | .NET Core 3.1 | 752.6 ns | 15.06 ns |
30.09 ns | 0.0038 | - | 1.19 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 34,170.1 ns | 572.80
ns | 478.31 ns | - | - | 10.75 KB |
| StartStopWithChild | .NET Core 3.1 | 4,905.0 ns | 31.63 ns | 29.58 ns
| 0.0153 | - | 5.05 KB |
| StartStopWithChild_Baseline | .NET Framework 4.7.2 | 770.7 ns | 6.03
ns | 5.64 ns | 0.2012 | - | 1.24 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 37,386.2 ns |
542.73 ns | 453.21 ns | 1.8921 | 0.1221 | 11.77 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 5,884.5 ns | 64.54 ns |
60.37 ns | 0.8621 | - | 5.3 KB |


## Other details

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

Part of a stack working to improve OTel performance

- #8036 👈
- #8037
- #8038
- #8039
- #8040
- #8041
- #8042
@andrewlock andrewlock force-pushed the andrew/otel/nullable-annotations branch from 301a07c to a604513 Compare January 9, 2026 11:30
@andrewlock andrewlock requested review from a team as code owners January 9, 2026 11:30
@andrewlock andrewlock force-pushed the andrew/otel/ActivityKey branch from ebcdc7c to b627664 Compare January 9, 2026 11:30
@pr-commenter
Copy link

pr-commenter bot commented Jan 9, 2026

Benchmarks

Benchmark execution time: 2026-01-09 14:32:56

Comparing candidate commit 246d467 in PR branch andrew/otel/nullable-annotations with baseline commit c865b1b in branch master.

Found 8 performance improvements and 7 performance regressions! Performance is the same for 159 metrics, 12 unstable metrics.

scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild net6.0

  • 🟩 execution_time [-14.849ms; -10.696ms] or [-7.042%; -5.072%]
  • 🟩 throughput [+5831.000op/s; +7943.392op/s] or [+6.022%; +8.203%]

scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild netcoreapp3.1

  • 🟩 throughput [+5248.382op/s; +6632.757op/s] or [+7.409%; +9.364%]

scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net6.0

  • 🟩 execution_time [-20.942ms; -20.329ms] or [-10.276%; -9.975%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472

  • 🟥 execution_time [+23.238ms; +28.418ms] or [+11.834%; +14.472%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1

  • 🟩 execution_time [-33.615ms; -28.088ms] or [-14.834%; -12.395%]

scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net472

  • 🟥 execution_time [+112.990µs; +118.290µs] or [+5.872%; +6.148%]
  • 🟥 throughput [-30.147op/s; -28.782op/s] or [-5.801%; -5.538%]

scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net6.0

  • 🟥 execution_time [+115.231µs; +129.303µs] or [+7.950%; +8.921%]
  • 🟥 throughput [-56.557op/s; -50.715op/s] or [-8.198%; -7.351%]

scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync net6.0

  • 🟩 throughput [+37324.859op/s; +43928.118op/s] or [+7.344%; +8.644%]

scenario:Benchmarks.Trace.HttpClientBenchmark.SendAsync netcoreapp3.1

  • 🟥 execution_time [+16.225ms; +19.286ms] or [+8.804%; +10.465%]

scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1

  • 🟩 execution_time [-36.813ms; -35.641ms] or [-18.210%; -17.631%]

scenario:Benchmarks.Trace.SerilogBenchmark.EnrichedLog netcoreapp3.1

  • 🟩 throughput [+8334.827op/s; +10750.334op/s] or [+5.049%; +6.512%]

scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan net6.0

  • 🟥 execution_time [+13.099ms; +16.350ms] or [+6.495%; +8.106%]

andrewlock added a commit that referenced this pull request Jan 9, 2026
## Summary of changes

Reduce allocation of `ActivityHandlerCommon` by removing `string`
concatenation

## Reason for change

The `ActivityHandlerCommon.ActivityStarted` and `ActivityStopped`
methods need to store and retrieve `Activity` instances from a
`ConcurrentDictionary<>`. Today they're doing that be concatenating the
`Activity`'s `TraceId` and `SpanID`, or by using it's `ID`. All that
concatenation causes a bunch of allocation, so instead introduce a
simple `struct` to use as the key instead

## Implementation details

Introduce `ActivityKey`, which is essentially `internal readonly record
struct ActivityKey(string TraceId, string SpanId`, and use that for all
of the dictionary lookups. Which avoids all the string concatenation
allocations.

## Test coverage

Added some unit tests for `ActivityKey`, by mostly covered by existing
integration tests for correctness. Benchmarks show a significant
improvement over [the previous
results](#8036),
particularly for Hierachical IDs which clearly were buggy


| Method | Runtime | Mean | StdDev | Gen0 | Allocated | Compared to
#8036 |
| ------------------------------- | -------------------- | -------: |
--------: | -----: | --------: | ----------------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 4.217 us | 0.3227 us |
0.0153 | 4.09 KB | -6.38 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 3.413 us | 0.2505 us |
0.0076 | 4.09 KB | -6.39 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 5.676 us | 0.4636 us
| 0.0153 | 4.32 KB | -6.43 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 6.813 us |
0.4969 us | 0.7324 | 4.53 KB | -7.24 KB |
| | | | | | | |
| StartStopWithChild | .NET 6.0 | 4.105 us | 0.2677 us | 0.0153 | 4.3 KB
| -0.57 KB |
| StartStopWithChild | .NET 8.0 | 3.475 us | 0.1570 us | 0.0114 | 4.2 KB
| -0.57 KB |
| StartStopWithChild | .NET Core 3.1 | 5.647 us | 0.3129 us | 0.0153 |
4.48 KB | -0.57 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 6.842 us | 0.2992 us |
0.7629 | 4.69 KB | -0.61 KB |



## Other details

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

Part of a stack working to improve OTel performance

- #8036 
- #8037 👈
- #8038
- #8039
- #8040
- #8041
- #8042
Base automatically changed from andrew/otel/ActivityKey to master January 9, 2026 13:47
@andrewlock andrewlock force-pushed the andrew/otel/nullable-annotations branch from a604513 to 246d467 Compare January 9, 2026 13:53
@github-actions github-actions bot added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Jan 9, 2026
@andrewlock andrewlock enabled auto-merge (squash) January 9, 2026 13:54
@andrewlock andrewlock merged commit b13a5e3 into master Jan 9, 2026
70 of 73 checks passed
@andrewlock andrewlock deleted the andrew/otel/nullable-annotations branch January 9, 2026 17:02
@github-actions github-actions bot added this to the vNext-v3 milestone Jan 9, 2026
andrewlock added a commit that referenced this pull request Jan 9, 2026
## Summary of changes

Avoids allocating a closure in .NET Core if we can avoid it

## Reason for change

.NET Core's `ConcurrentDictionary.GetOrAdd()` method allows providing a
"state" object which we can pass to the `GetOrAdd` method. Using this
method avoids allocating a closure every time the method is hit, and we
can pass the state using a value tuple to avoid additional allocation
there

## Implementation details

`#if`/`#else` to glory 

## Test coverage

Functionality is covered by existing tests, benchmarks show an
incremental improvement over #8037 for .NET Core, as expected:


| Method | Runtime | Mean | StdDev | Gen0 | Allocated | Compared to
#8037 |
| ------------------------------- | -------------------- | -------: |
--------: | -----: | --------: | ----------------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 3.746 us | 0.1128 us |
0.0114 | 3.81 KB | -0.28 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 2.759 us | 0.0374 us |
0.0114 | 3.81 KB | -0.28 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 4.762 us | 0.0584 us
| 0.0153 | 4.04 KB | -0.28 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 5.651 us |
0.0717 us | 0.7324 | 4.54 KB | 0.01 KB (noise) |
| | | | | | | |
| StartStopWithChild | .NET 6.0 | 3.607 us | 0.0508 us | 0.0153 | 4.02
KB | -0.28 KB |
| StartStopWithChild | .NET 8.0 | 2.921 us | 0.0617 us | 0.0114 | 3.91
KB | -0.29 KB |
| StartStopWithChild | .NET Core 3.1 | 4.922 us | 0.0407 us | 0.0153 |
4.2 KB | -0.28 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 6.008 us | 0.0979 us |
0.7629 | 4.69 KB | 0 |


## Other details

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

Part of a stack working to improve OTel performance

- #8036
- #8037
- #8038
- #8039 👈
- #8040
- #8041
- #8042
pablomartinezbernardo pushed a commit that referenced this pull request Jan 10, 2026
## Summary of changes

Reduce allocation of `ActivityHandlerCommon` by removing `string`
concatenation

## Reason for change

The `ActivityHandlerCommon.ActivityStarted` and `ActivityStopped`
methods need to store and retrieve `Activity` instances from a
`ConcurrentDictionary<>`. Today they're doing that be concatenating the
`Activity`'s `TraceId` and `SpanID`, or by using it's `ID`. All that
concatenation causes a bunch of allocation, so instead introduce a
simple `struct` to use as the key instead

## Implementation details

Introduce `ActivityKey`, which is essentially `internal readonly record
struct ActivityKey(string TraceId, string SpanId`, and use that for all
of the dictionary lookups. Which avoids all the string concatenation
allocations.

## Test coverage

Added some unit tests for `ActivityKey`, by mostly covered by existing
integration tests for correctness. Benchmarks show a significant
improvement over [the previous
results](#8036),
particularly for Hierachical IDs which clearly were buggy


| Method | Runtime | Mean | StdDev | Gen0 | Allocated | Compared to
#8036 |
| ------------------------------- | -------------------- | -------: |
--------: | -----: | --------: | ----------------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 4.217 us | 0.3227 us |
0.0153 | 4.09 KB | -6.38 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 3.413 us | 0.2505 us |
0.0076 | 4.09 KB | -6.39 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 5.676 us | 0.4636 us
| 0.0153 | 4.32 KB | -6.43 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 6.813 us |
0.4969 us | 0.7324 | 4.53 KB | -7.24 KB |
| | | | | | | |
| StartStopWithChild | .NET 6.0 | 4.105 us | 0.2677 us | 0.0153 | 4.3 KB
| -0.57 KB |
| StartStopWithChild | .NET 8.0 | 3.475 us | 0.1570 us | 0.0114 | 4.2 KB
| -0.57 KB |
| StartStopWithChild | .NET Core 3.1 | 5.647 us | 0.3129 us | 0.0153 |
4.48 KB | -0.57 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 6.842 us | 0.2992 us |
0.7629 | 4.69 KB | -0.61 KB |



## Other details

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

Part of a stack working to improve OTel performance

- #8036 
- #8037 👈
- #8038
- #8039
- #8040
- #8041
- #8042
andrewlock added a commit that referenced this pull request Jan 12, 2026
## Summary of changes

Various optimizations in the `Activity` handling code to reduce
allocations and execution time

## Reason for change

Some properties are triksy, as they do a bunch of allocation, so we
should avoid them if we can. The changes in here _look_ bigger than they
are diff-wise, I've added comments to aid review.

## Implementation details

- Don't call `ParentId` until we definitely need it.
- This property does a bunch of allocation to generate a "valid" value,
so we should avoid it if we can. This makes the conditions a bit harder
to read, but delays calling `ParentId` until we're sure we don't have
something better already
- Avoid calling `Tracer.Instance.ActiveScope?.Span` until we know we
need it (very minor optimisations but why not 🤷‍♂️)
- Extract `StopActivitySlow` to a separate method, as we _shouldn't_ hit
this now, so should help the JIT out with things like code size etc of
the calling method (conjecture, not tested, but I think it's better from
code understanding PoV too)
- Simplify `ShouldIgnoreByOperationName` which also improves execution
time 10x from ~30us to ~3us.

## Test coverage

Functionally covered by existing tests. Benchmarks compared to #8039
show improvments. Allocations listed below, but execution time is also
improved:

I re-ran the numbers after making the updates, and the benchmarks are
actually better:

<details><summary>Benchmarks for original PR</summary>
<p>


| Method | Runtime | Mean | StdDev | Gen0 | Allocated | Compared to
#8039 |
| ------------------------------- | -------------------- | -------: |
--------: | -----: | --------: | ----------------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 3.410 us | 0.0658 us |
0.0153 | 3.79 KB | -0.02 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 2.582 us | 0.0443 us |
0.0114 | 3.79 KB | -0.02 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 4.272 us | 0.0731 us
| 0.0153 | 4.02 KB | -0.02 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 5.165 us |
0.1245 us | 0.7324 | 4.51 KB | -0.03 KB |
| | | | | | | |
| StartStopWithChild | .NET 6.0 | 3.312 us | 0.0266 us | 0.0153 | 3.78
KB | -0.04 KB |
| StartStopWithChild | .NET 8.0 | 2.648 us | 0.0306 us | 0.0114 | 3.78
KB | -0.13 KB |
| StartStopWithChild | .NET Core 3.1 | 4.344 us | 0.0555 us | 0.0076 |
3.97 KB | -0.23 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 5.234 us | 0.1568 us |
0.7095 | 4.39 KB | -0.30 KB |


</p>
</details> 


| Method | Runtime | Mean | StdDev | Gen0 | Allocated | Alloc Ratio |
| ------------------------------- | -------------------- | -------: |
--------: | -----: | --------: | ----------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 3.770 us | 0.3101 us |
0.0076 | 3.58 KB | -0.23 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 3.039 us | 0.2377 us |
0.0076 | 3.58 KB | -0.23 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 4.490 us | 0.1135 us
| 0.0076 | 3.8 KB | -0.22 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 5.303 us |
0.2469 us | 0.6943 | 4.3 KB | -0.24 KB |
| | | | | | | |
| StartStopWithChild | .NET 6.0 | 3.386 us | 0.0971 us | 0.0114 | 3.59
KB | -0.43 KB |
| StartStopWithChild | .NET 8.0 | 2.661 us | 0.0218 us | 0.0114 | 3.59
KB | -0.32 KB |
| StartStopWithChild | .NET Core 3.1 | 4.540 us | 0.1625 us | 0.0076 |
3.78 KB | -0.42 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 5.563 us | 0.1946 us |
0.6790 | 4.2 KB | -0.49 KB |


## Other details

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

Part of a stack working to improve OTel performance

- #8036
- #8037
- #8038
- #8039
- #8040 👈
- #8041
- #8042
andrewlock added a commit that referenced this pull request Jan 13, 2026
## Summary of changes

Update code that enumerates properties of `IActivity` duck types to use
allocation-free enumeration

## Reason for change

Our duck types all use the public APIs of `Activity` to grab tags,
events, and links. These types are generally implemented internally as
custom linked list objects, but are exposed as `IEnumerable<>` types.
The internal types implement a `struct`-based enumerator, but as the
types aren't exposed directly, the compiler can't use those and ends up
allocating. That's... annoying.

To work around that, this adds a "hack" that OTel were [actually using
themselves](https://github.com/open-telemetry/opentelemetry-dotnet/blob/73bff75ef653f81fe6877299435b21131be36dc0/src/OpenTelemetry/Internal/EnumerationHelper.cs#L58)
(and [which has been discussed
elsewhere](https://www.macrosssoftware.com/2020/07/13/enumerator-performance-surprises/)),
of building a dynamic method that finds the `GetEnumerator` type and
uses that. We can't use it everywhere because we don't have the types
available (e.g. links and events), but we _can_ use it for tags. There's
essentially a trade-off of startup time vs runtime allocation, which is
worth it in this case IMO.

Additionally, the `Activity` implementation uses an empty array as the
return value when there _are_ no tags/links/events, so we can avoid the
enumerator allocation in those cases by specifically looking for an
empty array and bailing if so.

## Implementation details

I chose to add the `EnumerationHelper` as an explicitly
`Activity`-related thing for now, but we can consider exposing this more
widely if we get confidence.

I also avoided the concurrent dictionary approach that is the "default"
for this, and instead added an additional helper to read a static field
that we populate once. We have to wait until we have a "real" activity
to do the population, so that we get the right type, which is a bit
annoying, but it works...

## Test coverage

Added a couple of unit tests for the result and confirmed it works as
expected. Benchmarks vs #8040 shw a nice allocation improvement:


| Method | Runtime | Mean | StdDev | Gen0 | Gen1 | Allocated | Compared
to #8040 |
| ------------------------------- | -------------------- | ---------: |
--------: | -----: | -----: | --------: | ----------------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 3,248.6 ns | 28.71 ns |
0.0114 | 0.0038 | 3.6 KB | -0.19 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 2,651.1 ns | 80.80 ns |
0.0114 | - | 3.6 KB | -0.19 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 4,482.2 ns | 263.51
ns | 0.0153 | - | 3.83 KB | -0.19 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 4,915.8 ns |
63.81 ns | 0.7019 | - | 4.33 KB | -0.18 KB |
| StartStopWithChild | .NET 6.0 | 3,176.8 ns | 37.73 ns | 0.0114 | - |
3.59 KB | -0.19 KB |
| StartStopWithChild | .NET 8.0 | 2,598.0 ns | 40.05 ns | 0.0114 | - |
3.59 KB | -0.19 KB |
| StartStopWithChild | .NET Core 3.1 | 4,260.7 ns | 59.98 ns | 0.0076 |
- | 3.78 KB | -0.19 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 5,097.7 ns | 46.98 ns |
0.6790 | - | 4.2 KB | -0.19 KB |

## Other details

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

Part of a stack working to improve OTel performance

- #8036
- #8037
- #8038
- #8039
- #8040
- #8041 👈
- #8042

---------

Co-authored-by: Lucas Pimentel <lucas.pimentel@datadoghq.com>
andrewlock added a commit that referenced this pull request Jan 13, 2026
…8042)

## Summary of changes

Improve performance around the "population" of tags from an `Activity`
into a `Span`

## Reason for change

Currently we do a _lot_ of allocation in the
`OtlpHelpers.AgentConvertSpan` method. The `OpenTelemetryTags` object
also has a lot of properties on it that we never set, and are only used
for _reading_ tags in the `OperationMapper`, even though many may never
even be read. This increases the size of the tags object.

This PR aims to do various optimizations around the "close activity"
paths:
- Reduce the size of the `OpenTelemetryTags` object by removing
properties only used by `OperationMapper`
- Add properties to `OpenTelemetryTags` for values that we explictly set
in `OtlpHelpers.AgentConvertSpan`

## Implementation details

- Currently we're always creating the `OpenTelemetryTags`, even though
we _may_ throw it away if the activity is ignored, so that's an easy
win.
- I think we can assume that the tags object passed to
`AgentConvertSpan` is only ever `OpenTelemetryTags` based on the call
sites, so we can interact with the tags object directly where posisble.
- "Simplify" some of the code paths by avoiding `span.SetTag` and
favouring `tags.SetTag` when we know it's not a "special" tag.
- Inline a few method calls in places where they will often not be
called, at the expense of some clarity
- Refactor `OperationMapper` to use the `GetTag()` API, and cache the
values at various points. The resulting code is uglier, but overall
means we reduce ~100 bytes off every span, so I think it's worth it

## Test coverage

Functionality is covered by existing, this is just a refactoring, and
there's good tests for `OperationMapper` currently. Benchmarks vs
#8041 show a nice
allocation improvement, at the expense of slower execution (this was
local though, so not 100% on that, will see what the CI results show).
If there is a significant slow down, I'll try to isolate it, but I think
the allocation improvments are probably worth it either way


| Method | Runtime | Mean | StdDev | Gen0 | Allocated | Compared to
#8041 |
| ------------------------------- | -------------------- | -------: |
--------: | -----: | --------: | ----------------: |
| StartStopWithChild_Hierarchical | .NET 6.0 | 4.438 us | 0.7347 us |
0.0076 | 3.2 KB | -0.40 KB |
| StartStopWithChild_Hierarchical | .NET 8.0 | 3.216 us | 0.4260 us |
0.0076 | 3.2 KB | -0.40 KB |
| StartStopWithChild_Hierarchical | .NET Core 3.1 | 5.293 us | 0.8316 us
| 0.0076 | 3.42 KB | -0.41 KB |
| StartStopWithChild_Hierarchical | .NET Framework 4.7.2 | 5.735 us |
0.5592 us | 0.6332 | 3.92 KB | -0.41 KB |
| | | | | | | |
| StartStopWithChild | .NET 6.0 | 3.848 us | 0.5325 us | 0.0076 | 3.19
KB | -0.40 KB |
| StartStopWithChild | .NET 8.0 | 3.080 us | 0.2698 us | 0.0076 | 3.19
KB | -0.40 KB |
| StartStopWithChild | .NET Core 3.1 | 5.094 us | 0.4982 us | 0.0076 |
3.38 KB | -0.40 KB |
| StartStopWithChild | .NET Framework 4.7.2 | 6.172 us | 0.5948 us |
0.6104 | 3.79 KB | -0.41 KB |


## Other details

Added a couple of extra tests that were missing from the previous PR in
the stack too

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

Part of a stack working to improve OTel performance

- #8036
- #8037
- #8038
- #8039
- #8040
- #8041
- #8042 👈
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:opentelemetry OpenTelemetry support area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:reliability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants