[v3] Change manual instrumentation approach to not use reverse duck-typing #5214

andrewlock · 2024-02-20T11:36:17Z

Summary of changes

Changes the Manual instrumentation to use DuckCast<T> instead of DuckImplement<T>
CI Visibility
- Revert some of the changes to CI visibility public API that were originally required in [v3] Implement manual tracing functionality in Datadog.Trace.Manual #5072
- Move un-duck-typed-able methods on ITest to be extension methods instead

Reason for change

In #5183 we updated the throughput tests to use the new manual instrumentation approach, and we found a significant (10-15%) hit to throughput. After more investigation, we tied this back to the use of reverse duck-typing. Reverse duck-typing currently requires a call to DynamicInvoke() which is slow and allocatey.

Implementation details

Before this change we defined ManualScope et al. in Datadog.Trace and used reverse duck-typing to duck-implement the (manual) IScope types. After this change we simply ducktype the Scope as IScope (Datadog.Trace.Manual) directly. This avoids the performance hit and is generally a lot simpler.

Unfortunately, the ITest interface (based on Test's public API) is not amenable to duck-typing. To work around that, I removed those methods:

Removed AddParameters(), SetBenchmarkData() and AddBenchmarkData() from ITest. This means we can now ducktype Test directly
Moved them to be extension methods
Add instrumentation of the extension methods to call the underlying types

The main annoying thing about the extensions is that they are now methods that accept ISpan which is generally not something we want to do, but I don't see a good alternative here (I'm open to suggestions!)

Test coverage

Most of this is already covered by the manual integration tests. As part of this I also updated the ducktyping tests.

Similarly I added a benchmark (skipped for now) for the approach and compared the approach in #5072 and the approach here (using vendored DuckTyping). There's still an increase in allocations (which is expected, we're necessarily creating more objects), but the hit on speed is nothing like as bad (and throughput tests in #5183 show that this is much better).

before:

BenchmarkDotNet=v0.13.5, OS=Windows 10 (10.0.19045.4046/22H2/2022Update)
12th Gen Intel Core i9-12900H, 1 CPU, 20 logical and 14 physical cores
.NET SDK=8.0.102
  [Host]     : .NET 8.0.2 (8.0.224.6711), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.2 (8.0.224.6711), X64 RyuJIT AVX2


|                 Method |     Mean |    Error |   StdDev |   Gen0 | Allocated |
|----------------------- |---------:|---------:|---------:|-------:|----------:|
|       StartFinishScope | 345.4 ns |  6.83 ns | 15.42 ns | 0.0019 |     664 B |
| ManualStartFinishScope | 871.7 ns | 17.41 ns | 33.12 ns | 0.0019 |     968 B |

after:

|                 Method |     Mean |   Error |  StdDev |   Gen0 | Allocated |
|----------------------- |---------:|--------:|--------:|-------:|----------:|
|       StartFinishScope | 285.0 ns | 3.65 ns | 3.42 ns | 0.0019 |     664 B |
| ManualStartFinishScope | 327.8 ns | 2.68 ns | 2.51 ns | 0.0024 |     736 B |

Other details

Stacked on

You can review a "compressed" version of this stack here:

[v3] Stack for kevin #5235

Dependent of

[v3] Update manual throughput tests #5183

datadog-ddstaging · 2024-02-20T11:59:24Z

Datadog Report

Branch report: andrew/v3-public-apis/change-ducktyping-approach
Commit report: a90f420
Test service: dd-trace-dotnet

❌ 1 Failed (1 Known Flaky), 309426 Passed, 2385 Skipped, 40m 58.84s Wall Time

❌ Failed Tests (1)

CheckProfilesSentThroughNamedPipe - Datadog.Profiler.IntegrationTests.WindowsOnly.NamedPipeTestcs - ❄️ Known flaky - Details

Expand for error

 Found error message in the log file D:\a\1\s\profiler\build_data\NamedPipeTestcs\CheckProfilesSentThroughNamedPipe\net8.0\logs\DD-DotNet-Profiler-Native-Samples.Computer01-4200.log
 Expected: False
 Actual:   True

andrewlock · 2024-02-20T12:32:12Z

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 (5214) - mean (76ms)  : 65, 87
     .   : milestone, 76,
    master - mean (73ms)  : 65, 81
     .   : milestone, 73,

    section CallTarget+Inlining+NGEN
    This PR (5214) - mean (994ms)  : 974, 1014
     .   : milestone, 994,
    master - mean (987ms)  : 962, 1012
     .   : milestone, 987,

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

    section CallTarget+Inlining+NGEN
    This PR (5214) - mean (724ms)  : 703, 746
     .   : milestone, 724,
    master - mean (714ms)  : 696, 733
     .   : milestone, 714,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5214) - mean (94ms)  : 90, 97
     .   : milestone, 94,
    master - mean (94ms)  : 91, 96
     .   : milestone, 94,

    section CallTarget+Inlining+NGEN
    This PR (5214) - mean (677ms)  : 652, 703
     .   : milestone, 677,
    master - mean (673ms)  : 644, 701
     .   : milestone, 673,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5214) - mean (188ms)  : 183, 193
     .   : milestone, 188,
    master - mean (188ms)  : 185, 192
     .   : milestone, 188,

    section CallTarget+Inlining+NGEN
    This PR (5214) - mean (1,072ms)  : 1043, 1101
     .   : milestone, 1072,
    master - mean (1,062ms)  : 1037, 1088
     .   : milestone, 1062,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5214) - mean (271ms)  : 265, 276
     .   : milestone, 271,
    master - mean (271ms)  : 267, 276
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (5214) - mean (871ms)  : 846, 896
     .   : milestone, 871,
    master - mean (867ms)  : 848, 886
     .   : milestone, 867,

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

    section CallTarget+Inlining+NGEN
    This PR (5214) - mean (855ms)  : 828, 883
     .   : milestone, 855,
    master - mean (853ms)  : 829, 877
     .   : milestone, 853,

andrewlock · 2024-02-20T17:21:13Z

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 (5214) (10.810M)   : 0, 10809925
    master (11.452M)   : 0, 11452280
    benchmarks/2.9.0 (11.508M)   : 0, 11507797

    section Automatic
    This PR (5214) (7.554M)   : 0, 7553621
    master (7.791M)   : 0, 7790888
    benchmarks/2.9.0 (8.301M)   : 0, 8300571

    section Trace stats
    This PR (5214) (8.091M)   : 0, 8090975
    master (8.121M)   : 0, 8120844

    section Manual
    This PR (5214) (9.416M)   : 0, 9416481
    master (9.869M)   : 0, 9869220

    section Manual + Automatic
    This PR (5214) (7.087M)   : 0, 7087392
    master (7.404M)   : 0, 7403853

    section Version Conflict
    This PR (5214) (6.434M)   : 0, 6434203
    master (6.641M)   : 0, 6641084

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5214) (9.588M)   : 0, 9588020
    master (9.418M)   : 0, 9418403
    benchmarks/2.9.0 (9.336M)   : 0, 9336082

    section Automatic
    This PR (5214) (6.390M)   : 0, 6390344
    master (6.560M)   : 0, 6559687

    section Trace stats
    This PR (5214) (6.794M)   : 0, 6794473
    master (6.815M)   : 0, 6814975

    section Manual
    This PR (5214) (8.290M)   : 0, 8290111
    master (8.215M)   : 0, 8215195

    section Manual + Automatic
    This PR (5214) (6.198M)   : 0, 6197677
    master (6.106M)   : 0, 6105864

    section Version Conflict
    This PR (5214) (5.670M)   : 0, 5669855
    master (5.692M)   : 0, 5692455

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5214) (10.388M)   : 0, 10388351
    master (10.270M)   : 0, 10270238
    benchmarks/2.9.0 (10.049M)   : 0, 10048647

    section Automatic
    This PR (5214) (7.323M)   : 0, 7323065
    master (7.314M)   : 0, 7313801
    benchmarks/2.9.0 (7.621M)   : 0, 7620725

    section Trace stats
    This PR (5214) (7.729M)   : 0, 7728845
    master (7.627M)   : 0, 7627472

    section Manual
    This PR (5214) (9.033M)   : 0, 9033177
    master (9.080M)   : 0, 9080294

    section Manual + Automatic
    This PR (5214) (6.864M)   : 0, 6863990
    master (7.051M)   : 0, 7051366

    section Version Conflict
    This PR (5214) (5.788M)   : crit ,0, 5788274
    master (6.363M)   : 0, 6362577

gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5214) (7.277M)   : 0, 7276909
    master (7.508M)   : 0, 7508094
    benchmarks/2.9.0 (7.472M)   : 0, 7471592

    section No attack
    This PR (5214) (1.828M)   : 0, 1828331
    master (1.877M)   : 0, 1876915
    benchmarks/2.9.0 (3.145M)   : 0, 3144736

    section Attack
    This PR (5214) (1.471M)   : 0, 1470566
    master (1.498M)   : 0, 1497742
    benchmarks/2.9.0 (2.494M)   : 0, 2494070

    section Blocking
    This PR (5214) (3.096M)   : crit ,0, 3095955
    master (3.278M)   : 0, 3278177

    section IAST default
    This PR (5214) (6.403M)   : 0, 6403398
    master (6.581M)   : 0, 6581090

    section IAST full
    This PR (5214) (5.563M)   : 0, 5562591
    master (5.814M)   : 0, 5814172

    section Base vuln
    This PR (5214) (0.954M)   : 0, 954328
    master (0.981M)   : 0, 980670

    section IAST vuln
    This PR (5214) (0.852M)   : 0, 851909
    master (0.894M)   : 0, 894300

andrewlock · 2024-02-20T18:07:41Z

Benchmarks Report 🐌

Benchmarks for #5214 compared to master:

1 benchmarks are faster, with geometric mean 1.249
5 benchmarks are slower, with geometric mean 1.181
1 benchmarks have fewer allocations
2 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.63μs	46.8ns	256ns	0.0249	0.0124	0	7.49 KB
master	`StartStopWithChild`	netcoreapp3.1	10.6μs	48.2ns	193ns	0.0268	0.0107	0	7.59 KB
master	`StartStopWithChild`	net472	17μs	46.5ns	180ns	1.32	0.333	0.0938	7.96 KB
#5214	`StartStopWithChild`	net6.0	8.72μs	41.8ns	167ns	0.0219	0.00876	0	7.49 KB
#5214	`StartStopWithChild`	netcoreapp3.1	10.9μs	60.4ns	391ns	0.0217	0.0108	0	7.58 KB
#5214	`StartStopWithChild`	net472	17.1μs	52.5ns	204ns	1.33	0.344	0.12	7.96 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	445μs	331ns	1.24μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	651μs	637ns	2.47μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	788μs	358ns	1.34μs	0.393	3.3 KB
#5214	`WriteAndFlushEnrichedTraces`	net6.0	487μs	498ns	1.93μs	0	2.7 KB
#5214	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	615μs	341ns	1.32μs	0	2.7 KB
#5214	`WriteAndFlushEnrichedTraces`	net472	793μs	434ns	1.68μs	0.396	3.3 KB

Benchmarks.Trace.Asm.AppSecBodyBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5214

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1	1.186	214.99	254.90

