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.

Sign up for GitHub

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

Jump to bottom

[Dynamic Instrumentation] Remove async void in SymbolsUploader #5155

Merged
merged 2 commits into from
Mar 13, 2024
Merged

[Dynamic Instrumentation] Remove async void in SymbolsUploader #5155

merged 2 commits into from
Mar 13, 2024

Conversation

kevingosse
Copy link
Collaborator

Summary of changes

Change async void method to a simple fire-and-forget.

Reason for change

When an exception is thrown in an async void method, it's propagated to the synchronization context and it crashes the process. The caller can't await it anyway, so keeping it brings no benefits.

@kevingosse kevingosse requested a review from a team as a code owner February 6, 2024 17:37
@github-actions github-actions bot added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:debugger labels Feb 6, 2024
tonyredondo
tonyredondo reviewed Feb 6, 2024
View reviewed changes
tracer/src/Datadog.Trace/Debugger/Symbols/SymbolsUploader.cs
@@ -136,9 +136,9 @@ private void UnRegisterToAssemblyLoadEvent()
AppDomain.CurrentDomain.AssemblyLoad -= CurrentDomain_AssemblyLoad;
}

private async void CurrentDomain_AssemblyLoad(object? sender, AssemblyLoadEventArgs args)
Copy link
Member

Choose a reason for hiding this comment

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

we need an analyzer to detect this pattern.

Copy link
Member

Choose a reason for hiding this comment

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

Worth a look 👀 https://www.nuget.org/packages/Microsoft.VisualStudio.Threading.Analyzers#readme-body-tab

Copy link
Member

Choose a reason for hiding this comment

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

ok, looked at it, and it complains about a lot of things 😂 https://github.com/DataDog/dd-trace-dotnet/tree/andrew/ci/add-async-analyzer

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Feb 6, 2024 •
edited
Loading

Datadog Report

Branch report: kevin/async_void
Commit report: 52274ca
Test service: dd-trace-dotnet

❌ 1 Failed (0 Known Flaky), 308177 Passed, 2021 Skipped, 40m 49.46s Wall Time

❌ Failed Tests (1)

  • SubmitsTraces - Datadog.Trace.Security.IntegrationTests.Iast.DeduplicationTests - Details

    Expand for error 
     Expected exit code: 0, actual exit code: -1073740791.

@andrewlock
Copy link
Member

andrewlock commented Feb 6, 2024 •
edited
Loading

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 (5155) - mean (76ms)  : 63, 89
     .   : milestone, 76,
    master - mean (75ms)  : 70, 80
     .   : milestone, 75,

    section CallTarget+Inlining+NGEN
    This PR (5155) - mean (985ms)  : 965, 1005
     .   : milestone, 985,
    master - mean (988ms)  : 968, 1008
     .   : milestone, 988,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5155) - mean (111ms)  : 108, 115
     .   : milestone, 111,
    master - mean (113ms)  : 109, 116
     .   : milestone, 113,

    section CallTarget+Inlining+NGEN
    This PR (5155) - mean (718ms)  : 692, 744
     .   : milestone, 718,
    master - mean (723ms)  : 703, 742
     .   : milestone, 723,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5155) - mean (94ms)  : 91, 96
     .   : milestone, 94,
    master - mean (97ms)  : 93, 100
     .   : milestone, 97,

    section CallTarget+Inlining+NGEN
    This PR (5155) - mean (676ms)  : 649, 703
     .   : milestone, 676,
    master - mean (678ms)  : 653, 702
     .   : milestone, 678,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5155) - mean (188ms)  : 185, 191
     .   : milestone, 188,
    master - mean (187ms)  : 184, 190
     .   : milestone, 187,

    section CallTarget+Inlining+NGEN
    This PR (5155) - mean (1,060ms)  : 1041, 1080
     .   : milestone, 1060,
    master - mean (1,058ms)  : 1037, 1079
     .   : milestone, 1058,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5155) - mean (272ms)  : 266, 277
     .   : milestone, 272,
    master - mean (270ms)  : 265, 275
     .   : milestone, 270,

    section CallTarget+Inlining+NGEN
    This PR (5155) - mean (1,061ms)  : 1041, 1082
     .   : milestone, 1061,
    master - mean (1,058ms)  : 1030, 1086
     .   : milestone, 1058,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5155) - mean (260ms)  : 256, 264
     .   : milestone, 260,
    master - mean (259ms)  : 254, 264
     .   : milestone, 259,

    section CallTarget+Inlining+NGEN
    This PR (5155) - mean (994ms)  : 971, 1018
     .   : milestone, 994,
    master - mean (991ms)  : 966, 1016
     .   : milestone, 991,
Loading

@andrewlock
Copy link
Member

andrewlock commented Feb 6, 2024 •
edited
Loading

Throughput/Crank Report:zap:

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 (5155) (10.819M)   : 0, 10819334
    master (10.762M)   : 0, 10761999
    benchmarks/2.9.0 (11.160M)   : 0, 11159568

    section Automatic
    This PR (5155) (7.518M)   : 0, 7518113
    master (5.722M)   : 0, 5722409
    benchmarks/2.9.0 (8.094M)   : 0, 8093917

    section Trace stats
    This PR (5155) (7.868M)   : 0, 7868325
    master (7.764M)   : 0, 7764178

    section Manual
    This PR (5155) (9.521M)   : 0, 9520862
    master (9.405M)   : 0, 9404543

    section Manual + Automatic
    This PR (5155) (7.073M)   : 0, 7073054
    master (7.093M)   : 0, 7092517

    section Version Conflict
    This PR (5155) (6.360M)   : 0, 6360494
    master (6.325M)   : 0, 6325056
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5155) (9.400M)   : 0, 9399575
    master (9.555M)   : 0, 9555430
    benchmarks/2.9.0 (9.576M)   : 0, 9576035

    section Automatic
    This PR (5155) (6.780M)   : 0, 6779698
    master (6.605M)   : 0, 6604545

    section Trace stats
    This PR (5155) (6.999M)   : 0, 6999230
    master (6.788M)   : 0, 6788095

    section Manual
    This PR (5155) (8.409M)   : 0, 8409070
    master (8.511M)   : 0, 8510867

    section Manual + Automatic
    This PR (5155) (6.289M)   : 0, 6289498
    master (6.227M)   : 0, 6227413

    section Version Conflict
    This PR (5155) (5.696M)   : 0, 5696268
    master (5.790M)   : 0, 5789961
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5155) (10.222M)   : 0, 10222311
    master (8.718M)   : 0, 8717919
    benchmarks/2.9.0 (9.627M)   : 0, 9627029

    section Automatic
    This PR (5155) (7.208M)   : 0, 7207803
    master (6.273M)   : 0, 6273234
    benchmarks/2.9.0 (7.280M)   : 0, 7279862

    section Trace stats
    This PR (5155) (7.483M)   : 0, 7482778
    master (7.466M)   : 0, 7465801

    section Manual
    This PR (5155) (8.871M)   : 0, 8870872
    master (8.690M)   : 0, 8689661

    section Manual + Automatic
    This PR (5155) (6.917M)   : 0, 6917248
    master (6.763M)   : 0, 6762657

    section Version Conflict
    This PR (5155) (6.176M)   : 0, 6176245
    master (6.145M)   : 0, 6145260
Loading 
gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    master (7.405M)   : 0, 7405418
    benchmarks/2.9.0 (7.799M)   : 0, 7798783

    section No attack
    master (1.822M)   : 0, 1821685
    benchmarks/2.9.0 (3.256M)   : 0, 3255670

    section Attack
    master (1.465M)   : 0, 1465312
    benchmarks/2.9.0 (2.530M)   : 0, 2529607

    section Blocking
    master (3.147M)   : 0, 3147045

    section IAST default
    master (6.387M)   : 0, 6386517

    section IAST full
    master (5.602M)   : 0, 5602399

    section Base vuln
    master (0.934M)   : 0, 934358

    section IAST vuln
    master (0.868M)   : 0, 868269
Loading

@andrewlock
Copy link
Member

Benchmarks Report 🐌

Benchmarks for #5155 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.119
  • 3 benchmarks are slower, with geometric mean 1.181
  • 3 benchmarks have more 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.69μs 47ns 270ns 0.0217 0.00866 0 7.49 KB
master StartStopWithChild netcoreapp3.1 10.6μs 55.2ns 259ns 0.0265 0.0106 0 7.58 KB
master StartStopWithChild net472 17.1μs 47.2ns 183ns 1.33 0.342 0.111 7.95 KB
#5155 StartStopWithChild net6.0 8.73μs 46.5ns 271ns 0.0374 0.0166 0 7.48 KB
#5155 StartStopWithChild netcoreapp3.1 10.3μs 38.4ns 133ns 0.0298 0.00993 0 7.59 KB
#5155 StartStopWithChild net472 17.1μs 45ns 168ns 1.35 0.356 0.122 7.97 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 446μs 222ns 800ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 636μs 116ns 417ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 774μs 307ns 1.15μs 0.386 0 0 3.3 KB
#5155 WriteAndFlushEnrichedTraces net6.0 448μs 333ns 1.29μs 0 0 0 2.7 KB
#5155 WriteAndFlushEnrichedTraces netcoreapp3.1 607μs 109ns 408ns 0 0 0 2.7 KB
#5155 WriteAndFlushEnrichedTraces net472 789μs 268ns 965ns 0.393 0 0 3.3 KB
Benchmarks.Trace.Asm.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 38.8μs 10.5ns 39.2ns 0.0194 0 0 2.36 KB
master AllCycleSimpleBody netcoreapp3.1 42.6μs 51.7ns 200ns 0.0211 0 0 2.34 KB
master AllCycleSimpleBody net472 44.7μs 12.1ns 43.6ns 0.38 0 0 2.41 KB
master AllCycleMoreComplexBody net6.0 201μs 93.4ns 349ns 0.1 0 0 9.84 KB
master AllCycleMoreComplexBody netcoreapp3.1 214μs 248ns 961ns 0.106 0 0 9.73 KB
master AllCycleMoreComplexBody net472 230μs 89.1ns 333ns 1.5 0 0 9.91 KB
master ObjectExtractorSimpleBody net6.0 141ns 0.12ns 0.464ns 0.00396 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 207ns 0.193ns 0.746ns 0.00375 0 0 272 B
master ObjectExtractorSimpleBody net472 167ns 0.118ns 0.456ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 3.02μs 1.21ns 4.52ns 0.0531 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 4.18μs 1.8ns 6.96ns 0.0501 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 3.75μs 3.94ns 15.3ns 0.603 0.00563 0 3.8 KB
#5155 AllCycleSimpleBody net6.0 39.1μs 34.9ns 135ns 0.0196 0 0 2.36 KB
#5155 AllCycleSimpleBody netcoreapp3.1 42.2μs 37.6ns 146ns 0.0211 0 0 2.34 KB
#5155 AllCycleSimpleBody net472 44.9μs 8.08ns 31.3ns 0.361 0 0 2.41 KB
#5155 AllCycleMoreComplexBody net6.0 205μs 51.3ns 185ns 0.101 0 0 9.84 KB
#5155 AllCycleMoreComplexBody netcoreapp3.1 219μs 275ns 1.03μs 0.108 0 0 9.73 KB
#5155 AllCycleMoreComplexBody net472 229μs 49.3ns 184ns 1.49 0 0 9.91 KB
#5155 ObjectExtractorSimpleBody net6.0 142ns 0.0834ns 0.312ns 0.00393 0 0 280 B
#5155 ObjectExtractorSimpleBody netcoreapp3.1 227ns 0.282ns 1.05ns 0.00365 0 0 272 B
#5155 ObjectExtractorSimpleBody net472 177ns 0.264ns 1.02ns 0.0446 0 0 281 B
#5155 ObjectExtractorMoreComplexBody net6.0 2.97μs 6.33ns 22.8ns 0.0531 0 0 3.78 KB
#5155 ObjectExtractorMoreComplexBody netcoreapp3.1 4.07μs 1.36ns 5.28ns 0.0516 0 0 3.69 KB
#5155 ObjectExtractorMoreComplexBody net472 3.79μs 3.21ns 12.4ns 0.602 0.00564 0 3.8 KB
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunWaf(args=NestedMap (10)) net6.0 53.9μs 49ns 177ns 0.215 0 0 16.66 KB
master RunWaf(args=NestedMap (10)) netcoreapp3.1 72.8μs 177ns 663ns 0.216 0 0 16.65 KB
master RunWaf(args=NestedMap (10)) net472 97.1μs 32.8ns 114ns 2.62 0.0972 0 16.73 KB
master RunWafTwice(args=NestedMap (10)) net6.0 57.8μs 27.6ns 95.7ns 0.22 0 0 17.33 KB
master RunWafTwice(args=NestedMap (10)) netcoreapp3.1 75.8μs 418ns 2.58μs 0.231 0 0 17.31 KB
master RunWafTwice(args=NestedMap (10)) net472 106μs 47.8ns 172ns 2.74 0.105 0 17.42 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 104μs 62.3ns 241ns 0.256 0 0 19.49 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 122μs 386ns 1.5μs 0.235 0 0 19.44 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net472 149μs 49.5ns 179ns 3.11 0.148 0 19.72 KB
master RunWaf(args=NestedMap (100)) net6.0 101μs 25.4ns 141ns 0.453 0 0 33.35 KB
master RunWaf(args=NestedMap (100)) netcoreapp3.1 136μs 354ns 1.37μs 0.456 0 0 33.92 KB
master RunWaf(args=NestedMap (100)) net472 193μs 958ns 3.95μs 5.37 0.377 0 34.26 KB
master RunWafTwice(args=NestedMap (100)) net6.0 113μs 32.8ns 114ns 0.473 0 0 34.02 KB
master RunWafTwice(args=NestedMap (100)) netcoreapp3.1 142μs 775ns 4.1μs 0.417 0 0 34.58 KB
master RunWafTwice(args=NestedMap (100)) net472 204μs 502ns 1.95μs 5.51 0.394 0 34.96 KB
master RunWafWithAttack(args=Neste(...)tack) [23]) net6.0 153μs 76.5ns 296ns 0.456 0 0 36.18 KB
master RunWafWithAttack(args=Neste(...)tack) [23]) netcoreapp3.1 191μs 885ns 3.54μs 0.485 0 0 36.71 KB
master RunWafWithAttack(args=Neste(...)tack) [23]) net472 245μs 101ns 376ns 5.87 0.489 0 37.26 KB
master RunWaf(args=NestedMap (20)) net6.0 106μs 30.6ns 114ns 0.422 0 0 32.78 KB
master RunWaf(args=NestedMap (20)) netcoreapp3.1 135μs 680ns 3.19μs 0.391 0 0 32.89 KB
master RunWaf(args=NestedMap (20)) net472 188μs 87.5ns 339ns 5.26 0.376 0 33.23 KB
master RunWafTwice(args=NestedMap (20)) net6.0 107μs 41.2ns 143ns 0.415 0 0 33.45 KB
master RunWafTwice(args=NestedMap (20)) netcoreapp3.1 141μs 740ns 3.7μs 0.402 0 0 33.55 KB
master RunWafTwice(args=NestedMap (20)) net472 202μs 700ns 2.71μs 5.35 0.396 0 33.92 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 161μs 910ns 6.17μs 0.464 0 0 35.61 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 190μs 271ns 1.05μs 0.471 0 0 35.68 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net472 245μs 91.3ns 354ns 5.73 0.487 0 36.22 KB
#5155 RunWaf(args=NestedMap (10)) net6.0 57.5μs 54.9ns 213ns 0.23 0 0 16.66 KB
#5155 RunWaf(args=NestedMap (10)) netcoreapp3.1 68μs 101ns 390ns 0.202 0 0 16.65 KB
#5155 RunWaf(args=NestedMap (10)) net472 102μs 219ns 849ns 2.65 0.0982 0 16.73 KB
#5155 RunWafTwice(args=NestedMap (10)) net6.0 60.1μs 344ns 2.62μs 0.243 0 0 17.33 KB
#5155 RunWafTwice(args=NestedMap (10)) netcoreapp3.1 74.5μs 420ns 2.72μs 0.214 0 0 17.31 KB
#5155 RunWafTwice(args=NestedMap (10)) net472 106μs 49.9ns 187ns 2.76 0.106 0 17.42 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 106μs 150ns 583ns 0.267 0 0 19.49 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 119μs 185ns 715ns 0.239 0 0 19.44 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [22]) net472 157μs 105ns 393ns 3.09 0.151 0 19.72 KB
#5155 RunWaf(args=NestedMap (100)) net6.0 105μs 572ns 3.08μs 0.433 0 0 33.35 KB
#5155 RunWaf(args=NestedMap (100)) netcoreapp3.1 137μs 731ns 4.07μs 0.409 0 0 33.92 KB
#5155 RunWaf(args=NestedMap (100)) net472 188μs 173ns 668ns 5.39 0.372 0 34.26 KB
#5155 RunWafTwice(args=NestedMap (100)) net6.0 107μs 39.5ns 153ns 0.48 0 0 34.02 KB
#5155 RunWafTwice(args=NestedMap (100)) netcoreapp3.1 137μs 209ns 808ns 0.409 0 0 34.59 KB
#5155 RunWafTwice(args=NestedMap (100)) net472 205μs 136ns 529ns 5.52 0.394 0 34.96 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [23]) net6.0 153μs 91.3ns 316ns 0.483 0 0 36.18 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [23]) netcoreapp3.1 188μs 964ns 4.62μs 0.482 0 0 36.71 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [23]) net472 245μs 52.1ns 188ns 5.89 0.491 0 37.26 KB
#5155 RunWaf(args=NestedMap (20)) net6.0 106μs 562ns 2.92μs 0.432 0 0 32.78 KB
#5155 RunWaf(args=NestedMap (20)) netcoreapp3.1 133μs 638ns 2.55μs 0.394 0 0 32.89 KB
#5155 RunWaf(args=NestedMap (20)) net472 194μs 426ns 1.6μs 5.26 0.375 0 33.23 KB
#5155 RunWafTwice(args=NestedMap (20)) net6.0 107μs 121ns 451ns 0.45 0 0 33.45 KB
#5155 RunWafTwice(args=NestedMap (20)) netcoreapp3.1 137μs 285ns 988ns 0.472 0 0 33.55 KB
#5155 RunWafTwice(args=NestedMap (20)) net472 197μs 857ns 3.32μs 5.39 0.392 0 33.92 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 161μs 86.9ns 337ns 0.484 0 0 35.61 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 185μs 400ns 1.55μs 0.463 0 0 35.68 KB
#5155 RunWafWithAttack(args=Neste(...)tack) [22]) net472 250μs 803ns 3.11μs 5.65 0.368 0 36.22 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 172μs 100ns 375ns 0.172 0 0 18.26 KB
master SendRequest netcoreapp3.1 193μs 494ns 1.91μs 0.191 0 0 20.42 KB
master SendRequest net472 0.000226ns 0.00013ns 0.000467ns 0 0 0 0 b
#5155 SendRequest net6.0 172μs 129ns 483ns 0.259 0 0 18.26 KB
#5155 SendRequest netcoreapp3.1 192μs 239ns 924ns 0.192 0 0 20.42 KB
#5155 SendRequest net472 0.000243ns 0.000147ns 0.00055ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5155

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.53 KB 41.74 KB 209 B 0.50%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 541μs 1.33μs 5.13μs 0.548 0 0 41.53 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 653μs 1.1μs 4.26μs 0.324 0 0 41.78 KB
master WriteAndFlushEnrichedTraces net472 798μs 3.97μs 17.3μs 8.17 2.45 0.408 53.22 KB
#5155 WriteAndFlushEnrichedTraces net6.0 564μs 139ns 537ns 0.548 0 0 41.74 KB
#5155 WriteAndFlushEnrichedTraces netcoreapp3.1 644μs 367ns 1.32μs 0.317 0 0 41.75 KB
#5155 WriteAndFlushEnrichedTraces net472 829μs 3.19μs 11.9μs 8.28 2.48 0.414 53.24 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.21μs 0.52ns 1.95ns 0.0103 0 0 776 B
master ExecuteNonQuery netcoreapp3.1 1.49μs 0.734ns 2.84ns 0.00958 0 0 776 B
master ExecuteNonQuery net472 1.84μs 0.653ns 2.53ns 0.116 0 0 738 B
#5155 ExecuteNonQuery net6.0 1.17μs 0.677ns 2.62ns 0.0105 0 0 776 B
#5155 ExecuteNonQuery netcoreapp3.1 1.5μs 0.806ns 3.12ns 0.00987 0 0 776 B
#5155 ExecuteNonQuery net472 1.86μs 1.42ns 5.51ns 0.117 0 0 738 B
Benchmarks.Trace.ElasticsearchBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5155

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net6.0 1.179 1,135.00 1,338.25

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.13μs 0.547ns 2.05ns 0.0131 0 0 944 B
master CallElasticsearch netcoreapp3.1 1.56μs 1.34ns 5.2ns 0.0125 0 0 944 B
master CallElasticsearch net472 2.52μs 1.19ns 4.61ns 0.152 0 0 963 B
master CallElasticsearchAsync net6.0 1.3μs 0.792ns 2.96ns 0.0125 0 0 920 B
master CallElasticsearchAsync netcoreapp3.1 1.54μs 1.27ns 4.39ns 0.013 0 0 992 B
master CallElasticsearchAsync net472 2.54μs 1.1ns 4.26ns 0.161 0 0 1.02 KB
#5155 CallElasticsearch net6.0 1.34μs 0.595ns 2.23ns 0.0127 0 0 944 B
#5155 CallElasticsearch netcoreapp3.1 1.44μs 0.937ns 3.5ns 0.0123 0 0 944 B
#5155 CallElasticsearch net472 2.48μs 1.35ns 5.21ns 0.152 0 0 963 B
#5155 CallElasticsearchAsync net6.0 1.24μs 0.51ns 1.97ns 0.0125 0 0 920 B
#5155 CallElasticsearchAsync netcoreapp3.1 1.7μs 1.15ns 4.45ns 0.0133 0 0 992 B
#5155 CallElasticsearchAsync net472 2.6μs 0.956ns 3.7ns 0.161 0 0 1.02 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.31μs 0.743ns 2.78ns 0.013 0 0 920 B
master ExecuteAsync netcoreapp3.1 1.59μs 1.03ns 3.84ns 0.0127 0 0 920 B
master ExecuteAsync net472 1.83μs 1.12ns 4.35ns 0.14 0 0 883 B
#5155 ExecuteAsync net6.0 1.31μs 0.652ns 2.44ns 0.013 0 0 920 B
#5155 ExecuteAsync netcoreapp3.1 1.61μs 0.829ns 3.21ns 0.0128 0 0 920 B
#5155 ExecuteAsync net472 1.9μs 0.749ns 2.9ns 0.14 0 0 883 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.02μs 1.47ns 5.5ns 0.0285 0 0 2.1 KB
master SendAsync netcoreapp3.1 4.9μs 1.45ns 5.23ns 0.0343 0 0 2.64 KB
master SendAsync net472 7.7μs 27.2ns 94.3ns 0.525 0 0 3.31 KB
#5155 SendAsync net6.0 4.18μs 1.77ns 6.87ns 0.0293 0 0 2.1 KB
#5155 SendAsync netcoreapp3.1 5.02μs 2.92ns 10.9ns 0.035 0 0 2.64 KB
#5155 SendAsync net472 7.76μs 2.6ns 10.1ns 0.525 0 0 3.31 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5155

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 59.26 KB 61.9 KB 2.65 KB 4.47%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 202.51 KB 204.7 KB 2.18 KB 1.08%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 61.8μs 985ns 9.55μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 55.8μs 460ns 4.46μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 37.9μs 72.5ns 261ns 0 0 0 59.26 KB
master StringConcatAspectBenchmark net6.0 251μs 4.07μs 39.1μs 0 0 0 203.98 KB
master StringConcatAspectBenchmark netcoreapp3.1 284μs 1.63μs 12.1μs 0 0 0 202.51 KB
master StringConcatAspectBenchmark net472 239μs 2.84μs 26.9μs 0 0 0 221.18 KB
#5155 StringConcatBenchmark net6.0 52μs 190ns 657ns 0 0 0 43.44 KB
#5155 StringConcatBenchmark netcoreapp3.1 52.7μs 236ns 884ns 0 0 0 42.64 KB
#5155 StringConcatBenchmark net472 37.8μs 137ns 514ns 0 0 0 61.9 KB
#5155 StringConcatAspectBenchmark net6.0 261μs 1.23μs 5.07μs 0 0 0 203.53 KB
#5155 StringConcatAspectBenchmark netcoreapp3.1 271μs 3.13μs 30.3μs 0 0 0 204.7 KB
#5155 StringConcatAspectBenchmark net472 225μs 1.06μs 4.37μs 0 0 0 221.18 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.51μs 2.18ns 8.17ns 0.0223 0 0 1.58 KB
master EnrichedLog netcoreapp3.1 2.26μs 3.22ns 12.1ns 0.0218 0 0 1.58 KB
master EnrichedLog net472 2.73μs 5.02ns 19.5ns 0.239 0 0 1.51 KB
#5155 EnrichedLog net6.0 1.6μs 0.758ns 2.84ns 0.0221 0 0 1.58 KB
#5155 EnrichedLog netcoreapp3.1 2.11μs 0.751ns 2.81ns 0.0212 0 0 1.58 KB
#5155 EnrichedLog net472 2.74μs 2.41ns 9.35ns 0.239 0 0 1.51 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 112μs 98.2ns 368ns 0.0562 0 0 4.22 KB
master EnrichedLog netcoreapp3.1 117μs 111ns 416ns 0.0582 0 0 4.22 KB
master EnrichedLog net472 148μs 43.7ns 169ns 0.662 0.221 0 4.4 KB
#5155 EnrichedLog net6.0 113μs 98.3ns 381ns 0.0566 0 0 4.22 KB
#5155 EnrichedLog netcoreapp3.1 118μs 141ns 527ns 0.0587 0 0 4.22 KB
#5155 EnrichedLog net472 148μs 53.8ns 201ns 0.671 0.224 0 4.4 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.12μs 1.04ns 3.75ns 0.0297 0 0 2.14 KB
master EnrichedLog netcoreapp3.1 4.06μs 1.28ns 4.61ns 0.0284 0 0 2.14 KB
master EnrichedLog net472 4.84μs 1.42ns 5.31ns 0.309 0 0 1.95 KB
#5155 EnrichedLog net6.0 2.94μs 0.986ns 3.82ns 0.0296 0 0 2.14 KB
#5155 EnrichedLog netcoreapp3.1 4.29μs 3.31ns 11.9ns 0.0277 0 0 2.14 KB
#5155 EnrichedLog net472 4.8μs 2.12ns 8.2ns 0.311 0 0 1.95 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.39μs 0.775ns 3ns 0.0152 0 0 1.11 KB
master SendReceive netcoreapp3.1 1.79μs 0.61ns 2.28ns 0.0152 0 0 1.11 KB
master SendReceive net472 2.18μs 3.19ns 12.4ns 0.178 0 0 1.12 KB
#5155 SendReceive net6.0 1.35μs 1.11ns 3.85ns 0.0155 0 0 1.11 KB
#5155 SendReceive netcoreapp3.1 1.78μs 1.21ns 4.69ns 0.0151 0 0 1.11 KB
#5155 SendReceive net472 2.13μs 2.26ns 8.73ns 0.178 0 0 1.12 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.77μs 0.611ns 2.37ns 0.0221 0 0 1.54 KB
master EnrichedLog netcoreapp3.1 3.98μs 2.2ns 8.52ns 0.0199 0 0 1.58 KB
master EnrichedLog net472 4.44μs 2.24ns 8.66ns 0.312 0 0 1.97 KB
#5155 EnrichedLog net6.0 2.74μs 0.87ns 3.25ns 0.0216 0 0 1.54 KB
#5155 EnrichedLog netcoreapp3.1 3.84μs 1.09ns 4.08ns 0.0211 0 0 1.58 KB
#5155 EnrichedLog net472 4.46μs 1.52ns 5.68ns 0.312 0 0 1.97 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5155

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 1.155 438.19 506.26

Faster 🎉 in #5155

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.119 871.03 778.65

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 438ns 0.141ns 0.545ns 0.00757 0 0 544 B
master StartFinishSpan netcoreapp3.1 689ns 0.606ns 2.27ns 0.00731 0 0 544 B
master StartFinishSpan net472 772ns 0.464ns 1.8ns 0.0867 0 0 546 B
master StartFinishScope net6.0 534ns 0.177ns 0.687ns 0.00941 0 0 664 B
master StartFinishScope netcoreapp3.1 871ns 0.243ns 0.941ns 0.00885 0 0 664 B
master StartFinishScope net472 988ns 0.431ns 1.67ns 0.0994 0 0 626 B
#5155 StartFinishSpan net6.0 506ns 0.18ns 0.697ns 0.00752 0 0 544 B
#5155 StartFinishSpan netcoreapp3.1 727ns 0.722ns 2.7ns 0.00732 0 0 544 B
#5155 StartFinishSpan net472 743ns 0.221ns 0.828ns 0.0866 0 0 546 B
#5155 StartFinishScope net6.0 579ns 0.138ns 0.536ns 0.00923 0 0 664 B
#5155 StartFinishScope netcoreapp3.1 779ns 0.222ns 0.832ns 0.00874 0 0 664 B
#5155 StartFinishScope net472 1.05μs 0.741ns 2.87ns 0.0993 0 0 626 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5155

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.209 580.05 701.50

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 580ns 0.129ns 0.499ns 0.00935 0 0 664 B
master RunOnMethodBegin netcoreapp3.1 998ns 0.369ns 1.43ns 0.00893 0 0 664 B
master RunOnMethodBegin net472 1.05μs 0.569ns 2.2ns 0.0993 0 0 626 B
#5155 RunOnMethodBegin net6.0 701ns 0.213ns 0.823ns 0.00922 0 0 664 B
#5155 RunOnMethodBegin netcoreapp3.1 925ns 0.296ns 1.11ns 0.00874 0 0 664 B
#5155 RunOnMethodBegin net472 1μs 0.326ns 1.22ns 0.0992 0 0 626 B

