Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tracer] fix exception when closing a Span with null ServiceName #6748

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lucaspimentel
Copy link
Member

@lucaspimentel lucaspimentel commented Mar 6, 2025

Summary of changes

If Span.ServiceName is set to null after a span is create, a NullReferenceException is thrown when closing the span.

Reason for change

Prevent the exception.

Implementation details

Add two different null checks to be safe:

  • TraceContext.CloseSpan()
  • ExtraServicesProvider.AddService()

Test coverage

Added test for both TraceContext and ExtraServicesProvider. Confirmed they failed before, and that each one is resolved separately with each added null check.

Other details

n/a

@lucaspimentel lucaspimentel added type:bug area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:remote-config labels Mar 6, 2025
@lucaspimentel lucaspimentel requested a review from a team as a code owner March 6, 2025 18:24
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Mar 6, 2025

Datadog Report

Branch report: lpimentel/fix-null-service-name-nre
Commit report: d2c31d5
Test service: dd-trace-dotnet

❌ 1 Failed (0 Known Flaky), 248485 Passed, 2376 Skipped, 19h 39m 40.65s Total Time

❌ Failed Tests (1)

  • SubmitsTraces - Datadog.Trace.Security.IntegrationTests.IAST.GrpcDotNet.GrpcDotNetTests - Details

    Expand for error
     
     [
       {
         TraceId: Id_1,
         SpanId: Id_2,
         Name: aspnet_core.request,
         Resource: POST /greet.tester.greeter/streamingbothways,
         Service: Samples.GrpcDotNet,
         Type: web,
         ParentId: Id_3,
     ...
    

@andrewlock
Copy link
Member

andrewlock commented Mar 6, 2025

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 (6748) - mean (70ms)  : 66, 73
     .   : milestone, 70,
    master - mean (70ms)  : 66, 73
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6748) - mean (1,030ms)  : 954, 1107
     .   : milestone, 1030,
    master - mean (1,004ms)  : 982, 1026
     .   : milestone, 1004,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6748) - mean (104ms)  : 101, 106
     .   : milestone, 104,
    master - mean (102ms)  : 100, 105
     .   : milestone, 102,

    section CallTarget+Inlining+NGEN
    This PR (6748) - mean (693ms)  : 675, 710
     .   : milestone, 693,
    master - mean (688ms)  : 670, 706
     .   : milestone, 688,

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

    section CallTarget+Inlining+NGEN
    This PR (6748) - mean (644ms)  : 628, 661
     .   : milestone, 644,
    master - mean (644ms)  : 625, 662
     .   : milestone, 644,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6748) - mean (191ms)  : 187, 195
     .   : milestone, 191,
    master - mean (190ms)  : 185, 195
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6748) - mean (1,111ms)  : 1083, 1138
     .   : milestone, 1111,
    master - mean (1,105ms)  : 1081, 1130
     .   : milestone, 1105,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6748) - mean (270ms)  : 266, 274
     .   : milestone, 270,
    master - mean (269ms)  : 266, 272
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (6748) - mean (879ms)  : 846, 913
     .   : milestone, 879,
    master - mean (874ms)  : 842, 905
     .   : milestone, 874,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6748) - mean (263ms)  : 258, 267
     .   : milestone, 263,
    master - mean (261ms)  : 257, 265
     .   : milestone, 261,

    section CallTarget+Inlining+NGEN
    This PR (6748) - mean (856ms)  : 819, 893
     .   : milestone, 856,
    master - mean (856ms)  : 827, 886
     .   : milestone, 856,

Loading

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!

@andrewlock
Copy link
Member

andrewlock commented Mar 6, 2025

Benchmarks Report for tracer 🐌

Benchmarks for #6748 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.133
  • 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.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 8.56μs 48.8ns 384ns 0.0123 0.00821 0 5.61 KB
