-
Notifications
You must be signed in to change notification settings - Fork 151
[Test Optimization] Failed Test Replay #7102
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
…s) so there won't be any revert/re-instrumentation for exceptions
6d18493
to
7a33f3e
Compare
eca418e
to
1dde7e3
Compare
BenchmarksBenchmarks Report for benchmark platform 🐌Benchmarks for #7102 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 ✔️ Fewer allocations 🎉
|
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.ActivityBenchmark.StartStopWithChild‑net472 | 6.09 KB | 6.05 KB | -47 B | -0.77% |
Benchmarks.Trace.ActivityBenchmark.StartStopWithChild‑net6.0 | 5.67 KB | 5.5 KB | -167 B | -2.94% |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StartStopWithChild |
net6.0 | 10.4μs | 59.1ns | 426ns | 0 | 0 | 0 | 5.67 KB |
master | StartStopWithChild |
netcoreapp3.1 | 13.1μs | 69ns | 345ns | 0 | 0 | 0 | 5.69 KB |
master | StartStopWithChild |
net472 | 22.5μs | 125ns | 782ns | 1.06 | 0.317 | 0.106 | 6.09 KB |
#7102 | StartStopWithChild |
net6.0 | 10.6μs | 52.8ns | 224ns | 0 | 0 | 0 | 5.5 KB |
#7102 | StartStopWithChild |
netcoreapp3.1 | 13.6μs | 69.7ns | 327ns | 0 | 0 | 0 | 5.67 KB |
#7102 | StartStopWithChild |
net472 | 22.1μs | 113ns | 532ns | 0.906 | 0.226 | 0 | 6.05 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 | 938μs | 90.9ns | 328ns | 0 | 0 | 0 | 2.7 KB |
master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 1.02ms | 1.78μs | 6.9μs | 0 | 0 | 0 | 2.7 KB |
master | WriteAndFlushEnrichedTraces |
net472 | 1.2ms | 42.5ns | 159ns | 0 | 0 | 0 | 3.31 KB |
#7102 | WriteAndFlushEnrichedTraces |
net6.0 | 929μs | 64.1ns | 240ns | 0 | 0 | 0 | 2.71 KB |
#7102 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 1.03ms | 457ns | 1.77μs | 0 | 0 | 0 | 2.7 KB |
#7102 | WriteAndFlushEnrichedTraces |
net472 | 1.19ms | 52ns | 188ns | 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 | 319μs | 960ns | 3.72μs | 0 | 0 | 0 | 172.08 KB |
master | AllCycleSimpleBody |
netcoreapp3.1 | 462μs | 1.43μs | 5.55μs | 0 | 0 | 0 | 174.18 KB |
master | AllCycleSimpleBody |
net472 | 425μs | 94.7ns | 354ns | 29.2 | 0 | 0 | 194.24 KB |
master | AllCycleMoreComplexBody |
net6.0 | 326μs | 1.16μs | 4.35μs | 0 | 0 | 0 | 175.58 KB |
master | AllCycleMoreComplexBody |
netcoreapp3.1 | 480μs | 495ns | 1.92μs | 0 | 0 | 0 | 177.6 KB |
master | AllCycleMoreComplexBody |
net472 | 434μs | 475ns | 1.84μs | 30.2 | 0 | 0 | 197.76 KB |
master | ObjectExtractorSimpleBody |
net6.0 | 323ns | 1.78ns | 10.1ns | 0 | 0 | 0 | 280 B |
master | ObjectExtractorSimpleBody |
netcoreapp3.1 | 406ns | 2.1ns | 9.63ns | 0 | 0 | 0 | 272 B |
master | ObjectExtractorSimpleBody |
net472 | 299ns | 0.086ns | 0.333ns | 0.0435 | 0 | 0 | 281 B |
master | ObjectExtractorMoreComplexBody |
net6.0 | 6.36μs | 2.58ns | 9.98ns | 0 | 0 | 0 | 3.78 KB |
master | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 7.74μs | 38.3ns | 167ns | 0 | 0 | 0 | 3.69 KB |
master | ObjectExtractorMoreComplexBody |
net472 | 6.67μs | 2.01ns | 7.79ns | 0.6 | 0 | 0 | 3.8 KB |
#7102 | AllCycleSimpleBody |
net6.0 | 327μs | 1.05μs | 3.94μs | 0 | 0 | 0 | 172.08 KB |
#7102 | AllCycleSimpleBody |
netcoreapp3.1 | 493μs | 1.23μs | 4.78μs | 0 | 0 | 0 | 174.18 KB |
#7102 | AllCycleSimpleBody |
net472 | 424μs | 123ns | 476ns | 29.2 | 0 | 0 | 194.24 KB |
#7102 | AllCycleMoreComplexBody |
net6.0 | 326μs | 1.47μs | 5.69μs | 0 | 0 | 0 | 175.58 KB |
#7102 | AllCycleMoreComplexBody |
netcoreapp3.1 | 467μs | 1.24μs | 4.81μs | 0 | 0 | 0 | 177.6 KB |
#7102 | AllCycleMoreComplexBody |
net472 | 435μs | 124ns | 466ns | 30.2 | 0 | 0 | 197.76 KB |
#7102 | ObjectExtractorSimpleBody |
net6.0 | 323ns | 0.386ns | 1.5ns | 0 | 0 | 0 | 280 B |
#7102 | ObjectExtractorSimpleBody |
netcoreapp3.1 | 397ns | 2.15ns | 12.3ns | 0 | 0 | 0 | 272 B |
#7102 | ObjectExtractorSimpleBody |
net472 | 302ns | 0.0606ns | 0.235ns | 0.0442 | 0 | 0 | 281 B |
#7102 | ObjectExtractorMoreComplexBody |
net6.0 | 6.28μs | 32ns | 154ns | 0 | 0 | 0 | 3.78 KB |
#7102 | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 7.74μs | 34.2ns | 132ns | 0 | 0 | 0 | 3.69 KB |
#7102 | ObjectExtractorMoreComplexBody |
net472 | 6.7μs | 1.56ns | 5.63ns | 0.602 | 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 | 79μs | 234ns | 905ns | 0 | 0 | 0 | 32.4 KB |
master | EncodeArgs |
netcoreapp3.1 | 96.1μs | 261ns | 1.01μs | 0 | 0 | 0 | 32.4 KB |
master | EncodeArgs |
net472 | 109μs | 9.11ns | 31.6ns | 4.92 | 0 | 0 | 32.5 KB |
master | EncodeLegacyArgs |
net6.0 | 143μs | 18.1ns | 65.3ns | 0 | 0 | 0 | 2.15 KB |
master | EncodeLegacyArgs |
netcoreapp3.1 | 198μs | 233ns | 903ns | 0 | 0 | 0 | 2.14 KB |
master | EncodeLegacyArgs |
net472 | 262μs | 34.2ns | 123ns | 0 | 0 | 0 | 2.16 KB |
#7102 | EncodeArgs |
net6.0 | 76μs | 244ns | 944ns | 0 | 0 | 0 | 32.4 KB |
#7102 | EncodeArgs |
netcoreapp3.1 | 100μs | 257ns | 995ns | 0 | 0 | 0 | 32.4 KB |
#7102 | EncodeArgs |
net472 | 110μs | 40.4ns | 156ns | 4.92 | 0 | 0 | 32.51 KB |
#7102 | EncodeLegacyArgs |
net6.0 | 143μs | 14.4ns | 53.8ns | 0 | 0 | 0 | 2.15 KB |
#7102 | EncodeLegacyArgs |
netcoreapp3.1 | 197μs | 176ns | 658ns | 0 | 0 | 0 | 2.14 KB |
#7102 | EncodeLegacyArgs |
net472 | 262μs | 75.6ns | 283ns | 0 | 0 | 0 | 2.16 KB |
Benchmarks.Trace.Asm.AppSecWafBenchmark - Faster 🎉 Same allocations ✔️
Faster 🎉 in #7102
Benchmark
base/diff
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack‑netcoreapp3.1
2.464
745,065.86
302,430.64
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark‑netcoreapp3.1
1.872
861,977.37
460,446.06
Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack‑netcoreapp3.1 | 2.464 | 745,065.86 | 302,430.64 | |
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark‑netcoreapp3.1 | 1.872 | 861,977.37 | 460,446.06 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | RunWafRealisticBenchmark |
net6.0 | 400μs | 91.4ns | 342ns | 0 | 0 | 0 | 4.55 KB |
master | RunWafRealisticBenchmark |
netcoreapp3.1 | 860μs | 2.71μs | 9.38μs | 0 | 0 | 0 | 4.48 KB |
master | RunWafRealisticBenchmark |
net472 | 431μs | 69ns | 267ns | 0 | 0 | 0 | 4.66 KB |
master | RunWafRealisticBenchmarkWithAttack |
net6.0 | 290μs | 50.7ns | 196ns | 0 | 0 | 0 | 2.24 KB |
master | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 696μs | 10.2μs | 102μs | 0 | 0 | 0 | 2.22 KB |
master | RunWafRealisticBenchmarkWithAttack |
net472 | 315μs | 33.9ns | 131ns | 0 | 0 | 0 | 2.29 KB |
#7102 | RunWafRealisticBenchmark |
net6.0 | 404μs | 122ns | 439ns | 0 | 0 | 0 | 4.56 KB |
#7102 | RunWafRealisticBenchmark |
netcoreapp3.1 | 461μs | 814ns | 3.15μs | 0 | 0 | 0 | 4.48 KB |
#7102 | RunWafRealisticBenchmark |
net472 | 432μs | 76.7ns | 297ns | 0 | 0 | 0 | 4.66 KB |
#7102 | RunWafRealisticBenchmarkWithAttack |
net6.0 | 286μs | 139ns | 520ns | 0 | 0 | 0 | 2.24 KB |
#7102 | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 302μs | 292ns | 1.13μs | 0 | 0 | 0 | 2.22 KB |
#7102 | RunWafRealisticBenchmarkWithAttack |
net472 | 312μs | 27.4ns | 106ns | 0 | 0 | 0 | 2.29 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 | 60.6μs | 54.1ns | 203ns | 0 | 0 | 0 | 14.52 KB |
master | SendRequest |
netcoreapp3.1 | 72.1μs | 97.6ns | 365ns | 0 | 0 | 0 | 17.42 KB |
master | SendRequest |
net472 | 0.00558ns | 0.00208ns | 0.00806ns | 0 | 0 | 0 | 0 b |
#7102 | SendRequest |
net6.0 | 60.9μs | 66.2ns | 248ns | 0 | 0 | 0 | 14.52 KB |
#7102 | SendRequest |
netcoreapp3.1 | 71.2μs | 78.3ns | 282ns | 0 | 0 | 0 | 17.42 KB |
#7102 | SendRequest |
net472 | 0.00504ns | 0.00216ns | 0.00836ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CharSliceBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7102
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0
2 B
5 B
3 B
150.00%
Fewer allocations 🎉 in #7102
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net472
73 B
0 b
-73 B
-100.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑netcoreapp3.1
1 B
0 b
-1 B
-100.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472
46 B
0 b
-46 B
-100.00%
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0 | 2 B | 5 B | 3 B | 150.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‑netcoreapp3.1 | 1 B | 0 b | -1 B | -100.00% |
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472 | 46 B | 0 b | -46 B | -100.00% |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | OriginalCharSlice |
net6.0 | 1.93ms | 823ns | 3.08μs | 0 | 0 | 0 | 640.01 KB |
master | OriginalCharSlice |
netcoreapp3.1 | 2.07ms | 5.73μs | 20.7μs | 0 | 0 | 0 | 640 KB |
master | OriginalCharSlice |
net472 | 2.69ms | 827ns | 3.2μs | 100 | 0 | 0 | 641.95 KB |
master | OptimizedCharSlice |
net6.0 | 1.53ms | 463ns | 1.79μs | 0 | 0 | 0 | 2 B |
master | OptimizedCharSlice |
netcoreapp3.1 | 1.66ms | 283ns | 1.09μs | 0 | 0 | 0 | 1 B |
master | OptimizedCharSlice |
net472 | 2.01ms | 456ns | 1.77μs | 0 | 0 | 0 | 73 B |
master | OptimizedCharSliceWithPool |
net6.0 | 799μs | 40ns | 155ns | 0 | 0 | 0 | 2 B |
master | OptimizedCharSliceWithPool |
netcoreapp3.1 | 818μs | 270ns | 1.05μs | 0 | 0 | 0 | 1 B |
master | OptimizedCharSliceWithPool |
net472 | 1.13ms | 34.5ns | 129ns | 0 | 0 | 0 | 46 B |
#7102 | OriginalCharSlice |
net6.0 | 1.91ms | 4.37μs | 16.4μs | 0 | 0 | 0 | 640.01 KB |
#7102 | OriginalCharSlice |
netcoreapp3.1 | 2.09ms | 4.07μs | 15.8μs | 0 | 0 | 0 | 640 KB |
#7102 | OriginalCharSlice |
net472 | 2.7ms | 450ns | 1.62μs | 100 | 0 | 0 | 641.95 KB |
#7102 | OptimizedCharSlice |
net6.0 | 1.45ms | 1.77μs | 6.87μs | 0 | 0 | 0 | 2 B |
#7102 | OptimizedCharSlice |
netcoreapp3.1 | 1.65ms | 181ns | 676ns | 0 | 0 | 0 | 1 B |
#7102 | OptimizedCharSlice |
net472 | 1.98ms | 345ns | 1.29μs | 0 | 0 | 0 | 0 b |
#7102 | OptimizedCharSliceWithPool |
net6.0 | 800μs | 32.1ns | 120ns | 0 | 0 | 0 | 5 B |
#7102 | OptimizedCharSliceWithPool |
netcoreapp3.1 | 871μs | 79.2ns | 307ns | 0 | 0 | 0 | 0 b |
#7102 | OptimizedCharSliceWithPool |
net472 | 1.14ms | 78.4ns | 304ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Slower ⚠️ Fewer allocations 🎉
Slower ⚠️ in #7102
Benchmark
diff/base
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472
1.169
823,558.40
962,895.98
Fewer allocations 🎉 in #7102
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472
56.16 KB
55.83 KB
-334 B
-0.59%
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0
42.64 KB
41.65 KB
-989 B
-2.32%
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 | 1.169 | 823,558.40 | 962,895.98 |
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 | 56.16 KB | 55.83 KB | -334 B | -0.59% |
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 | 42.64 KB | 41.65 KB | -989 B | -2.32% |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | WriteAndFlushEnrichedTraces |
net6.0 | 756μs | 4.2μs | 27.5μs | 0 | 0 | 0 | 42.64 KB |
master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 756μs | 4.11μs | 23.9μs | 0 | 0 | 0 | 41.81 KB |
master | WriteAndFlushEnrichedTraces |
net472 | 824μs | 2.43μs | 9.07μs | 7.81 | 0 | 0 | 56.16 KB |
#7102 | WriteAndFlushEnrichedTraces |
net6.0 | 699μs | 464ns | 1.73μs | 0 | 0 | 0 | 41.65 KB |
#7102 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 735μs | 4.61μs | 45.6μs | 0 | 0 | 0 | 41.92 KB |
#7102 | WriteAndFlushEnrichedTraces |
net472 | 968μs | 5.14μs | 26.7μs | 4.46 | 0 | 0 | 55.83 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.83μs | 3.61ns | 13ns | 0 | 0 | 0 | 1.02 KB |
master | ExecuteNonQuery |
netcoreapp3.1 | 2.54μs | 8.85ns | 33.1ns | 0 | 0 | 0 | 1.02 KB |
master | ExecuteNonQuery |
net472 | 2.7μs | 3.47ns | 13.4ns | 0.151 | 0.0137 | 0 | 987 B |
#7102 | ExecuteNonQuery |
net6.0 | 1.8μs | 5.14ns | 19.2ns | 0 | 0 | 0 | 1.02 KB |
#7102 | ExecuteNonQuery |
netcoreapp3.1 | 2.48μs | 10.7ns | 41.3ns | 0 | 0 | 0 | 1.02 KB |
#7102 | ExecuteNonQuery |
net472 | 2.68μs | 1.81ns | 7.01ns | 0.146 | 0.0133 | 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.71μs | 0.925ns | 3.58ns | 0 | 0 | 0 | 1.03 KB |
master | CallElasticsearch |
netcoreapp3.1 | 2.24μs | 4.96ns | 18.5ns | 0 | 0 | 0 | 1.03 KB |
master | CallElasticsearch |
net472 | 3.53μs | 1.23ns | 4.42ns | 0.159 | 0 | 0 | 1.04 KB |
master | CallElasticsearchAsync |
net6.0 | 1.89μs | 5.11ns | 19.8ns | 0 | 0 | 0 | 1.01 KB |
master | CallElasticsearchAsync |
netcoreapp3.1 | 2.39μs | 7.82ns | 30.3ns | 0 | 0 | 0 | 1.08 KB |
master | CallElasticsearchAsync |
net472 | 3.7μs | 1.36ns | 5.28ns | 0.166 | 0 | 0 | 1.1 KB |
#7102 | CallElasticsearch |
net6.0 | 1.71μs | 8.34ns | 36.4ns | 0 | 0 | 0 | 1.03 KB |
#7102 | CallElasticsearch |
netcoreapp3.1 | 2.16μs | 10.4ns | 41.6ns | 0 | 0 | 0 | 1.03 KB |
#7102 | CallElasticsearch |
net472 | 3.69μs | 2.18ns | 8.43ns | 0.165 | 0 | 0 | 1.04 KB |
#7102 | CallElasticsearchAsync |
net6.0 | 1.78μs | 7.18ns | 27.8ns | 0 | 0 | 0 | 1.01 KB |
#7102 | CallElasticsearchAsync |
netcoreapp3.1 | 2.36μs | 11.3ns | 43.6ns | 0 | 0 | 0 | 1.08 KB |
#7102 | CallElasticsearchAsync |
net472 | 3.83μs | 4.19ns | 16.2ns | 0.172 | 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.85μs | 8.59ns | 34.3ns | 0 | 0 | 0 | 952 B |
master | ExecuteAsync |
netcoreapp3.1 | 2.26μs | 5.18ns | 20ns | 0 | 0 | 0 | 952 B |
master | ExecuteAsync |
net472 | 2.55μs | 1.62ns | 6.28ns | 0.139 | 0 | 0 | 915 B |
#7102 | ExecuteAsync |
net6.0 | 1.83μs | 0.668ns | 2.5ns | 0 | 0 | 0 | 952 B |
#7102 | ExecuteAsync |
netcoreapp3.1 | 2.39μs | 5.31ns | 20.6ns | 0 | 0 | 0 | 952 B |
#7102 | ExecuteAsync |
net472 | 2.59μs | 3.12ns | 12.1ns | 0.141 | 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 | 6.81μs | 22.3ns | 86.2ns | 0 | 0 | 0 | 2.36 KB |
master | SendAsync |
netcoreapp3.1 | 8.63μs | 21.7ns | 84.1ns | 0 | 0 | 0 | 2.9 KB |
master | SendAsync |
net472 | 12.2μs | 4.33ns | 16.2ns | 0.485 | 0 | 0 | 3.18 KB |
#7102 | SendAsync |
net6.0 | 7μs | 5.68ns | 21.3ns | 0 | 0 | 0 | 2.36 KB |
#7102 | SendAsync |
netcoreapp3.1 | 8.59μs | 27.4ns | 106ns | 0 | 0 | 0 | 2.9 KB |
#7102 | SendAsync |
net472 | 12.3μs | 7.25ns | 28.1ns | 0.487 | 0 | 0 | 3.18 KB |
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7102
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1
42.86 KB
43.54 KB
680 B
1.59%
Fewer allocations 🎉 in #7102
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net472
287.59 KB
278.53 KB
-9.06 KB
-3.15%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0
272.93 KB
256.92 KB
-16.01 KB
-5.87%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1
275.23 KB
257.09 KB
-18.14 KB
-6.59%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472
73.73 KB
65.54 KB
-8.19 KB
-11.11%
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 | 42.86 KB | 43.54 KB | 680 B | 1.59% |
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net472 | 287.59 KB | 278.53 KB | -9.06 KB | -3.15% |
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 | 272.93 KB | 256.92 KB | -16.01 KB | -5.87% |
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 275.23 KB | 257.09 KB | -18.14 KB | -6.59% |
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 | 73.73 KB | 65.54 KB | -8.19 KB | -11.11% |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StringConcatBenchmark |
net6.0 | 49.4μs | 648ns | 6.48μs | 0 | 0 | 0 | 43.78 KB |
master | StringConcatBenchmark |
netcoreapp3.1 | 52.5μs | 631ns | 6.21μs | 0 | 0 | 0 | 42.86 KB |
master | StringConcatBenchmark |
net472 | 57.5μs | 269ns | 1.04μs | 0 | 0 | 0 | 73.73 KB |
master | StringConcatAspectBenchmark |
net6.0 | 466μs | 1.23μs | 4.26μs | 0 | 0 | 0 | 272.93 KB |
master | StringConcatAspectBenchmark |
netcoreapp3.1 | 458μs | 4.97μs | 48.7μs | 0 | 0 | 0 | 275.23 KB |
master | StringConcatAspectBenchmark |
net472 | 405μs | 2.03μs | 9.52μs | 0 | 0 | 0 | 287.59 KB |
#7102 | StringConcatBenchmark |
net6.0 | 44.8μs | 247ns | 1.62μs | 0 | 0 | 0 | 43.93 KB |
#7102 | StringConcatBenchmark |
netcoreapp3.1 | 49.1μs | 254ns | 1.17μs | 0 | 0 | 0 | 43.54 KB |
#7102 | StringConcatBenchmark |
net472 | 57.3μs | 294ns | 1.35μs | 0 | 0 | 0 | 65.54 KB |
#7102 | StringConcatAspectBenchmark |
net6.0 | 468μs | 2.15μs | 7.75μs | 0 | 0 | 0 | 256.92 KB |
#7102 | StringConcatAspectBenchmark |
netcoreapp3.1 | 486μs | 2.3μs | 9.19μs | 0 | 0 | 0 | 257.09 KB |
#7102 | StringConcatAspectBenchmark |
net472 | 399μs | 2.19μs | 16.1μs | 0 | 0 | 0 | 278.53 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.65μs | 11.1ns | 43.2ns | 0 | 0 | 0 | 1.7 KB |
master | EnrichedLog |
netcoreapp3.1 | 3.36μs | 13.2ns | 51.2ns | 0 | 0 | 0 | 1.7 KB |
master | EnrichedLog |
net472 | 4.05μs | 2.5ns | 9.67ns | 0.242 | 0 | 0 | 1.64 KB |
#7102 | EnrichedLog |
net6.0 | 2.58μs | 3.62ns | 14ns | 0 | 0 | 0 | 1.7 KB |
#7102 | EnrichedLog |
netcoreapp3.1 | 3.33μs | 15.7ns | 58.8ns | 0 | 0 | 0 | 1.7 KB |
#7102 | EnrichedLog |
net472 | 4.1μs | 3.74ns | 14.5ns | 0.244 | 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 | 122μs | 93.4ns | 324ns | 0 | 0 | 0 | 4.31 KB |
master | EnrichedLog |
netcoreapp3.1 | 127μs | 485ns | 1.75μs | 0 | 0 | 0 | 4.31 KB |
master | EnrichedLog |
net472 | 166μs | 246ns | 887ns | 0 | 0 | 0 | 4.51 KB |
#7102 | EnrichedLog |
net6.0 | 123μs | 42.3ns | 152ns | 0 | 0 | 0 | 4.31 KB |
#7102 | EnrichedLog |
netcoreapp3.1 | 127μs | 416ns | 1.5μs | 0 | 0 | 0 | 4.31 KB |
#7102 | EnrichedLog |
net472 | 166μs | 43ns | 166ns | 0 | 0 | 0 | 4.51 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 | 4.8μs | 23.2ns | 101ns | 0 | 0 | 0 | 2.26 KB |
master | EnrichedLog |
netcoreapp3.1 | 6.97μs | 19.5ns | 75.7ns | 0 | 0 | 0 | 2.26 KB |
master | EnrichedLog |
net472 | 7.37μs | 6.96ns | 26.9ns | 0.297 | 0 | 0 | 2.08 KB |
#7102 | EnrichedLog |
net6.0 | 5.08μs | 21.2ns | 82.2ns | 0 | 0 | 0 | 2.26 KB |
#7102 | EnrichedLog |
netcoreapp3.1 | 6.75μs | 17.9ns | 69.3ns | 0 | 0 | 0 | 2.26 KB |
#7102 | EnrichedLog |
net472 | 7.45μs | 6.64ns | 25.7ns | 0.296 | 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.99μs | 9.5ns | 39.2ns | 0 | 0 | 0 | 1.2 KB |
master | SendReceive |
netcoreapp3.1 | 2.61μs | 7.53ns | 29.2ns | 0 | 0 | 0 | 1.2 KB |
master | SendReceive |
net472 | 3.08μs | 3.39ns | 13.1ns | 0.185 | 0 | 0 | 1.2 KB |
#7102 | SendReceive |
net6.0 | 2.06μs | 10ns | 40.1ns | 0 | 0 | 0 | 1.2 KB |
#7102 | SendReceive |
netcoreapp3.1 | 2.62μs | 10.6ns | 41.2ns | 0 | 0 | 0 | 1.2 KB |
#7102 | SendReceive |
net472 | 3.1μs | 2.44ns | 9.11ns | 0.185 | 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.15μs | 12.5ns | 48.3ns | 0 | 0 | 0 | 1.58 KB |
master | EnrichedLog |
netcoreapp3.1 | 5.56μs | 11.7ns | 45.5ns | 0 | 0 | 0 | 1.63 KB |
master | EnrichedLog |
net472 | 6.61μs | 6.55ns | 25.4ns | 0.298 | 0 | 0 | 2.03 KB |
#7102 | EnrichedLog |
net6.0 | 4.13μs | 17.6ns | 66ns | 0 | 0 | 0 | 1.58 KB |
#7102 | EnrichedLog |
netcoreapp3.1 | 5.71μs | 16.4ns | 63.7ns | 0 | 0 | 0 | 1.63 KB |
#7102 | EnrichedLog |
net472 | 6.84μs | 10.6ns | 41.2ns | 0.31 | 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 | 750ns | 0.286ns | 1.07ns | 0 | 0 | 0 | 576 B |
master | StartFinishSpan |
netcoreapp3.1 | 931ns | 4.97ns | 25.8ns | 0 | 0 | 0 | 576 B |
master | StartFinishSpan |
net472 | 892ns | 0.058ns | 0.201ns | 0.0894 | 0 | 0 | 578 B |
master | StartFinishScope |
net6.0 | 881ns | 3.97ns | 15.4ns | 0 | 0 | 0 | 696 B |
master | StartFinishScope |
netcoreapp3.1 | 1.12μs | 5.88ns | 31.1ns | 0 | 0 | 0 | 696 B |
master | StartFinishScope |
net472 | 1.09μs | 0.409ns | 1.58ns | 0.104 | 0 | 0 | 658 B |
#7102 | StartFinishSpan |
net6.0 | 750ns | 3.62ns | 14.5ns | 0 | 0 | 0 | 576 B |
#7102 | StartFinishSpan |
netcoreapp3.1 | 931ns | 5.02ns | 26.1ns | 0 | 0 | 0 | 576 B |
#7102 | StartFinishSpan |
net472 | 900ns | 0.726ns | 2.81ns | 0.0899 | 0 | 0 | 578 B |
#7102 | StartFinishScope |
net6.0 | 876ns | 4.7ns | 24.4ns | 0 | 0 | 0 | 696 B |
#7102 | StartFinishScope |
netcoreapp3.1 | 1.1μs | 5.34ns | 22.7ns | 0 | 0 | 0 | 696 B |
#7102 | StartFinishScope |
net472 | 1.07μs | 0.102ns | 0.395ns | 0.102 | 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.06μs | 3.42ns | 12.8ns | 0 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
netcoreapp3.1 | 1.38μs | 0.817ns | 3.16ns | 0 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
net472 | 1.39μs | 0.617ns | 2.31ns | 0.0979 | 0 | 0 | 658 B |
#7102 | RunOnMethodBegin |
net6.0 | 1.04μs | 5.68ns | 31.1ns | 0 | 0 | 0 | 696 B |
#7102 | RunOnMethodBegin |
netcoreapp3.1 | 1.37μs | 7.08ns | 28.3ns | 0 | 0 | 0 | 696 B |
#7102 | RunOnMethodBegin |
net472 | 1.41μs | 0.484ns | 1.81ns | 0.0992 | 0 | 0 | 658 B |
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:
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.8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (72ms) : 71, 73
. : milestone, 72,
master - mean (72ms) : 70, 73
. : milestone, 72,
section Baseline
This PR (7102) - mean (70ms) : 61, 79
. : milestone, 70,
master - mean (68ms) : 66, 70
. : milestone, 68,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (1,049ms) : 995, 1103
. : milestone, 1049,
master - mean (1,052ms) : 985, 1118
. : milestone, 1052,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (106ms) : 105, 107
. : milestone, 106,
master - mean (106ms) : 105, 108
. : milestone, 106,
section Baseline
This PR (7102) - mean (105ms) : 103, 108
. : milestone, 105,
master - mean (106ms) : 103, 108
. : milestone, 106,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (744ms) : 717, 770
. : milestone, 744,
master - mean (744ms) : 720, 768
. : milestone, 744,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (101ms) : 99, 102
. : milestone, 101,
master - mean (100ms) : 99, 102
. : milestone, 100,
section Baseline
This PR (7102) - mean (99ms) : 97, 102
. : milestone, 99,
master - mean (100ms) : 97, 102
. : milestone, 100,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (776ms) : 734, 817
. : milestone, 776,
master - mean (779ms) : 740, 818
. : milestone, 779,
gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (93ms) : 92, 94
. : milestone, 93,
master - mean (92ms) : 92, 93
. : milestone, 92,
section Baseline
This PR (7102) - mean (92ms) : 90, 94
. : milestone, 92,
master - mean (92ms) : 89, 95
. : milestone, 92,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (657ms) : 644, 671
. : milestone, 657,
master - mean (663ms) : 648, 678
. : milestone, 663,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (200ms) : 196, 205
. : milestone, 200,
master - mean (198ms) : 195, 201
. : milestone, 198,
section Baseline
This PR (7102) - mean (197ms) : 191, 204
. : milestone, 197,
master - mean (196ms) : 191, 200
. : milestone, 196,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (1,195ms) : 1109, 1281
. : milestone, 1195,
master - mean (1,174ms) : 1104, 1244
. : milestone, 1174,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (285ms) : 278, 292
. : milestone, 285,
master - mean (282ms) : 277, 287
. : milestone, 282,
section Baseline
This PR (7102) - mean (284ms) : 277, 292
. : milestone, 284,
master - mean (281ms) : 276, 285
. : milestone, 281,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (949ms) : 915, 984
. : milestone, 949,
master - mean (947ms) : 907, 988
. : milestone, 947,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (290ms) : 280, 299
. : milestone, 290,
master - mean (284ms) : 279, 288
. : milestone, 284,
section Baseline
This PR (7102) - mean (288ms) : 279, 297
. : milestone, 288,
master - mean (285ms) : 277, 292
. : milestone, 285,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (1,008ms) : 963, 1053
. : milestone, 1008,
master - mean (1,005ms) : 963, 1048
. : milestone, 1005,
gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7102) - mean (276ms) : 268, 284
. : milestone, 276,
master - mean (273ms) : 269, 278
. : milestone, 273,
section Baseline
This PR (7102) - mean (276ms) : 265, 286
. : milestone, 276,
master - mean (271ms) : 267, 276
. : milestone, 271,
section CallTarget+Inlining+NGEN
This PR (7102) - mean (873ms) : 845, 902
. : milestone, 873,
master - mean (858ms) : 828, 889
. : milestone, 858,
|
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.
Codex Review: Here are some suggestions.
Reply with @codex fix comments
to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
_doneTask = Task.FromResult<ExceptionIdentifier?>(null); | ||
if (settings.DynamicInstrumentationEnabled == null && clientSettingsResponse.DynamicInstrumentationEnabled.HasValue) | ||
{ | ||
Log.Information("TestOptimizationDynamicInstrumentationFeature: Dynamic instrumentation has been changed to {Value} by the settings api.", clientSettingsResponse.DynamicInstrumentationEnabled.Value); | ||
settings.SetFlakyRetryEnabled(clientSettingsResponse.DynamicInstrumentationEnabled.Value); |
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.
[P1] Remote DI setting updates wrong configuration field
When the settings API returns a value for dynamic instrumentation, the constructor logs the change but then calls settings.SetFlakyRetryEnabled(...)
instead of the dedicated dynamic instrumentation setter. As a result the DynamicInstrumentationEnabled
property remains null/false
, so the feature never activates when it is enabled remotely and the environment variables configured in CiUtils
are never applied. This makes failed test replay dependent on local env vars and ignores the remote switch.
Useful? React with 👍 / 👎.
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.
Not true, on line [31, 40] we check again the settings property to set the DynamicInstrumentationEnabled property.
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.
LGTM overall, just some minor comments really
/// <summary> | ||
/// Configuration key for a kill-switch that allows to explicitly disable dynamic instrumentation even if the remote setting is enabled. | ||
/// </summary> | ||
public const string DynamicInstrumentationEnabled = "DD_CIVISIBILITY_DI_ENABLED"; |
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.
Do we need a new key for this? Shouldn't we use the existing DI enabled keys? 🤔
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 dynamic instrumentation they have 4 or 5 environment variables with defaults that we might want to change on test optimization mode. Similar to the logs implementation, I want to control everything with a single environment variable to set the new defaults to those 4 or 5 environment variables.
{ | ||
if (_dynamicInstrumentationFeature is null) | ||
{ | ||
_additionalFeaturesTask?.SafeWait(); |
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.
Anything called Safe
makes me nervous 😛
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.
hahaha, yeah I know, this is just a .GetAwaiter().GetResult()
inside a block to remove the synchronization context. Maybe not the best name tbh.
public static void SafeWait(this Task task)
{
if (task.IsCompleted)
{
return;
}
var originalContext = SynchronizationContext.Current;
try
{
// Set the synchronization context to null to avoid deadlocks.
SynchronizationContext.SetSynchronizationContext(null);
// Wait synchronously for the task to complete.
task.GetAwaiter().GetResult();
}
finally
{
// Restore the original synchronization context.
SynchronizationContext.SetSynchronizationContext(originalContext);
}
}
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.
WaitWithoutContext or WaitIgnoreContext 🤔
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.
oh I like those names, I'll handle the renaming in a different PR though.
s.TracerSettings.Exporter, | ||
tcpTimeout: TimeSpan.FromSeconds(5), | ||
initialRetryDelayMs: 10, | ||
initialRetryDelayMs: 100, |
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.
Is this an intentional fix? I assume so, 10 ms is very low 😄
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.
yeah exactly that, I saw 10ms and I thought this must be an error, 10ms is just too low.
} | ||
else | ||
{ | ||
settings.SetDynamicInstrumentationEnabled(false); |
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 this can change, do we need to potentially remove the event handler from ExceptionTrackManager.ExceptionCaseInstrumented
?
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.
Yes, but the handler is only set when we set the settings to true
we do settings.SetDynamicInstrumentationEnabled(false)
just to make sure that the settings has the right value, because is a nullable. I hate it and all the settings needs a refactor from my side.
tracer/src/Datadog.Trace/Ci/TestOptimizationTracerManagement.cs
Outdated
Show resolved
Hide resolved
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/TestingFrameworkRetriesTests.cs
Outdated
Show resolved
Hide resolved
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/TestingFrameworkRetriesTests.cs
Show resolved
Hide resolved
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/TestingFrameworkRetriesTests.cs
Outdated
Show resolved
Hide resolved
SetEnvironmentVariable(ConfigurationKeys.Debugger.UploadFlushInterval, "1000"); | ||
SetEnvironmentVariable("DD_CLR_ENABLE_INLINING", "0"); | ||
|
||
var tests = await FlakyRetries(packageVersion); |
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 know you love your explicit checks, but should we really just have snapshots here? It's hard to spot things that should or shouldn't be there 🤷♂️
"DD_CIVISIBILITY_FLAKY_RETRY_COUNT": "ci_visibility_flaky_retry_count", | ||
"DD_CIVISIBILITY_TOTAL_FLAKY_RETRY_COUNT": "ci_visibility_total_flaky_retry_count", | ||
"DD_CIVISIBILITY_IMPACTED_TESTS_DETECTION_ENABLED": "dd_civisibility_impacted_tests_detection_enabled", | ||
"DD_CIVISIBILITY_DI_ENABLED": "ci_visibility_dynamic_instrumentation_enabled", |
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.
Don't forget to add this into dd-go
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.
LGTM. I left minor comments \ questions.
public readonly string? DefaultBranch; | ||
|
||
[JsonProperty("di_enabled")] | ||
public readonly bool? DynamicInstrumentationEnabled; |
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.
Is it intentionally nullable instead of giving it a default value (false)?
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.
Yes currently the TestOptimization product uses nullable everywhere to represent that it needs to gather information from the backend to make the final decision (true or false). Values can be rewritten using the environment variables (no nullable) and that prevents the backend request.
I actually don't like this and I'll do a refactor of all this mechanism in the future
{ | ||
if (_dynamicInstrumentationFeature is null) | ||
{ | ||
_additionalFeaturesTask?.SafeWait(); |
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.
WaitWithoutContext or WaitIgnoreContext 🤔
|
||
var dts = _doneTaskSource; | ||
dts = Interlocked.CompareExchange(ref _doneTaskSource, dts, dts); | ||
return Task.WhenAny(dts.Task, Task.Delay(timeout)); |
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.
@GreenMatan timeout (2000 ms) is enough?
/// <summary> | ||
/// Configuration key for a kill-switch that allows to explicitly disable dynamic instrumentation even if the remote setting is enabled. | ||
/// </summary> | ||
public const string DynamicInstrumentationEnabled = "DD_CIVISIBILITY_DI_ENABLED"; |
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.
Maybe I'm wrong but it seems that what you want here is ExceptionReplayEnabled
and not DI.
|
||
var request = _apiRequestFactory.Create(new Uri(uri)); | ||
|
||
Log.Debug("SnapshotUploadApi: Sending snapshots to {Uri}", uri); |
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.
Are you using the same endpoint as the debugger? It may not matter, but from the next version of agent\tracer we have a new endpoint for snapshots. #7388
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.
Currently I'm just using what exception replay is using, I'm just adding some logs in debug mode. I need to talk with you about the new endpoint though, let's talk offline.
Summary of changes
This PR enables Exception Replay on the Test Optimization product.
Reason for change
This is a Test Optimization new feature called Failed Test Replay. When enabled all automatic test retries for a tests are executed with the Exception replay feature turned on, meaning that if the same exception happens again we will retrieve debug information of the exception stackframes.
Implementation details
DynamicInstrumentationEnabled
into test optimization settings and added logic to enable related environment variables when the feature is active. (tracer/src/Datadog.Trace.Tools.Runner/CiUtils.cs)DynamicInstrumentationEnabled
and its corresponding setter method to manage the feature's configuration. (tracer/src/Datadog.Trace/Ci/Configuration/TestOptimizationSettings.cs)Test coverage
A new serie of test suite were added by every case to assert if exception replay add the tags to the span.
Other details
DI team (@GreenMatan) it's investigating why the feature is not working on xUnit v3. That's why the test is currently disabled on this framework version.