Use vendored spans in tags generation #5298

kevingosse · 2024-03-11T12:24:53Z

Summary of changes

In our tags code, use spans instead of arrays on .net framework and netstandard.

Reason for change

It won't actually speed up anything (spans are used here on netcoreapp to benefit from a compiler optimization that, afaik, does not exist on netfx), but this is an experiment to validate that we can seamlessly use vendored spans on netfx and real spans on netcore. Plus, it makes this code much simpler.

datadog-ddstaging · 2024-03-11T12:53:00Z

Datadog Report

Branch report: kevin/spans
Commit report: f7c26c2
Test service: dd-trace-dotnet

✅ 0 Failed, 332651 Passed, 1562 Skipped, 36m 1.92s Wall Time

andrewlock · 2024-03-11T18:49:52Z

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 (5298) - mean (74ms)  : 66, 82
     .   : milestone, 74,
    master - mean (75ms)  : 65, 85
     .   : milestone, 75,

    section CallTarget+Inlining+NGEN
    This PR (5298) - mean (999ms)  : 973, 1024
     .   : milestone, 999,
    master - mean (990ms)  : 966, 1014
     .   : milestone, 990,

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

    section CallTarget+Inlining+NGEN
    This PR (5298) - mean (721ms)  : 693, 748
     .   : milestone, 721,
    master - mean (717ms)  : 697, 736
     .   : milestone, 717,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5298) - mean (95ms)  : 92, 99
     .   : milestone, 95,
    master - mean (95ms)  : 92, 97
     .   : milestone, 95,

    section CallTarget+Inlining+NGEN
    This PR (5298) - mean (668ms)  : 646, 690
     .   : milestone, 668,
    master - mean (670ms)  : 646, 695
     .   : milestone, 670,

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

    section CallTarget+Inlining+NGEN
    This PR (5298) - mean (1,069ms)  : 1042, 1095
     .   : milestone, 1069,
    master - mean (1,069ms)  : 1039, 1098
     .   : milestone, 1069,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5298) - mean (270ms)  : 266, 275
     .   : milestone, 270,
    master - mean (270ms)  : 265, 275
     .   : milestone, 270,

    section CallTarget+Inlining+NGEN
    This PR (5298) - mean (868ms)  : 844, 892
     .   : milestone, 868,
    master - mean (868ms)  : 841, 895
     .   : milestone, 868,

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

    section CallTarget+Inlining+NGEN
    This PR (5298) - mean (859ms)  : 820, 898
     .   : milestone, 859,
    master - mean (852ms)  : 830, 873
     .   : milestone, 852,

daniel-romano-DD · 2024-03-12T07:48:32Z

tracer/src/Datadog.Trace/Agent/MessagePack/SpanMessagePackFormatter.cs

@@ -430,11 +430,7 @@ private void WriteTag(ref byte[] bytes, ref int offset, string key, string value
        }

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-#if !NETCOREAPP


Only to get rid of these #ifs it's already worth it

daniel-romano-DD · 2024-03-12T07:54:45Z

tracer/src/Datadog.Trace/GlobalUsings.cs

+
+global using ThrowHelper = Datadog.Trace.Util.ThrowHelper;
+
+#if NETFRAMEWORK || NETSTANDARD2_0


daniel-romano-DD

I think it's an improvement in code readability also. Nicely done.

tonyredondo

LGTM