master StartStopWithChild netcoreapp3.1 10.6μs 61ns 510ns 0.0202 0.0101 0 5.8 KB
master StartStopWithChild net472 16.3μs 37.6ns 146ns 1.04 0.309 0.0976 6.22 KB
#6748 StartStopWithChild net6.0 8.37μs 46.9ns 367ns 0.0162 0.00808 0 5.62 KB
#6748 StartStopWithChild netcoreapp3.1 10.4μs 56.9ns 322ns 0.0261 0.0104 0 5.81 KB
#6748 StartStopWithChild net472 16.4μs 59ns 228ns 1.04 0.31 0.098 6.2 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 481μs 374ns 1.4μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 653μs 409ns 1.53μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 834μs 623ns 2.41μs 0.417 0 0 3.3 KB
#6748 WriteAndFlushEnrichedTraces net6.0 511μs 574ns 2.22μs 0 0 0 2.7 KB
#6748 WriteAndFlushEnrichedTraces netcoreapp3.1 690μs 450ns 1.74μs 0 0 0 2.7 KB
#6748 WriteAndFlushEnrichedTraces net472 853μs 657ns 2.55μs 0.428 0 0 3.3 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 133μs 336ns 1.3μs 0.198 0 0 14.47 KB
master SendRequest netcoreapp3.1 149μs 241ns 935ns 0.149 0 0 17.27 KB
master SendRequest net472 0.000394ns 0.000203ns 0.00076ns 0 0 0 0 b
#6748 SendRequest net6.0 131μs 401ns 1.55μs 0.196 0 0 14.47 KB
#6748 SendRequest netcoreapp3.1 148μs 362ns 1.4μs 0.222 0 0 17.27 KB
#6748 SendRequest net472 0.000549ns 0.000259ns 0.001ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #6748

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.74 KB 41.45 KB -292 B -0.70%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 575μs 2.56μs 9.21μs 0.541 0 0 41.74 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 676μs 3.58μs 18.9μs 0.336 0 0 41.85 KB
master WriteAndFlushEnrichedTraces net472 854μs 3.98μs 16.4μs 8.5 2.55 0.425 53.25 KB
#6748 WriteAndFlushEnrichedTraces net6.0 550μs 2.59μs 10.3μs 0.561 0 0 41.45 KB
#6748 WriteAndFlushEnrichedTraces netcoreapp3.1 687μs 3.64μs 22.4μs 0.331 0 0 41.75 KB
#6748 WriteAndFlushEnrichedTraces net472 868μs 4.23μs 18.5μs 8.39 2.52 0.419 53.3 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.34μs 1.01ns 3.9ns 0.014 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.85μs 2.19ns 8.21ns 0.0136 0 0 1.02 KB
master ExecuteNonQuery net472 2.12μs 2.2ns 8.5ns 0.157 0.00105 0 987 B
#6748 ExecuteNonQuery net6.0 1.35μs 1.8ns 6.99ns 0.0142 0 0 1.02 KB
#6748 ExecuteNonQuery netcoreapp3.1 1.75μs 1.15ns 4.45ns 0.0139 0 0 1.02 KB
#6748 ExecuteNonQuery net472 2.02μs 2.08ns 7.79ns 0.156 0.00101 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.18μs 0.658ns 2.55ns 0.0139 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.66μs 1.04ns 3.91ns 0.0133 0 0 976 B
master CallElasticsearch net472 2.54μs 1.22ns 4.73ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.35μs 1.14ns 4.41ns 0.0134 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.7μs 1.21ns 4.69ns 0.0136 0 0 1.02 KB
master CallElasticsearchAsync net472 2.53μs 2.01ns 7.79ns 0.166 0 0 1.05 KB
#6748 CallElasticsearch net6.0 1.3μs 0.718ns 2.49ns 0.0137 0 0 976 B
#6748 CallElasticsearch netcoreapp3.1 1.59μs 0.74ns 2.77ns 0.0126 0 0 976 B
#6748 CallElasticsearch net472 2.57μs 1.51ns 5.84ns 0.158 0 0 995 B
#6748 CallElasticsearchAsync net6.0 1.22μs 0.449ns 1.62ns 0.0134 0 0 952 B
#6748 CallElasticsearchAsync netcoreapp3.1 1.63μs 1.2ns 4.67ns 0.0138 0 0 1.02 KB
#6748 CallElasticsearchAsync net472 2.59μs 1.22ns 4.58ns 0.167 0 0 1.05 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.32μs 0.397ns 1.49ns 0.0133 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.63μs 0.893ns 3.34ns 0.0124 0 0 952 B
master ExecuteAsync net472 1.86μs 0.524ns 2.03ns 0.145 0 0 915 B
#6748 ExecuteAsync net6.0 1.36μs 0.428ns 1.66ns 0.0136 0 0 952 B
#6748 ExecuteAsync netcoreapp3.1 1.58μs 0.324ns 1.25ns 0.0127 0 0 952 B
#6748 ExecuteAsync net472 1.84μs 0.455ns 1.64ns 0.145 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.37μs 2.94ns 11.4ns 0.0326 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.36μs 3.26ns 12.6ns 0.0374 0 0 2.85 KB
master SendAsync net472 7.55μs 1.91ns 7.38ns 0.493 0 0 3.12 KB
#6748 SendAsync net6.0 4.37μs 1.3ns 5.02ns 0.0309 0 0 2.31 KB
#6748 SendAsync netcoreapp3.1 5.38μs 2.91ns 11.3ns 0.0374 0 0 2.85 KB
#6748 SendAsync net472 7.5μs 2.08ns 8.06ns 0.496 0 0 3.12 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.55μs 2.25ns 8.72ns 0.0233 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.09μs 0.837ns 3.13ns 0.0219 0 0 1.64 KB
master EnrichedLog net472 2.53μs 0.775ns 3ns 0.249 0 0 1.57 KB
#6748 EnrichedLog net6.0 1.54μs 1.55ns 5.8ns 0.0232 0 0 1.64 KB
#6748 EnrichedLog netcoreapp3.1 2.19μs 0.774ns 2.79ns 0.0221 0 0 1.64 KB
#6748 EnrichedLog net472 2.57μs 1.12ns 4.33ns 0.25 0 0 1.57 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 113μs 184ns 712ns 0.0561 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 115μs 109ns 423ns 0.0577 0 0 4.28 KB
master EnrichedLog net472 149μs 141ns 548ns 0.674 0.225 0 4.46 KB
#6748 EnrichedLog net6.0 112μs 110ns 428ns 0.0561 0 0 4.28 KB
#6748 EnrichedLog netcoreapp3.1 116μs 147ns 571ns 0.0578 0 0 4.28 KB
#6748 EnrichedLog net472 152μs 237ns 919ns 0.681 0.227 0 4.46 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 3.06μs 1.1ns 4.26ns 0.0305 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.21μs 1.62ns 6.29ns 0.0295 0 0 2.2 KB
master EnrichedLog net472 4.89μs 0.819ns 3.07ns 0.32 0 0 2.02 KB
#6748 EnrichedLog net6.0 3.18μs 0.833ns 3.12ns 0.0302 0 0 2.2 KB
#6748 EnrichedLog netcoreapp3.1 4.13μs 1.72ns 6.65ns 0.0289 0 0 2.2 KB
#6748 EnrichedLog net472 4.96μs 1.62ns 6.26ns 0.32 0 0 2.02 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.4μs 0.793ns 3.07ns 0.0161 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.79μs 0.931ns 3.61ns 0.0152 0 0 1.14 KB
master SendReceive net472 2.05μs 0.393ns 1.47ns 0.183 0 0 1.16 KB
#6748 SendReceive net6.0 1.4μs 0.601ns 2.25ns 0.0161 0 0 1.14 KB
#6748 SendReceive netcoreapp3.1 1.77μs 0.673ns 2.52ns 0.0151 0 0 1.14 KB
#6748 SendReceive net472 2.16μs 1.47ns 5.29ns 0.183 0 0 1.16 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.64μs 1.03ns 3.87ns 0.0224 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.86μs 1.33ns 5.14ns 0.0212 0 0 1.65 KB
master EnrichedLog net472 4.24μs 1.75ns 6.53ns 0.323 0 0 2.04 KB
#6748 EnrichedLog net6.0 2.71μs 1.96ns 7.59ns 0.0217 0 0 1.6 KB
#6748 EnrichedLog netcoreapp3.1 3.86μs 1.67ns 6.01ns 0.0212 0 0 1.65 KB
#6748 EnrichedLog net472 4.09μs 2.59ns 9.7ns 0.322 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6748

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.133 752.36 664.07

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 441ns 0.796ns 3.08ns 0.0081 0 0 576 B
master StartFinishSpan netcoreapp3.1 599ns 0.307ns 1.19ns 0.00776 0 0 576 B
master StartFinishSpan net472 659ns 0.431ns 1.67ns 0.0915 0 0 578 B
master StartFinishScope net6.0 530ns 0.2ns 0.774ns 0.00981 0 0 696 B
master StartFinishScope netcoreapp3.1 752ns 0.46ns 1.78ns 0.00938 0 0 696 B
master StartFinishScope net472 784ns 0.44ns 1.7ns 0.104 0 0 658 B
#6748 StartFinishSpan net6.0 410ns 0.152ns 0.568ns 0.00814 0 0 576 B
#6748 StartFinishSpan netcoreapp3.1 589ns 0.583ns 2.1ns 0.00781 0 0 576 B
#6748 StartFinishSpan net472 638ns 0.116ns 0.433ns 0.0916 0 0 578 B
#6748 StartFinishScope net6.0 486ns 0.247ns 0.955ns 0.0097 0 0 696 B
#6748 StartFinishScope netcoreapp3.1 664ns 0.293ns 1.13ns 0.00928 0 0 696 B
#6748 StartFinishScope net472 831ns 0.254ns 0.952ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 644ns 0.226ns 0.876ns 0.00964 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 934ns 0.627ns 2.26ns 0.00913 0 0 696 B
master RunOnMethodBegin net472 1.07μs 0.244ns 0.915ns 0.105 0 0 658 B
#6748 RunOnMethodBegin net6.0 653ns 0.174ns 0.626ns 0.00971 0 0 696 B
#6748 RunOnMethodBegin netcoreapp3.1 922ns 0.437ns 1.58ns 0.00933 0 0 696 B
#6748 RunOnMethodBegin net472 1.11μs 0.604ns 2.34ns 0.104 0 0 658 B

@lucaspimentel lucaspimentel requested a review from a team March 6, 2025 22:21
@lucaspimentel lucaspimentel changed the title [tracer] fix NRE when Span.ServiceName is null [tracer] fix exception when closing a Span with null ServiceName Mar 7, 2025
@lucaspimentel lucaspimentel force-pushed the lpimentel/fix-null-service-name-nre branch from 3c7e111 to d2c31d5 Compare March 7, 2025 15:17
@lucaspimentel lucaspimentel marked this pull request as draft March 10, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:remote-config area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants