-
Notifications
You must be signed in to change notification settings - Fork 151
Removing V1 tags class from AWS integrations #7746
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
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (7746) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (77ms) : 70, 84
master - mean (75ms) : 70, 79
section Bailout
This PR (7746) - mean (80ms) : 76, 85
master - mean (79ms) : 73, 85
section CallTarget+Inlining+NGEN
This PR (7746) - mean (1,107ms) : 1039, 1175
master - mean (1,114ms) : 1019, 1208
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (119ms) : 113, 124
master - mean (117ms) : 111, 124
section Bailout
This PR (7746) - mean (118ms) : 111, 125
master - mean (120ms) : 112, 128
section CallTarget+Inlining+NGEN
This PR (7746) - mean (809ms) : 756, 861
master - mean (801ms) : 766, 835
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (104ms) : 96, 112
master - mean (104ms) : 97, 111
section Bailout
This PR (7746) - mean (106ms) : 99, 112
master - mean (104ms) : 96, 112
section CallTarget+Inlining+NGEN
This PR (7746) - mean (742ms) : 710, 774
master - mean (757ms) : 696, 817
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (104ms) : 97, 110
master - mean (102ms) : 95, 109
section Bailout
This PR (7746) - mean (104ms) : 99, 109
master - mean (103ms) : 96, 110
section CallTarget+Inlining+NGEN
This PR (7746) - mean (701ms) : 676, 725
master - mean (717ms) : 681, 753
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (194ms) : 189, 199
master - mean (193ms) : 189, 197
section Bailout
This PR (7746) - mean (197ms) : 194, 200
master - mean (196ms) : 193, 198
section CallTarget+Inlining+NGEN
This PR (7746) - mean (1,170ms) : 1113, 1226
master - mean (1,160ms) : 1100, 1220
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (277ms) : 272, 282
master - mean (276ms) : 272, 281
section Bailout
This PR (7746) - mean (279ms) : 274, 283
master - mean (277ms) : 272, 281
section CallTarget+Inlining+NGEN
This PR (7746) - mean (953ms) : 913, 993
master - mean (951ms) : 907, 995
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (270ms) : 265, 275
master - mean (269ms) : 264, 275
section Bailout
This PR (7746) - mean (271ms) : 267, 274
master - mean (269ms) : 265, 273
section CallTarget+Inlining+NGEN
This PR (7746) - mean (944ms) : 881, 1007
master - mean (925ms) : 876, 975
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7746) - mean (276ms) : 266, 286
master - mean (268ms) : 264, 272
section Bailout
This PR (7746) - mean (275ms) : 264, 286
master - mean (269ms) : 264, 273
section CallTarget+Inlining+NGEN
This PR (7746) - mean (858ms) : 833, 883
master - mean (853ms) : 836, 869
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmarks Report for benchmark platform 🐌Benchmarks for #7746 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ More allocations
|
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.ActivityBenchmark.StartStopWithChild‑netcoreapp3.1 | 5.68 KB | 5.73 KB | 48 B | 0.84% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StartStopWithChild |
net6.0 | 10.7μs | 58.8ns | 338ns | 0 | 0 | 0 | 5.51 KB |
| master | StartStopWithChild |
netcoreapp3.1 | 14.3μs | 71.8ns | 337ns | 0 | 0 | 0 | 5.68 KB |
| master | StartStopWithChild |
net472 | 22μs | 124ns | 792ns | 1 | 0.334 | 0.111 | 6.06 KB |
| #7746 | StartStopWithChild |
net6.0 | 11.2μs | 56.5ns | 271ns | 0 | 0 | 0 | 5.54 KB |
| #7746 | StartStopWithChild |
netcoreapp3.1 | 14μs | 67.7ns | 295ns | 0 | 0 | 0 | 5.73 KB |
| #7746 | StartStopWithChild |
net472 | 21.8μs | 109ns | 509ns | 0.983 | 0.328 | 0.109 | 6.09 KB |
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Fewer allocations 🎉
Fewer allocations 🎉 in #7746
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces‑net472
3.35 KB
3.31 KB
-46 B
-1.37%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 | 3.35 KB | 3.31 KB | -46 B | -1.37% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | WriteAndFlushEnrichedTraces |
net6.0 | 952μs | 280ns | 1.08μs | 0 | 0 | 0 | 2.72 KB |
| master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 1.03ms | 170ns | 660ns | 0 | 0 | 0 | 2.7 KB |
| master | WriteAndFlushEnrichedTraces |
net472 | 1.2ms | 77ns | 278ns | 0 | 0 | 0 | 3.35 KB |
| #7746 | WriteAndFlushEnrichedTraces |
net6.0 | 928μs | 273ns | 983ns | 0 | 0 | 0 | 2.71 KB |
| #7746 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 1.03ms | 146ns | 564ns | 0 | 0 | 0 | 2.7 KB |
| #7746 | WriteAndFlushEnrichedTraces |
net472 | 1.21ms | 249ns | 965ns | 0 | 0 | 0 | 3.31 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 | 1.09μs | 6.29ns | 50.3ns | 0 | 0 | 0 | 1.22 KB |
| master | AllCycleSimpleBody |
netcoreapp3.1 | 1.45μs | 8.23ns | 57ns | 0 | 0 | 0 | 1.2 KB |
| master | AllCycleSimpleBody |
net472 | 1.05μs | 0.276ns | 1.03ns | 0.191 | 0 | 0 | 1.23 KB |
| master | AllCycleMoreComplexBody |
net6.0 | 6.99μs | 38.6ns | 222ns | 0 | 0 | 0 | 4.72 KB |
| master | AllCycleMoreComplexBody |
netcoreapp3.1 | 8.98μs | 22.8ns | 88.3ns | 0 | 0 | 0 | 4.62 KB |
| master | AllCycleMoreComplexBody |
net472 | 7.64μs | 6.04ns | 23.4ns | 0.729 | 0 | 0 | 4.74 KB |
| master | ObjectExtractorSimpleBody |
net6.0 | 334ns | 0.361ns | 1.4ns | 0 | 0 | 0 | 280 B |
| master | ObjectExtractorSimpleBody |
netcoreapp3.1 | 400ns | 1.93ns | 8.2ns | 0 | 0 | 0 | 272 B |
| master | ObjectExtractorSimpleBody |
net472 | 296ns | 0.0186ns | 0.0644ns | 0.0434 | 0 | 0 | 281 B |
| master | ObjectExtractorMoreComplexBody |
net6.0 | 6.33μs | 4.04ns | 15.6ns | 0 | 0 | 0 | 3.78 KB |
| master | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 7.72μs | 36.4ns | 150ns | 0 | 0 | 0 | 3.69 KB |
| master | ObjectExtractorMoreComplexBody |
net472 | 6.71μs | 2.01ns | 7.52ns | 0.574 | 0 | 0 | 3.8 KB |
| #7746 | AllCycleSimpleBody |
net6.0 | 1.09μs | 2.25ns | 8.7ns | 0 | 0 | 0 | 1.22 KB |
| #7746 | AllCycleSimpleBody |
netcoreapp3.1 | 1.39μs | 7.76ns | 51.5ns | 0 | 0 | 0 | 1.2 KB |
| #7746 | AllCycleSimpleBody |
net472 | 1.01μs | 0.582ns | 2.18ns | 0.195 | 0 | 0 | 1.23 KB |
| #7746 | AllCycleMoreComplexBody |
net6.0 | 7.05μs | 36.7ns | 176ns | 0 | 0 | 0 | 4.72 KB |
| #7746 | AllCycleMoreComplexBody |
netcoreapp3.1 | 9.36μs | 13ns | 50.2ns | 0 | 0 | 0 | 4.62 KB |
| #7746 | AllCycleMoreComplexBody |
net472 | 7.69μs | 3.69ns | 14.3ns | 0.728 | 0 | 0 | 4.74 KB |
| #7746 | ObjectExtractorSimpleBody |
net6.0 | 327ns | 0.0943ns | 0.353ns | 0 | 0 | 0 | 280 B |
| #7746 | ObjectExtractorSimpleBody |
netcoreapp3.1 | 397ns | 2.16ns | 12.6ns | 0 | 0 | 0 | 272 B |
| #7746 | ObjectExtractorSimpleBody |
net472 | 297ns | 0.032ns | 0.115ns | 0.0433 | 0 | 0 | 281 B |
| #7746 | ObjectExtractorMoreComplexBody |
net6.0 | 6.3μs | 29.5ns | 114ns | 0 | 0 | 0 | 3.78 KB |
| #7746 | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 7.74μs | 27ns | 104ns | 0 | 0 | 0 | 3.69 KB |
| #7746 | ObjectExtractorMoreComplexBody |
net472 | 6.68μs | 2.23ns | 8.63ns | 0.601 | 0 | 0 | 3.8 KB |
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EncodeArgs |
net6.0 | 77.4μs | 32.7ns | 118ns | 0 | 0 | 0 | 32.4 KB |
| master | EncodeArgs |
netcoreapp3.1 | 97.8μs | 36.1ns | 140ns | 0 | 0 | 0 | 32.4 KB |
| master | EncodeArgs |
net472 | 110μs | 12.1ns | 46.8ns | 4.96 | 0 | 0 | 32.51 KB |
| master | EncodeLegacyArgs |
net6.0 | 146μs | 194ns | 752ns | 0 | 0 | 0 | 2.15 KB |
| master | EncodeLegacyArgs |
netcoreapp3.1 | 197μs | 168ns | 650ns | 0 | 0 | 0 | 2.15 KB |
| master | EncodeLegacyArgs |
net472 | 263μs | 212ns | 735ns | 0 | 0 | 0 | 2.16 KB |
| #7746 | EncodeArgs |
net6.0 | 77.6μs | 216ns | 837ns | 0 | 0 | 0 | 32.4 KB |
| #7746 | EncodeArgs |
netcoreapp3.1 | 97.1μs | 404ns | 1.56μs | 0 | 0 | 0 | 32.4 KB |
| #7746 | EncodeArgs |
net472 | 109μs | 8.36ns | 32.4ns | 4.9 | 0 | 0 | 32.51 KB |
| #7746 | EncodeLegacyArgs |
net6.0 | 146μs | 17.8ns | 66.6ns | 0 | 0 | 0 | 2.15 KB |
| #7746 | EncodeLegacyArgs |
netcoreapp3.1 | 197μs | 231ns | 895ns | 0 | 0 | 0 | 2.14 KB |
| #7746 | EncodeLegacyArgs |
net472 | 266μs | 49.4ns | 185ns | 0 | 0 | 0 | 2.16 KB |
Benchmarks.Trace.Asm.AppSecWafBenchmark - Slower ⚠️ Same allocations ✔️
Slower ⚠️ in #7746
Benchmark
diff/base
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark‑netcoreapp3.1
2.072
412,767.50
855,110.83
| Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark‑netcoreapp3.1 | 2.072 | 412,767.50 | 855,110.83 |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunWafRealisticBenchmark |
net6.0 | 406μs | 111ns | 400ns | 0 | 0 | 0 | 4.55 KB |
| master | RunWafRealisticBenchmark |
netcoreapp3.1 | 413μs | 229ns | 885ns | 0 | 0 | 0 | 4.48 KB |
| master | RunWafRealisticBenchmark |
net472 | 447μs | 101ns | 391ns | 0 | 0 | 0 | 4.66 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net6.0 | 294μs | 118ns | 457ns | 0 | 0 | 0 | 2.24 KB |
| master | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 299μs | 161ns | 556ns | 0 | 0 | 0 | 2.22 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net472 | 319μs | 49.9ns | 180ns | 0 | 0 | 0 | 2.28 KB |
| #7746 | RunWafRealisticBenchmark |
net6.0 | 425μs | 241ns | 902ns | 0 | 0 | 0 | 4.56 KB |
| #7746 | RunWafRealisticBenchmark |
netcoreapp3.1 | 806μs | 14μs | 139μs | 0 | 0 | 0 | 4.48 KB |
| #7746 | RunWafRealisticBenchmark |
net472 | 459μs | 89ns | 345ns | 0 | 0 | 0 | 4.66 KB |
| #7746 | RunWafRealisticBenchmarkWithAttack |
net6.0 | 308μs | 283ns | 1.1μs | 0 | 0 | 0 | 2.24 KB |
| #7746 | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 298μs | 70.4ns | 263ns | 0 | 0 | 0 | 2.22 KB |
| #7746 | RunWafRealisticBenchmarkWithAttack |
net472 | 338μs | 79.2ns | 296ns | 0 | 0 | 0 | 2.28 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 | 62.2μs | 87.6ns | 303ns | 0 | 0 | 0 | 14.52 KB |
| master | SendRequest |
netcoreapp3.1 | 71.8μs | 76.2ns | 285ns | 0 | 0 | 0 | 17.42 KB |
| master | SendRequest |
net472 | 0.0175ns | 0.00341ns | 0.0132ns | 0 | 0 | 0 | 0 b |
| #7746 | SendRequest |
net6.0 | 61.1μs | 37ns | 143ns | 0 | 0 | 0 | 14.52 KB |
| #7746 | SendRequest |
netcoreapp3.1 | 71.9μs | 77.9ns | 302ns | 0 | 0 | 0 | 17.42 KB |
| #7746 | SendRequest |
net472 | 0.00327ns | 0.0015ns | 0.00581ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CharSliceBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7746
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0
2 B
5 B
3 B
150.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0
2 B
4 B
2 B
100.00%
Fewer allocations 🎉 in #7746
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net472
73 B
0 b
-73 B
-100.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472
47 B
0 b
-47 B
-100.00%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0 | 2 B | 5 B | 3 B | 150.00% |
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0 | 2 B | 4 B | 2 B | 100.00% |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net472 | 73 B | 0 b | -73 B | -100.00% |
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472 | 47 B | 0 b | -47 B | -100.00% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | OriginalCharSlice |
net6.0 | 1.86ms | 3.83μs | 14.9μs | 0 | 0 | 0 | 640 KB |
| master | OriginalCharSlice |
netcoreapp3.1 | 2.05ms | 4.47μs | 16.1μs | 0 | 0 | 0 | 640 KB |
| master | OriginalCharSlice |
net472 | 2.69ms | 2.18μs | 8.46μs | 100 | 0 | 0 | 641.95 KB |
| master | OptimizedCharSlice |
net6.0 | 1.44ms | 1.36μs | 5.28μs | 0 | 0 | 0 | 2 B |
| master | OptimizedCharSlice |
netcoreapp3.1 | 1.65ms | 264ns | 989ns | 0 | 0 | 0 | 1 B |
| master | OptimizedCharSlice |
net472 | 1.9ms | 298ns | 1.12μs | 0 | 0 | 0 | 73 B |
| master | OptimizedCharSliceWithPool |
net6.0 | 820μs | 66.1ns | 256ns | 0 | 0 | 0 | 2 B |
| master | OptimizedCharSliceWithPool |
netcoreapp3.1 | 811μs | 61.5ns | 238ns | 0 | 0 | 0 | 1 B |
| master | OptimizedCharSliceWithPool |
net472 | 1.19ms | 98.4ns | 381ns | 0 | 0 | 0 | 47 B |
| #7746 | OriginalCharSlice |
net6.0 | 2.04ms | 1.39μs | 5.4μs | 0 | 0 | 0 | 640.01 KB |
| #7746 | OriginalCharSlice |
netcoreapp3.1 | 2.2ms | 6μs | 38μs | 0 | 0 | 0 | 640 KB |
| #7746 | OriginalCharSlice |
net472 | 2.64ms | 157ns | 566ns | 100 | 0 | 0 | 641.95 KB |
| #7746 | OptimizedCharSlice |
net6.0 | 1.52ms | 152ns | 590ns | 0 | 0 | 0 | 4 B |
| #7746 | OptimizedCharSlice |
netcoreapp3.1 | 1.67ms | 637ns | 2.47μs | 0 | 0 | 0 | 1 B |
| #7746 | OptimizedCharSlice |
net472 | 1.99ms | 293ns | 1.14μs | 0 | 0 | 0 | 0 b |
| #7746 | OptimizedCharSliceWithPool |
net6.0 | 867μs | 92.5ns | 358ns | 0 | 0 | 0 | 5 B |
| #7746 | OptimizedCharSliceWithPool |
netcoreapp3.1 | 810μs | 131ns | 507ns | 0 | 0 | 0 | 1 B |
| #7746 | OptimizedCharSliceWithPool |
net472 | 1.15ms | 107ns | 401ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7746
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0
41.57 KB
42.59 KB
1.02 KB
2.44%
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472
55.98 KB
56.32 KB
342 B
0.61%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 | 41.57 KB | 42.59 KB | 1.02 KB | 2.44% |
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 | 55.98 KB | 56.32 KB | 342 B | 0.61% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | WriteAndFlushEnrichedTraces |
net6.0 | 747μs | 1.15μs | 4.45μs | 0 | 0 | 0 | 41.57 KB |
| master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 772μs | 5.49μs | 54.9μs | 0 | 0 | 0 | 41.76 KB |
| master | WriteAndFlushEnrichedTraces |
net472 | 931μs | 4.28μs | 16.6μs | 8.33 | 0 | 0 | 55.98 KB |
| #7746 | WriteAndFlushEnrichedTraces |
net6.0 | 697μs | 1.93μs | 7.23μs | 0 | 0 | 0 | 42.59 KB |
| #7746 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 753μs | 4.33μs | 36μs | 0 | 0 | 0 | 41.7 KB |
| #7746 | WriteAndFlushEnrichedTraces |
net472 | 969μs | 4.84μs | 22.2μs | 8.93 | 4.46 | 0 | 56.32 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.9μs | 9.66ns | 44.3ns | 0 | 0 | 0 | 1.02 KB |
| master | ExecuteNonQuery |
netcoreapp3.1 | 2.62μs | 12.6ns | 53.6ns | 0 | 0 | 0 | 1.02 KB |
| master | ExecuteNonQuery |
net472 | 2.87μs | 3.83ns | 14.3ns | 0.156 | 0.0142 | 0 | 987 B |
| #7746 | ExecuteNonQuery |
net6.0 | 1.85μs | 9.79ns | 49.9ns | 0 | 0 | 0 | 1.02 KB |
| #7746 | ExecuteNonQuery |
netcoreapp3.1 | 2.59μs | 8.73ns | 33.8ns | 0 | 0 | 0 | 1.02 KB |
| #7746 | ExecuteNonQuery |
net472 | 2.88μs | 2.25ns | 8.71ns | 0.143 | 0.0143 | 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.73μs | 1.46ns | 5.65ns | 0 | 0 | 0 | 1.03 KB |
| master | CallElasticsearch |
netcoreapp3.1 | 2.18μs | 11.1ns | 49.8ns | 0 | 0 | 0 | 1.03 KB |
| master | CallElasticsearch |
net472 | 3.53μs | 1.9ns | 6.84ns | 0.158 | 0 | 0 | 1.04 KB |
| master | CallElasticsearchAsync |
net6.0 | 1.84μs | 7.43ns | 28.8ns | 0 | 0 | 0 | 1.01 KB |
| master | CallElasticsearchAsync |
netcoreapp3.1 | 2.36μs | 8.34ns | 32.3ns | 0 | 0 | 0 | 1.08 KB |
| master | CallElasticsearchAsync |
net472 | 3.63μs | 0.776ns | 2.8ns | 0.163 | 0 | 0 | 1.1 KB |
| #7746 | CallElasticsearch |
net6.0 | 1.66μs | 8.81ns | 44.9ns | 0 | 0 | 0 | 1.03 KB |
| #7746 | CallElasticsearch |
netcoreapp3.1 | 2.3μs | 7.99ns | 30.9ns | 0 | 0 | 0 | 1.03 KB |
| #7746 | CallElasticsearch |
net472 | 3.5μs | 6.32ns | 24.5ns | 0.157 | 0 | 0 | 1.04 KB |
| #7746 | CallElasticsearchAsync |
net6.0 | 1.79μs | 7.28ns | 26.2ns | 0 | 0 | 0 | 1.01 KB |
| #7746 | CallElasticsearchAsync |
netcoreapp3.1 | 2.41μs | 10.7ns | 41.5ns | 0 | 0 | 0 | 1.08 KB |
| #7746 | CallElasticsearchAsync |
net472 | 3.64μs | 2.83ns | 10.9ns | 0.164 | 0 | 0 | 1.1 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.87μs | 4.01ns | 15.5ns | 0 | 0 | 0 | 952 B |
| master | ExecuteAsync |
netcoreapp3.1 | 2.51μs | 8.35ns | 32.3ns | 0 | 0 | 0 | 952 B |
| master | ExecuteAsync |
net472 | 2.58μs | 1.31ns | 4.89ns | 0.141 | 0 | 0 | 915 B |
| #7746 | ExecuteAsync |
net6.0 | 1.86μs | 9.18ns | 40ns | 0 | 0 | 0 | 952 B |
| #7746 | ExecuteAsync |
netcoreapp3.1 | 2.51μs | 2.73ns | 10.2ns | 0 | 0 | 0 | 952 B |
| #7746 | ExecuteAsync |
net472 | 2.58μs | 2.09ns | 8.09ns | 0.142 | 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 | 7.19μs | 3.43ns | 12.4ns | 0 | 0 | 0 | 2.36 KB |
| master | SendAsync |
netcoreapp3.1 | 8.67μs | 19.4ns | 75ns | 0 | 0 | 0 | 2.9 KB |
| master | SendAsync |
net472 | 12.5μs | 8.63ns | 33.4ns | 0.497 | 0 | 0 | 3.18 KB |
| #7746 | SendAsync |
net6.0 | 6.79μs | 19.8ns | 74ns | 0 | 0 | 0 | 2.36 KB |
| #7746 | SendAsync |
netcoreapp3.1 | 8.42μs | 14.1ns | 54.7ns | 0 | 0 | 0 | 2.9 KB |
| #7746 | SendAsync |
net472 | 11.9μs | 10.2ns | 39.4ns | 0.474 | 0 | 0 | 3.18 KB |
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7746
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472
57.34 KB
65.54 KB
8.19 KB
14.29%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net472
275.24 KB
294.91 KB
19.67 KB
7.15%
Fewer allocations 🎉 in #7746
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net6.0
45.32 KB
43.83 KB
-1.49 KB
-3.28%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0
274.41 KB
259.12 KB
-15.29 KB
-5.57%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1
275.95 KB
258.69 KB
-17.26 KB
-6.26%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 | 57.34 KB | 65.54 KB | 8.19 KB | 14.29% |
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net472 | 275.24 KB | 294.91 KB | 19.67 KB | 7.15% |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net6.0 | 45.32 KB | 43.83 KB | -1.49 KB | -3.28% |
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 | 274.41 KB | 259.12 KB | -15.29 KB | -5.57% |
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 275.95 KB | 258.69 KB | -17.26 KB | -6.26% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StringConcatBenchmark |
net6.0 | 44.9μs | 314ns | 2.93μs | 0 | 0 | 0 | 45.32 KB |
| master | StringConcatBenchmark |
netcoreapp3.1 | 51.9μs | 506ns | 4.83μs | 0 | 0 | 0 | 42.9 KB |
| master | StringConcatBenchmark |
net472 | 56.5μs | 270ns | 1.05μs | 0 | 0 | 0 | 57.34 KB |
| master | StringConcatAspectBenchmark |
net6.0 | 470μs | 1.99μs | 7.18μs | 0 | 0 | 0 | 274.41 KB |
| master | StringConcatAspectBenchmark |
netcoreapp3.1 | 468μs | 5.37μs | 52.3μs | 0 | 0 | 0 | 275.95 KB |
| master | StringConcatAspectBenchmark |
net472 | 417μs | 2.31μs | 14.4μs | 0 | 0 | 0 | 275.24 KB |
| #7746 | StringConcatBenchmark |
net6.0 | 44.6μs | 262ns | 2.2μs | 0 | 0 | 0 | 43.83 KB |
| #7746 | StringConcatBenchmark |
netcoreapp3.1 | 50.8μs | 300ns | 2.63μs | 0 | 0 | 0 | 42.83 KB |
| #7746 | StringConcatBenchmark |
net472 | 57.3μs | 101ns | 365ns | 0 | 0 | 0 | 65.54 KB |
| #7746 | StringConcatAspectBenchmark |
net6.0 | 456μs | 2.21μs | 8.86μs | 0 | 0 | 0 | 259.12 KB |
| #7746 | StringConcatAspectBenchmark |
netcoreapp3.1 | 503μs | 1.67μs | 6.01μs | 0 | 0 | 0 | 258.69 KB |
| #7746 | StringConcatAspectBenchmark |
net472 | 398μs | 1.72μs | 7.49μs | 0 | 0 | 0 | 294.91 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 | 2.67μs | 13.5ns | 58.8ns | 0 | 0 | 0 | 1.7 KB |
| master | EnrichedLog |
netcoreapp3.1 | 3.6μs | 17.5ns | 74.4ns | 0 | 0 | 0 | 1.7 KB |
| master | EnrichedLog |
net472 | 4.02μs | 4.38ns | 16.4ns | 0.241 | 0 | 0 | 1.64 KB |
| #7746 | EnrichedLog |
net6.0 | 2.65μs | 12.1ns | 49.7ns | 0 | 0 | 0 | 1.7 KB |
| #7746 | EnrichedLog |
netcoreapp3.1 | 3.58μs | 16.2ns | 62.6ns | 0 | 0 | 0 | 1.7 KB |
| #7746 | EnrichedLog |
net472 | 4μs | 3.56ns | 13.8ns | 0.24 | 0 | 0 | 1.64 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 | 121μs | 64.4ns | 241ns | 0 | 0 | 0 | 4.31 KB |
| master | EnrichedLog |
netcoreapp3.1 | 128μs | 244ns | 944ns | 0 | 0 | 0 | 4.31 KB |
| master | EnrichedLog |
net472 | 167μs | 105ns | 406ns | 0 | 0 | 0 | 4.52 KB |
| #7746 | EnrichedLog |
net6.0 | 123μs | 122ns | 440ns | 0 | 0 | 0 | 4.31 KB |
| #7746 | EnrichedLog |
netcoreapp3.1 | 130μs | 560ns | 2.17μs | 0 | 0 | 0 | 4.31 KB |
| #7746 | EnrichedLog |
net472 | 169μs | 251ns | 972ns | 0 | 0 | 0 | 4.52 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 | 5.13μs | 7.62ns | 28.5ns | 0 | 0 | 0 | 2.26 KB |
| master | EnrichedLog |
netcoreapp3.1 | 6.77μs | 16.5ns | 63.7ns | 0 | 0 | 0 | 2.26 KB |
| master | EnrichedLog |
net472 | 7.49μs | 5.68ns | 22ns | 0.299 | 0 | 0 | 2.08 KB |
| #7746 | EnrichedLog |
net6.0 | 5.09μs | 8.3ns | 32.2ns | 0 | 0 | 0 | 2.26 KB |
| #7746 | EnrichedLog |
netcoreapp3.1 | 7.07μs | 8.96ns | 33.5ns | 0 | 0 | 0 | 2.26 KB |
| #7746 | EnrichedLog |
net472 | 7.48μs | 5.56ns | 20.8ns | 0.3 | 0 | 0 | 2.08 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.93μs | 10ns | 49.1ns | 0 | 0 | 0 | 1.2 KB |
| master | SendReceive |
netcoreapp3.1 | 2.65μs | 12.4ns | 48.2ns | 0 | 0 | 0 | 1.2 KB |
| master | SendReceive |
net472 | 3.05μs | 1.37ns | 5.12ns | 0.182 | 0 | 0 | 1.2 KB |
| #7746 | SendReceive |
net6.0 | 2.06μs | 5.67ns | 22ns | 0 | 0 | 0 | 1.2 KB |
| #7746 | SendReceive |
netcoreapp3.1 | 2.76μs | 13ns | 50.4ns | 0 | 0 | 0 | 1.2 KB |
| #7746 | SendReceive |
net472 | 3.02μs | 2.38ns | 9.21ns | 0.181 | 0 | 0 | 1.2 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 | 4.25μs | 16.5ns | 59.5ns | 0 | 0 | 0 | 1.58 KB |
| master | EnrichedLog |
netcoreapp3.1 | 5.64μs | 11.1ns | 42.9ns | 0 | 0 | 0 | 1.63 KB |
| master | EnrichedLog |
net472 | 6.55μs | 5.53ns | 21.4ns | 0.293 | 0 | 0 | 2.03 KB |
| #7746 | EnrichedLog |
net6.0 | 4.37μs | 3.06ns | 11ns | 0 | 0 | 0 | 1.58 KB |
| #7746 | EnrichedLog |
netcoreapp3.1 | 5.65μs | 8.15ns | 31.6ns | 0 | 0 | 0 | 1.63 KB |
| #7746 | EnrichedLog |
net472 | 6.57μs | 4.98ns | 18.7ns | 0.298 | 0 | 0 | 2.03 KB |
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StartFinishSpan |
net6.0 | 785ns | 0.322ns | 1.25ns | 0 | 0 | 0 | 576 B |
| master | StartFinishSpan |
netcoreapp3.1 | 966ns | 4.8ns | 22.5ns | 0 | 0 | 0 | 576 B |
| master | StartFinishSpan |
net472 | 920ns | 0.57ns | 2.21ns | 0.0878 | 0 | 0 | 578 B |
| master | StartFinishScope |
net6.0 | 924ns | 1.26ns | 4.86ns | 0 | 0 | 0 | 696 B |
| master | StartFinishScope |
netcoreapp3.1 | 1.24μs | 5.83ns | 22.6ns | 0 | 0 | 0 | 696 B |
| master | StartFinishScope |
net472 | 1.11μs | 1.01ns | 3.63ns | 0.101 | 0 | 0 | 658 B |
| #7746 | StartFinishSpan |
net6.0 | 770ns | 0.643ns | 2.49ns | 0 | 0 | 0 | 576 B |
| #7746 | StartFinishSpan |
netcoreapp3.1 | 999ns | 5.1ns | 22.2ns | 0 | 0 | 0 | 576 B |
| #7746 | StartFinishSpan |
net472 | 919ns | 0.742ns | 2.87ns | 0.0912 | 0 | 0 | 578 B |
| #7746 | StartFinishScope |
net6.0 | 931ns | 0.351ns | 1.31ns | 0 | 0 | 0 | 696 B |
| #7746 | StartFinishScope |
netcoreapp3.1 | 1.21μs | 6.27ns | 30.7ns | 0 | 0 | 0 | 696 B |
| #7746 | StartFinishScope |
net472 | 1.12μs | 0.93ns | 3.6ns | 0.101 | 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 | 1.07μs | 5.35ns | 22.7ns | 0 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
netcoreapp3.1 | 1.42μs | 4.5ns | 17.4ns | 0 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
net472 | 1.51μs | 1.53ns | 5.71ns | 0.0981 | 0 | 0 | 658 B |
| #7746 | RunOnMethodBegin |
net6.0 | 1.07μs | 5.72ns | 31.9ns | 0 | 0 | 0 | 696 B |
| #7746 | RunOnMethodBegin |
netcoreapp3.1 | 1.45μs | 7.7ns | 38.5ns | 0 | 0 | 0 | 696 B |
| #7746 | RunOnMethodBegin |
net472 | 1.47μs | 0.51ns | 1.98ns | 0.102 | 0 | 0 | 658 B |
only delete tracer/src/Datadog.Trace/Generated/*/Datadog.Trace.SourceGenerators/TagListGenerator/*V1Tags.g.cs
…dd-trace-dotnet into rithika.narayan/removing-v1-aws
| [Tag(Trace.Tags.PeerService)] | ||
| public string PeerService { get; set; } | ||
|
|
||
| [Tag(Trace.Tags.PeerServiceSource)] | ||
| public string PeerServiceSource { get; set; } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are planning to set them in future PRs, let's wait until that logic is ready before actually adding the tags empty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, if the tag has no value (null or empty string), we don't serialize it into MessagePack when sending to the agent, so this is pretty harmless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, this work was going to be a single PR, but it was growing too big so @rithikanarayan decided to split it into two PRs. She will be setting these tags in the next PR.
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AWS/AwsDynamoDbTests.cs
Outdated
Show resolved
Hide resolved
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AWS/AwsDynamoDbTests.cs
Outdated
Show resolved
Hide resolved
| span => span.Tags.TryGetValue("component", out var component) && component == "aws-sdk"); | ||
|
|
||
| dynamoDbSpans.Should().NotBeEmpty(); | ||
| ValidateIntegrationSpans(dynamoDbSpans, metadataSchemaVersion, expectedServiceName: clientSpanServiceName, isExternalSpan); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just completely get rid of the metadataSchemaVersion variable? Is there a way to ValidateIntegrationSpans that doesn't make us call this "v0"? Don't really like calling the snapshots v0 because this is now a mix between v0 and v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValidateIntegationSpans is a shared function between many tests, including those that still have V1 schemas, so for readability and consistency between these tests I think we should keep the metadataSchemaVersion. I have removed its uses where possible (AWS-specific functions).
| [Theory] | ||
| [MemberData(nameof(GetAllConfigs))] | ||
| public void CreateAwsS3TagsReturnsCorrectImplementation(object schemaVersionObject, bool peerServiceTagsEnabled, bool removeClientServiceNamesEnabled) | ||
| { | ||
| var schemaVersion = (SchemaVersion)schemaVersionObject; // Unbox SchemaVersion, which is only defined internally | ||
| var expectedType = schemaVersion switch | ||
| { | ||
| SchemaVersion.V0 when peerServiceTagsEnabled == false => typeof(AwsS3Tags), | ||
| _ => typeof(AwsS3V1Tags), | ||
| }; | ||
| var expectedType = typeof(AwsS3Tags); | ||
|
|
||
| var namingSchema = new NamingSchema(schemaVersion, peerServiceTagsEnabled, removeClientServiceNamesEnabled, DefaultServiceName, _mappings, new Dictionary<string, string>()); | ||
| namingSchema.Messaging.CreateAwsS3Tags("spanKind").Should().BeOfType(expectedType); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we skip these tests altogether now? I don't think they are testing much anymore, maybe you were planning on repurposing them in future PRs?
…ducing use of metadataSchemaVersions
| SetEnvironmentVariable("DD_TRACE_SPAN_ATTRIBUTE_SCHEMA", metadataSchemaVersion); | ||
| var isExternalSpan = metadataSchemaVersion == "v0"; | ||
| var clientSpanServiceName = isExternalSpan ? $"{EnvironmentHelper.FullSampleName}-aws-dynamodb" : EnvironmentHelper.FullSampleName; | ||
| string metadataSchemaVersion = "v0"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small nit-pick, when a local variable never changes, we like to make it a constant. It makes it more obvious to readers, and can avoid bugs where someone changes the value later on (the compiler won't let you if it's const).
| string metadataSchemaVersion = "v0"; | |
| const string metadataSchemaVersion = "v0"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't matter much here since this a test, but I think that following guidelines everywhere is a good practice 😅
|
|
||
| dynamoDbSpans.Should().NotBeEmpty(); | ||
| ValidateIntegrationSpans(dynamoDbSpans, metadataSchemaVersion, expectedServiceName: clientSpanServiceName, isExternalSpan); | ||
| ValidateIntegrationSpans(dynamoDbSpans, metadataSchemaVersion, expectedServiceName: clientSpanServiceName, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In C# you can optionally include the parameter name. It can be used to provide arguments out of order or to skip optional parameters, or, in this case, just to document which value you are passing to make the code more readable. It's especially useful when passing a Boolean ("true what??") 😅
| ValidateIntegrationSpans(dynamoDbSpans, metadataSchemaVersion, expectedServiceName: clientSpanServiceName, true); | |
| ValidateIntegrationSpans(dynamoDbSpans, metadataSchemaVersion, expectedServiceName: clientSpanServiceName, isExternalSpan: true); |
lucaspimentel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left small coding-style comments, but feel free to do those in the next PR.
| SchemaVersion.V0 when !_peerServiceTagsEnabled => new AwsS3Tags(), | ||
| _ => new AwsS3V1Tags(spanKind), | ||
| }; | ||
| public AwsS3Tags CreateAwsS3Tags(string spanKind) => new AwsS3Tags(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These types all have constructors that take a spanKind, so we should pass that long.
| public AwsS3Tags CreateAwsS3Tags(string spanKind) => new AwsS3Tags(); | |
| public AwsS3Tags CreateAwsS3Tags(string spanKind) => new AwsS3Tags(spanKind); |
You already did this with AwsKinesisTags and AwsStepFunctionsTags below.
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. 17 occurrences of : - span.kind: client,
+ span.kind: producer,
12 occurrences of : - span.kind: client,
+ span.kind: consumer,
|
Summary of changes
Reason for change
Cleanup of unused V1 code. Preparation for follow up PR addressing Serverless Service Representation for AWS Managed Services, which requires the V0 schema for these services to have access to the peer service and peer service source tags to hardcode their values in serverless environments.
Implementation details
Test coverage
Other details