andrewlock · 2024-03-13T14:18:26Z

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 (5298) (11.129M)   : 0, 11128579
    master (11.387M)   : 0, 11386870
    benchmarks/2.9.0 (11.099M)   : 0, 11099186

    section Automatic
    This PR (5298) (7.636M)   : 0, 7636217
    master (7.766M)   : 0, 7766388
    benchmarks/2.9.0 (7.823M)   : 0, 7823285

    section Trace stats
    This PR (5298) (7.808M)   : 0, 7808046
    master (8.071M)   : 0, 8071440

    section Manual
    This PR (5298) (9.568M)   : 0, 9568459
    master (9.806M)   : 0, 9806449

    section Manual + Automatic
    This PR (5298) (7.196M)   : 0, 7195572
    master (7.297M)   : 0, 7297336

    section Version Conflict
    This PR (5298) (6.382M)   : 0, 6382456
    master (6.603M)   : 0, 6602958

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5298) (9.414M)   : 0, 9413611
    master (9.706M)   : 0, 9706267
    benchmarks/2.9.0 (9.709M)   : 0, 9708719

    section Automatic
    This PR (5298) (6.663M)   : 0, 6662548
    master (6.592M)   : 0, 6591996

    section Trace stats
    This PR (5298) (6.934M)   : 0, 6934435
    master (6.912M)   : 0, 6912452

    section Manual
    This PR (5298) (8.166M)   : 0, 8165580
    master (8.134M)   : 0, 8134221

    section Manual + Automatic
    This PR (5298) (6.300M)   : 0, 6299560
    master (6.177M)   : 0, 6177016

    section Version Conflict
    This PR (5298) (5.571M)   : 0, 5570770
    master (5.622M)   : 0, 5621557

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5298) (9.824M)   : 0, 9823797
    master (10.038M)   : 0, 10037686
    benchmarks/2.9.0 (10.184M)   : 0, 10183565

    section Automatic
    This PR (5298) (7.094M)   : 0, 7093793
    master (7.146M)   : 0, 7146271
    benchmarks/2.9.0 (7.254M)   : 0, 7253763

    section Trace stats
    This PR (5298) (7.329M)   : 0, 7329116
    master (7.389M)   : 0, 7389237

    section Manual
    This PR (5298) (8.735M)   : 0, 8735399
    master (8.795M)   : 0, 8795107

    section Manual + Automatic
    This PR (5298) (6.785M)   : 0, 6784700
    master (6.908M)   : 0, 6907539

    section Version Conflict
    This PR (5298) (6.162M)   : 0, 6161763
    master (6.366M)   : 0, 6365847

andrewlock

nice

Use vendored spans in tags generation

ccc3258

kevingosse requested review from a team as code owners March 11, 2024 12:24

Fix tests, move usings to csproj

e898ed1

kevingosse mentioned this pull request Mar 11, 2024

Optimize HttpMessage.GetContentEncoding using spans #5301

Merged

Move the global usings to a dedicated file

677c852

daniel-romano-DD reviewed Mar 12, 2024

View reviewed changes

daniel-romano-DD approved these changes Mar 12, 2024

View reviewed changes

Update GlobalUsings.cs

f7c26c2

tonyredondo approved these changes Mar 13, 2024

View reviewed changes

andrewlock approved these changes Mar 13, 2024

View reviewed changes

kevingosse merged commit 7813100 into master Mar 13, 2024
54 of 56 checks passed

kevingosse deleted the kevin/spans branch March 13, 2024 16:41

github-actions bot added this to the vNext milestone Mar 13, 2024

link04 added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:performance Performance, speed, latency, resource usage (CPU, memory) labels Mar 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use vendored spans in tags generation #5298

Use vendored spans in tags generation #5298

kevingosse commented Mar 11, 2024

datadog-ddstaging bot commented Mar 11, 2024 •

edited

Loading

andrewlock commented Mar 11, 2024 •

edited

Loading

daniel-romano-DD Mar 12, 2024

daniel-romano-DD Mar 12, 2024

daniel-romano-DD left a comment

tonyredondo left a comment

andrewlock commented Mar 13, 2024

andrewlock left a comment


		global using ThrowHelper = Datadog.Trace.Util.ThrowHelper;

		#if NETFRAMEWORK \|\| NETSTANDARD2_0

Use vendored spans in tags generation #5298

Use vendored spans in tags generation #5298

Conversation

kevingosse commented Mar 11, 2024

Summary of changes

Reason for change

datadog-ddstaging bot commented Mar 11, 2024 • edited Loading

Datadog Report

andrewlock commented Mar 11, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

daniel-romano-DD Mar 12, 2024

Choose a reason for hiding this comment

daniel-romano-DD Mar 12, 2024

Choose a reason for hiding this comment

daniel-romano-DD left a comment

Choose a reason for hiding this comment

tonyredondo left a comment

Choose a reason for hiding this comment

andrewlock commented Mar 13, 2024

Throughput/Crank Report:zap:

andrewlock left a comment

Choose a reason for hiding this comment

datadog-ddstaging bot commented Mar 11, 2024 •

edited

Loading

andrewlock commented Mar 11, 2024 •

edited

Loading