andrewlock
andrewlock reviewed Feb 12, 2024
View reviewed changes
tracer/src/Datadog.Trace/Debugger/Symbols/SymbolsUploader.cs
{
await ProcessItemAsync(args.LoadedAssembly).ConfigureAwait(false);
_ = ProcessItemAsync(args.LoadedAssembly);
Copy link
Member

Choose a reason for hiding this comment

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

This task is unobserved right, and given ProcessItemAsync may throw (e.g. WaitAsync() could throw Cancellation exception), could that not be an issue? It's a niche edgecase, but as I assume we just need another try-catch in ProcessItemAsync, is that worth doing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unobserved task exceptions are not an issue since .NET 4.5, but I guess it can't hurt to log the exception.

Copy link
Member

Choose a reason for hiding this comment

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

Unobserved task exceptions are not an issue since .NET 4.5

For some reason I thought they were still a thing on .NET FX, in which case, meh

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Feb 27, 2024 •
edited
Loading

Datadog Report

Branch report: kevin/async_void
Commit report: f0d7c09
Test service: dd-trace-dotnet

✅ 0 Failed, 329236 Passed, 2034 Skipped, 36m 9.05s Wall Time

@kevingosse kevingosse requested a review from a team as a code owner March 11, 2024 09:51
GreenMatan
GreenMatan approved these changes Mar 13, 2024
View reviewed changes
Copy link
Contributor

@GreenMatan GreenMatan left a comment

Choose a reason for hiding this comment

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

LGTM

@kevingosse kevingosse merged commit c2d8654 into master Mar 13, 2024
56 of 60 checks passed
@kevingosse kevingosse deleted the kevin/async_void branch March 13, 2024 13:08
@github-actions github-actions bot added this to the vNext milestone Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewlock andrewlock andrewlock left review comments

@tonyredondo tonyredondo tonyredondo approved these changes

@daniel-romano-DD daniel-romano-DD daniel-romano-DD approved these changes

@GreenMatan GreenMatan GreenMatan approved these changes

Assignees
No one assigned
Labels
area:debugger area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)
Projects
None yet
Milestone
2.49.0
Development

Successfully merging this pull request may close these issues.
5 participants
@kevingosse @andrewlock @tonyredondo @GreenMatan @daniel-romano-DD