Faster 🎉 in #5214

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net6.0	1.249	3,795.85	3,039.57

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	41.3μs	13.9ns	53.8ns	0.0206	0	2.36 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	44.2μs	64.7ns	251ns	0.022	0	2.34 KB
master	`AllCycleSimpleBody`	net472	46.4μs	16.8ns	65ns	0.371	0	2.41 KB
master	`AllCycleMoreComplexBody`	net6.0	216μs	83.5ns	324ns	0.107	0	9.84 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	229μs	325ns	1.26μs	0.113	0	9.73 KB
master	`AllCycleMoreComplexBody`	net472	242μs	127ns	492ns	1.57	0	9.91 KB
master	`ObjectExtractorSimpleBody`	net6.0	140ns	0.11ns	0.411ns	0.00391	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	215ns	0.235ns	0.88ns	0.00375	0	272 B
master	`ObjectExtractorSimpleBody`	net472	168ns	0.138ns	0.534ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	3.8μs	1.53ns	5.71ns	0.0533	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.84μs	1.65ns	6.39ns	0.0502	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	3.75μs	1.97ns	7.36ns	0.602	0.00561	3.8 KB
#5214	`AllCycleSimpleBody`	net6.0	42.8μs	16.6ns	64.3ns	0.0213	0	2.36 KB
#5214	`AllCycleSimpleBody`	netcoreapp3.1	44.6μs	53.8ns	208ns	0.0223	0	2.34 KB
#5214	`AllCycleSimpleBody`	net472	47.1μs	90.9ns	352ns	0.375	0	2.41 KB
#5214	`AllCycleMoreComplexBody`	net6.0	214μs	87.2ns	326ns	0.107	0	9.84 KB
#5214	`AllCycleMoreComplexBody`	netcoreapp3.1	225μs	267ns	1.03μs	0.112	0	9.73 KB
#5214	`AllCycleMoreComplexBody`	net472	240μs	60.9ns	228ns	1.56	0	9.91 KB
#5214	`ObjectExtractorSimpleBody`	net6.0	141ns	0.0685ns	0.256ns	0.00393	0	280 B
#5214	`ObjectExtractorSimpleBody`	netcoreapp3.1	255ns	0.251ns	0.971ns	0.00364	0	272 B
#5214	`ObjectExtractorSimpleBody`	net472	182ns	0.418ns	1.62ns	0.0446	0	281 B
#5214	`ObjectExtractorMoreComplexBody`	net6.0	3.04μs	1.4ns	5.24ns	0.0531	0	3.78 KB
#5214	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.94μs	1.87ns	7.24ns	0.0494	0	3.69 KB
#5214	`ObjectExtractorMoreComplexBody`	net472	3.79μs	2.8ns	10.8ns	0.602	0.0057	3.8 KB

Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	52μs	49.9ns	193ns	0.436	0	30.94 KB
master	`EncodeArgs`	netcoreapp3.1	70μs	66.1ns	247ns	0.42	0	31.47 KB
master	`EncodeArgs`	net472	83.7μs	69.8ns	270ns	5.09	0.0835	32.27 KB
master	`EncodeLegacyArgs`	net6.0	129μs	151ns	566ns	0.444	0	33.89 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	157μs	400ns	1.44μs	0.458	0	34.09 KB
master	`EncodeLegacyArgs`	net472	209μs	88.6ns	307ns	5.56	0.419	35 KB
#5214	`EncodeArgs`	net6.0	53.6μs	72.9ns	283ns	0.427	0	30.94 KB
#5214	`EncodeArgs`	netcoreapp3.1	68.6μs	81.8ns	317ns	0.409	0	31.47 KB
#5214	`EncodeArgs`	net472	84.4μs	38.7ns	150ns	5.12	0.0847	32.27 KB
#5214	`EncodeLegacyArgs`	net6.0	121μs	194ns	673ns	0.44	0	33.89 KB
#5214	`EncodeLegacyArgs`	netcoreapp3.1	161μs	894ns	5.93μs	0.311	0	34.09 KB
#5214	`EncodeLegacyArgs`	net472	212μs	746ns	2.89μs	5.56	0.412	35 KB

Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunWafRealisticBenchmark`	net6.0	193μs	128ns	498ns	0.0965	6.51 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	203μs	243ns	942ns	0	6.49 KB
master	`RunWafRealisticBenchmark`	net472	225μs	157ns	607ns	1.01	6.59 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	128μs	104ns	403ns	0	4.15 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	134μs	202ns	783ns	0	4.14 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	146μs	90ns	349ns	0.656	4.19 KB
#5214	`RunWafRealisticBenchmark`	net6.0	191μs	153ns	574ns	0	6.51 KB
#5214	`RunWafRealisticBenchmark`	netcoreapp3.1	203μs	185ns	691ns	0	6.49 KB
#5214	`RunWafRealisticBenchmark`	net472	223μs	65.2ns	253ns	1	6.59 KB
#5214	`RunWafRealisticBenchmarkWithAttack`	net6.0	126μs	79.7ns	309ns	0.0625	4.15 KB
#5214	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	135μs	166ns	642ns	0	4.14 KB
#5214	`RunWafRealisticBenchmarkWithAttack`	net472	147μs	94.8ns	367ns	0.66	4.19 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	174μs	173ns	648ns	0.174	18.26 KB
master	`SendRequest`	netcoreapp3.1	191μs	175ns	679ns	0.192	20.42 KB
master	`SendRequest`	net472	0.000121ns	0.000119ns	0.000462ns	0	0 b
#5214	`SendRequest`	net6.0	173μs	153ns	594ns	0.172	18.26 KB
#5214	`SendRequest`	netcoreapp3.1	193μs	294ns	1.14μs	0.191	20.42 KB
#5214	`SendRequest`	net472	0.000472ns	0.000201ns	0.000778ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5214

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0	41.54 KB	41.97 KB	438 B	1.05%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	555μs	2.54μs	9.85μs	0.563	0	0	41.54 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	671μs	1.1μs	4.25μs	0.336	0	0	41.78 KB
master	`WriteAndFlushEnrichedTraces`	net472	842μs	3.73μs	14.4μs	8.22	2.47	0.411	53.23 KB
#5214	`WriteAndFlushEnrichedTraces`	net6.0	570μs	1.88μs	7.27μs	0.536	0	0	41.97 KB
#5214	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	645μs	523ns	2.03μs	0.324	0	0	41.76 KB
#5214	`WriteAndFlushEnrichedTraces`	net472	845μs	4.07μs	16.3μs	8.33	2.5	0.417	53.25 KB

Benchmarks.Trace.DbCommandBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5214

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0	1.210	965.80	1,168.67
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net472	1.118	1,695.82	1,895.58

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	966ns	0.495ns	1.92ns	0.0108	776 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.47μs	0.985ns	3.82ns	0.0104	776 B
master	`ExecuteNonQuery`	net472	1.7μs	1.12ns	4.03ns	0.117	738 B
#5214	`ExecuteNonQuery`	net6.0	1.17μs	0.615ns	2.38ns	0.0111	776 B
#5214	`ExecuteNonQuery`	netcoreapp3.1	1.47μs	1.38ns	5.35ns	0.0103	776 B
#5214	`ExecuteNonQuery`	net472	1.9μs	3.31ns	12.8ns	0.117	738 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.3μs	0.709ns	2.65ns	0.013	944 B
master	`CallElasticsearch`	netcoreapp3.1	1.5μs	1.25ns	4.69ns	0.0127	944 B
master	`CallElasticsearch`	net472	2.52μs	0.815ns	3.05ns	0.152	963 B
master	`CallElasticsearchAsync`	net6.0	1.27μs	0.745ns	2.88ns	0.0127	920 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.6μs	0.443ns	1.66ns	0.0138	992 B
master	`CallElasticsearchAsync`	net472	2.58μs	1.2ns	4.5ns	0.161	1.02 KB
#5214	`CallElasticsearch`	net6.0	1.18μs	0.543ns	2.1ns	0.0135	944 B
#5214	`CallElasticsearch`	netcoreapp3.1	1.53μs	1.18ns	4.59ns	0.0124	944 B
#5214	`CallElasticsearch`	net472	2.46μs	1.22ns	4.56ns	0.153	963 B
#5214	`CallElasticsearchAsync`	net6.0	1.32μs	0.6ns	2.24ns	0.0125	920 B
#5214	`CallElasticsearchAsync`	netcoreapp3.1	1.61μs	0.741ns	2.67ns	0.0128	992 B
#5214	`CallElasticsearchAsync`	net472	2.63μs	1.05ns	4.07ns	0.162	1.02 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.32μs	1.12ns	4.34ns	0.0129	920 B
master	`ExecuteAsync`	netcoreapp3.1	1.55μs	0.617ns	2.39ns	0.0125	920 B
master	`ExecuteAsync`	net472	1.91μs	0.657ns	2.46ns	0.14	883 B
#5214	`ExecuteAsync`	net6.0	1.27μs	0.79ns	3.06ns	0.0126	920 B
#5214	`ExecuteAsync`	netcoreapp3.1	1.67μs	1.23ns	4.74ns	0.0125	920 B
#5214	`ExecuteAsync`	net472	1.88μs	1.63ns	6.32ns	0.14	883 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.05μs	2.03ns	7.6ns	0.0284	2.1 KB
master	`SendAsync`	netcoreapp3.1	4.94μs	8.15ns	31.5ns	0.0345	2.64 KB
master	`SendAsync`	net472	7.91μs	2.22ns	8.58ns	0.525	3.31 KB
#5214	`SendAsync`	net6.0	4.19μs	1.4ns	5.43ns	0.0292	2.1 KB
#5214	`SendAsync`	netcoreapp3.1	4.93μs	1.53ns	5.73ns	0.0344	2.64 KB
#5214	`SendAsync`	net472	7.83μs	1.48ns	5.12ns	0.525	3.31 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5214

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	212.36 KB	213.61 KB	1.25 KB	0.59%

Fewer allocations 🎉 in #5214

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0	214.42 KB	204.21 KB	-10.22 KB	-4.76%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	51.1μs	259ns	1.19μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	52.3μs	165ns	596ns	42.64 KB
master	`StringConcatBenchmark`	net472	37.8μs	89.6ns	323ns	60.49 KB
master	`StringConcatAspectBenchmark`	net6.0	256μs	3.71μs	34.6μs	214.42 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	283μs	1.65μs	14.5μs	212.36 KB
master	`StringConcatAspectBenchmark`	net472	235μs	3.03μs	28μs	221.18 KB
#5214	`StringConcatBenchmark`	net6.0	53.9μs	248ns	994ns	43.44 KB
#5214	`StringConcatBenchmark`	netcoreapp3.1	52.8μs	194ns	699ns	42.64 KB
#5214	`StringConcatBenchmark`	net472	37.5μs	159ns	635ns	60.7 KB
#5214	`StringConcatAspectBenchmark`	net6.0	271μs	968ns	3.49μs	204.21 KB
#5214	`StringConcatAspectBenchmark`	netcoreapp3.1	273μs	3.5μs	34.1μs	213.61 KB
#5214	`StringConcatAspectBenchmark`	net472	231μs	1.1μs	4.4μs	221.18 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.41μs	0.831ns	3.22ns	0.0225	1.58 KB
master	`EnrichedLog`	netcoreapp3.1	2.16μs	1.02ns	3.8ns	0.0217	1.58 KB
master	`EnrichedLog`	net472	2.5μs	2.38ns	9.2ns	0.238	1.51 KB
#5214	`EnrichedLog`	net6.0	1.44μs	0.53ns	1.98ns	0.0224	1.58 KB
#5214	`EnrichedLog`	netcoreapp3.1	2.27μs	0.806ns	2.91ns	0.0212	1.58 KB
#5214	`EnrichedLog`	net472	2.57μs	0.974ns	3.64ns	0.239	1.51 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	113μs	84.5ns	327ns	0	0	4.22 KB
master	`EnrichedLog`	netcoreapp3.1	119μs	211ns	818ns	0	0	4.22 KB
master	`EnrichedLog`	net472	149μs	229ns	887ns	0.662	0.221	4.4 KB
#5214	`EnrichedLog`	net6.0	114μs	257ns	927ns	0.0567	0	4.22 KB
#5214	`EnrichedLog`	netcoreapp3.1	118μs	154ns	596ns	0	0	4.22 KB
#5214	`EnrichedLog`	net472	148μs	159ns	596ns	0.669	0.223	4.4 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3μs	0.856ns	3.31ns	0.0299	2.14 KB
master	`EnrichedLog`	netcoreapp3.1	4.19μs	1.42ns	5.48ns	0.0295	2.14 KB
master	`EnrichedLog`	net472	4.99μs	0.782ns	2.82ns	0.31	1.95 KB
#5214	`EnrichedLog`	net6.0	2.91μs	0.829ns	2.99ns	0.0292	2.14 KB
#5214	`EnrichedLog`	netcoreapp3.1	4.05μs	1.29ns	4.83ns	0.0288	2.14 KB
#5214	`EnrichedLog`	net472	4.9μs	1.6ns	6.2ns	0.31	1.95 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.3μs	1.66ns	6.42ns	0.0156	1.11 KB
master	`SendReceive`	netcoreapp3.1	1.76μs	3.47ns	13.4ns	0.0151	1.11 KB
master	`SendReceive`	net472	2.08μs	4.32ns	16.7ns	0.178	1.12 KB
#5214	`SendReceive`	net6.0	1.37μs	0.635ns	2.46ns	0.0157	1.11 KB
#5214	`SendReceive`	netcoreapp3.1	1.77μs	0.693ns	2.68ns	0.015	1.11 KB
#5214	`SendReceive`	net472	2.04μs	1.77ns	6.37ns	0.178	1.12 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.75μs	1.4ns	5.42ns	0.0219	1.54 KB
master	`EnrichedLog`	netcoreapp3.1	4.01μs	4.2ns	16.3ns	0.0198	1.58 KB
master	`EnrichedLog`	net472	4.29μs	2.27ns	8.79ns	0.312	1.97 KB
#5214	`EnrichedLog`	net6.0	2.86μs	0.352ns	1.22ns	0.0202	1.54 KB
#5214	`EnrichedLog`	netcoreapp3.1	3.78μs	1.16ns	4.36ns	0.021	1.58 KB
#5214	`EnrichedLog`	net472	4.38μs	3.01ns	11.2ns	0.313	1.97 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5214

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1	1.214	650.16	789.30
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472	1.178	680.75	801.86

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	481ns	0.232ns	0.898ns	0.00757	544 B
master	`StartFinishSpan`	netcoreapp3.1	650ns	0.252ns	0.944ns	0.00736	544 B
master	`StartFinishSpan`	net472	680ns	0.222ns	0.861ns	0.0865	546 B
master	`StartFinishScope`	net6.0	554ns	0.183ns	0.71ns	0.00921	664 B
master	`StartFinishScope`	netcoreapp3.1	814ns	0.147ns	0.55ns	0.00883	664 B
master	`StartFinishScope`	net472	945ns	0.899ns	3.48ns	0.0994	626 B
#5214	`StartFinishSpan`	net6.0	516ns	0.0933ns	0.349ns	0.00755	544 B
#5214	`StartFinishSpan`	netcoreapp3.1	788ns	0.626ns	2.42ns	0.00751	544 B
#5214	`StartFinishSpan`	net472	802ns	0.389ns	1.51ns	0.0866	546 B
#5214	`StartFinishScope`	net6.0	555ns	0.155ns	0.602ns	0.00932	664 B
#5214	`StartFinishScope`	netcoreapp3.1	875ns	0.369ns	1.43ns	0.0092	664 B
#5214	`StartFinishScope`	net472	1μs	0.485ns	1.88ns	0.0992	626 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	699ns	0.453ns	1.76ns	0.00912	664 B
master	`RunOnMethodBegin`	netcoreapp3.1	921ns	0.265ns	0.957ns	0.00898	664 B
master	`RunOnMethodBegin`	net472	1.08μs	0.662ns	2.56ns	0.099	626 B
#5214	`RunOnMethodBegin`	net6.0	707ns	0.275ns	1.07ns	0.0092	664 B
#5214	`RunOnMethodBegin`	netcoreapp3.1	991ns	0.647ns	2.33ns	0.009	664 B
#5214	`RunOnMethodBegin`	net472	1.12μs	0.922ns	3.57ns	0.0995	626 B

bouwkast

Well the non-reverse duck type way here is a lot easier to read :)

I just had a few small comments and questions, but looked good to me

bouwkast · 2024-02-27T22:43:33Z

...Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/Ci/Proxies/IBenchmarkHostInfo.cs

@@ -4,28 +4,39 @@
 // </copyright>

 #nullable enable
+using Datadog.Trace.DuckTyping;
+
 namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Ci.Proxies;

 /// <summary>
 /// Reverse duck type for Datadog.Trace.Ci.BenchmarkHostInfo in Datadog.Trace.Manual


nitpick

Suggested change

/// Reverse duck type for Datadog.Trace.Ci.BenchmarkHostInfo in Datadog.Trace.Manual

/// Duck type for Datadog.Trace.Ci.BenchmarkHostInfo in Datadog.Trace.Manual

...iler/AutoInstrumentation/ManualInstrumentation/Extensions/SpanExtensionsSetTagIntegration.cs

...er/AutoInstrumentation/ManualInstrumentation/Ci/TestExtensionsAddBenchmarkDataIntegration.cs

bouwkast · 2024-02-27T23:52:57Z

...utoInstrumentation/ManualInstrumentation/Ci/TestExtensionsSetBenchmarkMetadataIntegration.cs

+    // internal static CallTargetState OnMethodBegin<TTarget, TTest, TStats>(TTest test, BenchmarkMeasureType measureType, string info, in TStats statistics)
+    //     where TStats : IBenchmarkDiscreteStats


nit: Think these can be deleted

Suggested change

// internal static CallTargetState OnMethodBegin<TTarget, TTest, TStats>(TTest test, BenchmarkMeasureType measureType, string info, in TStats statistics)

// where TStats : IBenchmarkDiscreteStats

Oops, yep, thanks!

andrewlock · 2024-03-04T08:52:49Z

Additional comments and suggestions can be found on #5235 and have been applied in that branch for simplicity

…yping (#5214) * Fix null ref in `GetManagedTracerVersion()` * Add benchmark for manual instrumentation * Try alternative ducktyping approach * Fix OpenTracing helpers * Revert the changes to benchmark stat types Seeing as we're not reverse duck-typing any more, we don't need to do this * Update manual Ci Visibility to not rely on reverse duck-typing This is made rather irritating by the fact we _can't_ ducktype Test as an ITest. That, coupled with the APIs that connect module + suite + test together means we need to duplicate the interfaces in their entirety. It seems to work though 🎉 * Revert the snapshots to match the existing public API * Use instrumentation of DuckType.DuckCast<T> in Datadog.Trace.Manual.csproj Means we can avoid having to include the whole DuckTyping library inside Datadog.Trace.Manual The downside is that it's hard to test in unit tests. as everything requires auto instrumentation * Skip the duck typing tests that won't work and add some others * Simplify the implementation Instead of having wrapper ManualScope implementations etc, we can just use duck typing instead directly. This reduces one level of indirection and reduces the allocations * Redesign approach with CI Visibility - Use the standard duck typing approach instead of reverse duck typing - Remove the ManualTest etc - Move methods that we _can't_ duck type to be extension methods that we instrument * Update generated code * Apply suggestions from code review Co-authored-by: Kevin Gosse <kevin.gosse@datadoghq.com> * PR feedback - Fix previous PR feedback suggestions ;) - Add [Instrumented] to Tracer.ctor - Fix merge conflict in IntegrationIdExtensions - Add missing telemetry integration - Move TracerSettingsPopulateDictionaryIntegration into correct folder, and rename to PopulateDictionaryIntegration - Add a unit test for SetTag * Add unit tests for the mapping between tracersettings and public API settings * PR Feedback - add helper class for serialization of integration settings * PR feedback - include initial value in OverrideValue<T> helper * PR feedback - too many braces * Fix bug introduced * Use defaults for null test suite * Fix merge conflict --------- Co-authored-by: Kevin Gosse <kevin.gosse@datadoghq.com>

andrewlock requested review from a team as code owners February 20, 2024 11:36

andrewlock force-pushed the andrew/v3-public-apis/change-ducktyping-approach branch from 4d47166 to a90f420 Compare February 20, 2024 11:37

This was referenced Feb 20, 2024

[v3] Implement manual tracing functionality in Datadog.Trace.Manual #5072

Merged

[v3] Update OpenTracing library to use new public APIs #5073

Merged

[v3] Update manual throughput tests #5183

Merged

andrewlock force-pushed the andrew/v3-public-apis/opentracing branch from 7dab6cf to 7c55190 Compare February 23, 2024 10:18

andrewlock force-pushed the andrew/v3-public-apis/change-ducktyping-approach branch from a90f420 to b3e4322 Compare February 23, 2024 10:18

andrewlock mentioned this pull request Feb 23, 2024

[v3] Stack for kevin #5235

Closed

andrewlock force-pushed the andrew/v3-public-apis/opentracing branch from 7c55190 to 9030290 Compare February 23, 2024 14:50

andrewlock requested review from a team as code owners February 23, 2024 14:50

andrewlock force-pushed the andrew/v3-public-apis/change-ducktyping-approach branch 2 times, most recently from 9859635 to 564da04 Compare February 27, 2024 15:50

andrewlock added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:ci-visibility labels Feb 27, 2024

bouwkast approved these changes Feb 27, 2024

View reviewed changes

andrewlock force-pushed the andrew/v3-public-apis/opentracing branch from 9030290 to c63b1a6 Compare February 29, 2024 17:39

andrewlock force-pushed the andrew/v3-public-apis/change-ducktyping-approach branch from 12ab014 to 446cee0 Compare February 29, 2024 17:40

kevingosse approved these changes Mar 1, 2024

View reviewed changes

andrewlock force-pushed the andrew/v3-public-apis/opentracing branch from c63b1a6 to 4dac33a Compare March 1, 2024 13:25

andrewlock force-pushed the andrew/v3-public-apis/change-ducktyping-approach branch from 8156f3d to 622dc54 Compare March 1, 2024 13:25

andrewlock force-pushed the andrew/v3-public-apis/opentracing branch from 4dac33a to cc60b60 Compare March 4, 2024 08:52

An error occurred while trying to automatically change base from andrew/v3-public-apis/opentracing to v3-main March 4, 2024 08:52

andrewlock added this to the vNext-v3 milestone Apr 22, 2024

	/// Reverse duck type for Datadog.Trace.Ci.BenchmarkHostInfo in Datadog.Trace.Manual
	/// Duck type for Datadog.Trace.Ci.BenchmarkHostInfo in Datadog.Trace.Manual

		// internal static CallTargetState OnMethodBegin<TTarget, TTest, TStats>(TTest test, BenchmarkMeasureType measureType, string info, in TStats statistics)
		// where TStats : IBenchmarkDiscreteStats

[v3] Change manual instrumentation approach to not use reverse duck-typing #5214

[v3] Change manual instrumentation approach to not use reverse duck-typing #5214

Conversation

andrewlock commented Feb 20, 2024 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

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

Datadog Report

❌ Failed Tests (1)

andrewlock commented Feb 20, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Feb 20, 2024

Throughput/Crank Report:zap:

andrewlock commented Feb 20, 2024

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Slower ⚠️ in #5214

Faster 🎉 in #5214

Raw results

Raw results

Raw results

Raw results

More allocations ⚠️ in #5214

Raw results

Slower ⚠️ in #5214

Raw results

Raw results

Raw results

Raw results

More allocations ⚠️ in #5214

Fewer allocations 🎉 in #5214

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #5214

Raw results

Raw results

bouwkast left a comment

Choose a reason for hiding this comment

bouwkast Feb 27, 2024

Choose a reason for hiding this comment

bouwkast Feb 27, 2024

Choose a reason for hiding this comment

andrewlock Feb 28, 2024

Choose a reason for hiding this comment

andrewlock commented Mar 4, 2024

andrewlock commented Feb 20, 2024 •

edited

Loading

datadog-ddstaging bot commented Feb 20, 2024 •

edited

Loading

andrewlock commented Feb 20, 2024 •

edited

Loading