-
Notifications
You must be signed in to change notification settings - Fork 151
[AWS Lambda] Add request-id as header to Lambda start/end invocation #7835
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
[AWS Lambda] Add request-id as header to Lambda start/end invocation #7835
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (7835) 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 (7835) - mean (77ms) : 70, 84
master - mean (76ms) : 70, 81
section Bailout
This PR (7835) - mean (81ms) : 74, 89
master - mean (80ms) : 74, 86
section CallTarget+Inlining+NGEN
This PR (7835) - mean (1,076ms) : 992, 1161
master - mean (1,063ms) : 977, 1149
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 (7835) - mean (118ms) : 108, 129
master - mean (119ms) : 108, 129
section Bailout
This PR (7835) - mean (119ms) : 110, 128
master - mean (119ms) : 110, 128
section CallTarget+Inlining+NGEN
This PR (7835) - mean (761ms) : 703, 820
master - mean (767ms) : 705, 828
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7835) - mean (105ms) : 96, 114
master - mean (106ms) : 98, 115
section Bailout
This PR (7835) - mean (106ms) : 96, 115
master - mean (105ms) : 98, 112
section CallTarget+Inlining+NGEN
This PR (7835) - mean (716ms) : 659, 773
master - mean (718ms) : 676, 760
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7835) - mean (104ms) : 95, 114
master - mean (104ms) : 96, 113
section Bailout
This PR (7835) - mean (105ms) : 98, 113
master - mean (105ms) : 96, 113
section CallTarget+Inlining+NGEN
This PR (7835) - mean (688ms) : 659, 717
master - mean (687ms) : 655, 718
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 (7835) - mean (198ms) : 190, 205
master - mean (193ms) : 189, 196
section Bailout
This PR (7835) - mean (200ms) : 194, 206
master - mean (197ms) : 194, 199
section CallTarget+Inlining+NGEN
This PR (7835) - mean (1,130ms) : 1054, 1206
master - mean (1,115ms) : 1045, 1185
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 (7835) - mean (278ms) : 272, 283
master - mean (277ms) : 271, 283
section Bailout
This PR (7835) - mean (278ms) : 274, 283
master - mean (278ms) : 273, 283
section CallTarget+Inlining+NGEN
This PR (7835) - mean (902ms) : 854, 949
master - mean (905ms) : 854, 956
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7835) - mean (271ms) : 266, 275
master - mean (270ms) : 266, 274
section Bailout
This PR (7835) - mean (271ms) : 266, 276
master - mean (272ms) : 265, 279
section CallTarget+Inlining+NGEN
This PR (7835) - mean (888ms) : 836, 941
master - mean (884ms) : 844, 924
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7835) - mean (269ms) : 264, 274
master - mean (269ms) : 265, 273
section Bailout
This PR (7835) - mean (270ms) : 267, 274
master - mean (270ms) : 265, 275
section CallTarget+Inlining+NGEN
This PR (7835) - mean (824ms) : 799, 849
master - mean (829ms) : 812, 847
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmarks Report for benchmark platform 🐌Benchmarks for #7835 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‑net472 | 6.01 KB | 6.06 KB | 54 B | 0.90% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StartStopWithChild |
net6.0 | 11.2μs | 50.8ns | 203ns | 0 | 0 | 0 | 5.5 KB |
| master | StartStopWithChild |
netcoreapp3.1 | 13.6μs | 69.1ns | 317ns | 0 | 0 | 0 | 5.68 KB |
| master | StartStopWithChild |
net472 | 22.3μs | 123ns | 776ns | 0.97 | 0.431 | 0.108 | 6.01 KB |
| #7835 | StartStopWithChild |
net6.0 | 11μs | 55.3ns | 259ns | 0 | 0 | 0 | 5.52 KB |
| #7835 | StartStopWithChild |
netcoreapp3.1 | 15.4μs | 33.3ns | 129ns | 0 | 0 | 0 | 5.68 KB |
| #7835 | StartStopWithChild |
net472 | 21.5μs | 118ns | 677ns | 0.985 | 0.328 | 0.109 | 6.06 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 | 926μs | 53.1ns | 199ns | 0 | 0 | 0 | 2.7 KB |
| master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 1.05ms | 2.65μs | 9.9μs | 0 | 0 | 0 | 2.7 KB |
| master | WriteAndFlushEnrichedTraces |
net472 | 1.2ms | 61.9ns | 240ns | 0 | 0 | 0 | 3.31 KB |
| #7835 | WriteAndFlushEnrichedTraces |
net6.0 | 940μs | 93.2ns | 349ns | 0 | 0 | 0 | 2.7 KB |
| #7835 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 1.05ms | 113ns | 422ns | 0 | 0 | 0 | 2.7 KB |
| #7835 | WriteAndFlushEnrichedTraces |
net472 | 1.22ms | 504ns | 1.95μs | 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.08μs | 5.6ns | 26.8ns | 0 | 0 | 0 | 1.22 KB |
| master | AllCycleSimpleBody |
netcoreapp3.1 | 1.37μs | 6.78ns | 28.8ns | 0 | 0 | 0 | 1.2 KB |
| master | AllCycleSimpleBody |
net472 | 1.01μs | 0.574ns | 2.22ns | 0.192 | 0 | 0 | 1.23 KB |
| master | AllCycleMoreComplexBody |
net6.0 | 7.2μs | 7.78ns | 30.2ns | 0 | 0 | 0 | 4.72 KB |
| master | AllCycleMoreComplexBody |
netcoreapp3.1 | 9.01μs | 33.1ns | 128ns | 0 | 0 | 0 | 4.62 KB |
| master | AllCycleMoreComplexBody |
net472 | 7.58μs | 5.15ns | 19.9ns | 0.721 | 0 | 0 | 4.74 KB |
| master | ObjectExtractorSimpleBody |
net6.0 | 321ns | 1.8ns | 11ns | 0 | 0 | 0 | 280 B |
| master | ObjectExtractorSimpleBody |
netcoreapp3.1 | 402ns | 2.07ns | 10.2ns | 0 | 0 | 0 | 272 B |
| master | ObjectExtractorSimpleBody |
net472 | 297ns | 0.084ns | 0.303ns | 0.0433 | 0 | 0 | 281 B |
| master | ObjectExtractorMoreComplexBody |
net6.0 | 6.34μs | 33.3ns | 163ns | 0 | 0 | 0 | 3.78 KB |
| master | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 7.94μs | 28.9ns | 112ns | 0 | 0 | 0 | 3.69 KB |
| master | ObjectExtractorMoreComplexBody |
net472 | 6.75μs | 6.17ns | 23.9ns | 0.572 | 0 | 0 | 3.8 KB |
| #7835 | AllCycleSimpleBody |
net6.0 | 1.09μs | 5.96ns | 37.7ns | 0 | 0 | 0 | 1.22 KB |
| #7835 | AllCycleSimpleBody |
netcoreapp3.1 | 1.39μs | 7.19ns | 28.8ns | 0 | 0 | 0 | 1.2 KB |
| #7835 | AllCycleSimpleBody |
net472 | 1.03μs | 0.233ns | 0.87ns | 0.191 | 0 | 0 | 1.23 KB |
| #7835 | AllCycleMoreComplexBody |
net6.0 | 7.15μs | 3.78ns | 14.7ns | 0 | 0 | 0 | 4.72 KB |
| #7835 | AllCycleMoreComplexBody |
netcoreapp3.1 | 9.08μs | 3.15ns | 12.2ns | 0 | 0 | 0 | 4.62 KB |
| #7835 | AllCycleMoreComplexBody |
net472 | 7.76μs | 7.91ns | 30.6ns | 0.734 | 0 | 0 | 4.74 KB |
| #7835 | ObjectExtractorSimpleBody |
net6.0 | 322ns | 0.0731ns | 0.283ns | 0 | 0 | 0 | 280 B |
| #7835 | ObjectExtractorSimpleBody |
netcoreapp3.1 | 398ns | 2.13ns | 10.9ns | 0 | 0 | 0 | 272 B |
| #7835 | ObjectExtractorSimpleBody |
net472 | 300ns | 0.0136ns | 0.0511ns | 0.0446 | 0 | 0 | 281 B |
| #7835 | ObjectExtractorMoreComplexBody |
net6.0 | 6.29μs | 32.8ns | 161ns | 0 | 0 | 0 | 3.78 KB |
| #7835 | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 7.77μs | 38.1ns | 157ns | 0 | 0 | 0 | 3.69 KB |
| #7835 | ObjectExtractorMoreComplexBody |
net472 | 6.73μs | 5.91ns | 22.9ns | 0.574 | 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 | 75.4μs | 343ns | 1.33μs | 0 | 0 | 0 | 32.4 KB |
| master | EncodeArgs |
netcoreapp3.1 | 96.9μs | 220ns | 823ns | 0 | 0 | 0 | 32.4 KB |
| master | EncodeArgs |
net472 | 109μs | 11.6ns | 43.5ns | 4.88 | 0 | 0 | 32.51 KB |
| master | EncodeLegacyArgs |
net6.0 | 146μs | 218ns | 844ns | 0 | 0 | 0 | 2.15 KB |
| master | EncodeLegacyArgs |
netcoreapp3.1 | 199μs | 170ns | 659ns | 0 | 0 | 0 | 2.15 KB |
| master | EncodeLegacyArgs |
net472 | 263μs | 23.8ns | 92.2ns | 0 | 0 | 0 | 2.17 KB |
| #7835 | EncodeArgs |
net6.0 | 76.1μs | 342ns | 1.32μs | 0 | 0 | 0 | 32.4 KB |
| #7835 | EncodeArgs |
netcoreapp3.1 | 97.5μs | 236ns | 913ns | 0 | 0 | 0 | 32.4 KB |
| #7835 | EncodeArgs |
net472 | 109μs | 33.9ns | 131ns | 4.89 | 0 | 0 | 32.51 KB |
| #7835 | EncodeLegacyArgs |
net6.0 | 145μs | 5.38ns | 18.7ns | 0 | 0 | 0 | 2.15 KB |
| #7835 | EncodeLegacyArgs |
netcoreapp3.1 | 195μs | 60.7ns | 227ns | 0 | 0 | 0 | 2.14 KB |
| #7835 | EncodeLegacyArgs |
net472 | 263μs | 37.2ns | 139ns | 0 | 0 | 0 | 2.17 KB |
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunWafRealisticBenchmark |
net6.0 | 393μs | 118ns | 427ns | 0 | 0 | 0 | 4.56 KB |
| master | RunWafRealisticBenchmark |
netcoreapp3.1 | 409μs | 112ns | 404ns | 0 | 0 | 0 | 4.48 KB |
| master | RunWafRealisticBenchmark |
net472 | 428μs | 56.9ns | 220ns | 0 | 0 | 0 | 4.68 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net6.0 | 285μs | 48.4ns | 187ns | 0 | 0 | 0 | 2.24 KB |
| master | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 678μs | 11.1μs | 111μs | 0 | 0 | 0 | 2.22 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net472 | 310μs | 35.7ns | 138ns | 0 | 0 | 0 | 2.29 KB |
| #7835 | RunWafRealisticBenchmark |
net6.0 | 414μs | 77.9ns | 281ns | 0 | 0 | 0 | 4.55 KB |
| #7835 | RunWafRealisticBenchmark |
netcoreapp3.1 | 411μs | 375ns | 1.45μs | 0 | 0 | 0 | 4.48 KB |
| #7835 | RunWafRealisticBenchmark |
net472 | 428μs | 44.3ns | 172ns | 0 | 0 | 0 | 4.66 KB |
| #7835 | RunWafRealisticBenchmarkWithAttack |
net6.0 | 285μs | 50.4ns | 189ns | 0 | 0 | 0 | 2.24 KB |
| #7835 | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 678μs | 11.2μs | 112μs | 0 | 0 | 0 | 2.22 KB |
| #7835 | RunWafRealisticBenchmarkWithAttack |
net472 | 311μs | 37.3ns | 145ns | 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 | 61.3μs | 127ns | 491ns | 0 | 0 | 0 | 14.52 KB |
| master | SendRequest |
netcoreapp3.1 | 71.5μs | 317ns | 1.55μs | 0 | 0 | 0 | 17.42 KB |
| master | SendRequest |
net472 | 0.00768ns | 0.00289ns | 0.0112ns | 0 | 0 | 0 | 0 b |
| #7835 | SendRequest |
net6.0 | 60.2μs | 207ns | 878ns | 0 | 0 | 0 | 14.52 KB |
| #7835 | SendRequest |
netcoreapp3.1 | 70.9μs | 238ns | 1.04μs | 0 | 0 | 0 | 17.42 KB |
| #7835 | SendRequest |
net472 | 0.00552ns | 0.00167ns | 0.00646ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CharSliceBenchmark - Same speed ✔️ Fewer allocations 🎉
Fewer allocations 🎉 in #7835
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472
48 B
47 B
-1 B
-2.08%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0
7 B
6 B
-1 B
-14.29%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net472 | 48 B | 47 B | -1 B | -2.08% |
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0 | 7 B | 6 B | -1 B | -14.29% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | OriginalCharSlice |
net6.0 | 1.91ms | 5.91μs | 22.1μs | 0 | 0 | 0 | 640.01 KB |
| master | OriginalCharSlice |
netcoreapp3.1 | 2.05ms | 8.75μs | 32.7μs | 0 | 0 | 0 | 640 KB |
| master | OriginalCharSlice |
net472 | 2.66ms | 629ns | 2.27μs | 100 | 0 | 0 | 641.95 KB |
| master | OptimizedCharSlice |
net6.0 | 1.41ms | 221ns | 854ns | 0 | 0 | 0 | 7 B |
| master | OptimizedCharSlice |
netcoreapp3.1 | 1.73ms | 245ns | 850ns | 0 | 0 | 0 | 1 B |
| master | OptimizedCharSlice |
net472 | 1.98ms | 422ns | 1.58μs | 0 | 0 | 0 | 73 B |
| master | OptimizedCharSliceWithPool |
net6.0 | 822μs | 67.2ns | 260ns | 0 | 0 | 0 | 3 B |
| master | OptimizedCharSliceWithPool |
netcoreapp3.1 | 827μs | 63.5ns | 229ns | 0 | 0 | 0 | 0 b |
| master | OptimizedCharSliceWithPool |
net472 | 1.14ms | 130ns | 488ns | 0 | 0 | 0 | 48 B |
| #7835 | OriginalCharSlice |
net6.0 | 1.99ms | 4.4μs | 17.1μs | 0 | 0 | 0 | 640.01 KB |
| #7835 | OriginalCharSlice |
netcoreapp3.1 | 2.07ms | 6.27μs | 24.3μs | 0 | 0 | 0 | 640 KB |
| #7835 | OriginalCharSlice |
net472 | 2.64ms | 170ns | 615ns | 100 | 0 | 0 | 641.95 KB |
| #7835 | OptimizedCharSlice |
net6.0 | 1.35ms | 408ns | 1.58μs | 0 | 0 | 0 | 6 B |
| #7835 | OptimizedCharSlice |
netcoreapp3.1 | 1.66ms | 819ns | 3.17μs | 0 | 0 | 0 | 1 B |
| #7835 | OptimizedCharSlice |
net472 | 1.98ms | 500ns | 1.94μs | 0 | 0 | 0 | 73 B |
| #7835 | OptimizedCharSliceWithPool |
net6.0 | 853μs | 46.5ns | 180ns | 0 | 0 | 0 | 3 B |
| #7835 | OptimizedCharSliceWithPool |
netcoreapp3.1 | 837μs | 187ns | 726ns | 0 | 0 | 0 | 0 b |
| #7835 | OptimizedCharSliceWithPool |
net472 | 1.13ms | 89.1ns | 345ns | 0 | 0 | 0 | 47 B |
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Slower ⚠️ More allocations ⚠️
Slower ⚠️ in #7835
Benchmark
diff/base
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1
1.140
697,481.67
795,079.46
bimodal
More allocations ⚠️ in #7835
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1
41.89 KB
42.77 KB
884 B
2.11%
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0
41.77 KB
42.52 KB
752 B
1.80%
Fewer allocations 🎉 in #7835
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472
56.32 KB
55.95 KB
-376 B
-0.67%
| Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 | 1.140 | 697,481.67 | 795,079.46 | bimodal |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 | 41.89 KB | 42.77 KB | 884 B | 2.11% |
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 | 41.77 KB | 42.52 KB | 752 B | 1.80% |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 | 56.32 KB | 55.95 KB | -376 B | -0.67% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | WriteAndFlushEnrichedTraces |
net6.0 | 719μs | 4.05μs | 28.9μs | 0 | 0 | 0 | 41.77 KB |
| master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 692μs | 3.85μs | 24.4μs | 0 | 0 | 0 | 41.89 KB |
| master | WriteAndFlushEnrichedTraces |
net472 | 895μs | 2.42μs | 9.05μs | 8.33 | 0 | 0 | 56.32 KB |
| #7835 | WriteAndFlushEnrichedTraces |
net6.0 | 728μs | 3.47μs | 14.7μs | 0 | 0 | 0 | 42.52 KB |
| #7835 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 796μs | 3.53μs | 15.8μs | 0 | 0 | 0 | 42.77 KB |
| #7835 | WriteAndFlushEnrichedTraces |
net472 | 880μs | 2.36μs | 8.83μs | 8.33 | 0 | 0 | 55.95 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.87μs | 9.43ns | 43.2ns | 0 | 0 | 0 | 1.02 KB |
| master | ExecuteNonQuery |
netcoreapp3.1 | 2.62μs | 8.37ns | 32.4ns | 0 | 0 | 0 | 1.02 KB |
| master | ExecuteNonQuery |
net472 | 2.74μs | 3.82ns | 14.8ns | 0.15 | 0.0137 | 0 | 987 B |
| #7835 | ExecuteNonQuery |
net6.0 | 1.91μs | 1.1ns | 4.11ns | 0 | 0 | 0 | 1.02 KB |
| #7835 | ExecuteNonQuery |
netcoreapp3.1 | 2.68μs | 1.43ns | 5.56ns | 0 | 0 | 0 | 1.02 KB |
| #7835 | ExecuteNonQuery |
net472 | 2.78μs | 3.15ns | 12.2ns | 0.154 | 0.014 | 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.74μs | 7.62ns | 29.5ns | 0 | 0 | 0 | 1.03 KB |
| master | CallElasticsearch |
netcoreapp3.1 | 2.23μs | 10.9ns | 47.5ns | 0 | 0 | 0 | 1.03 KB |
| master | CallElasticsearch |
net472 | 3.68μs | 4.13ns | 16ns | 0.165 | 0 | 0 | 1.04 KB |
| master | CallElasticsearchAsync |
net6.0 | 1.75μs | 8.79ns | 39.3ns | 0 | 0 | 0 | 1.01 KB |
| master | CallElasticsearchAsync |
netcoreapp3.1 | 2.47μs | 8.41ns | 32.6ns | 0 | 0 | 0 | 1.08 KB |
| master | CallElasticsearchAsync |
net472 | 3.85μs | 3.8ns | 14.7ns | 0.172 | 0 | 0 | 1.1 KB |
| #7835 | CallElasticsearch |
net6.0 | 1.68μs | 7.38ns | 28.6ns | 0 | 0 | 0 | 1.03 KB |
| #7835 | CallElasticsearch |
netcoreapp3.1 | 2.16μs | 10.5ns | 44.7ns | 0 | 0 | 0 | 1.03 KB |
| #7835 | CallElasticsearch |
net472 | 3.42μs | 1.66ns | 6.22ns | 0.155 | 0 | 0 | 1.04 KB |
| #7835 | CallElasticsearchAsync |
net6.0 | 1.84μs | 8.92ns | 36.8ns | 0 | 0 | 0 | 1.01 KB |
| #7835 | CallElasticsearchAsync |
netcoreapp3.1 | 2.43μs | 1.2ns | 4.5ns | 0 | 0 | 0 | 1.08 KB |
| #7835 | CallElasticsearchAsync |
net472 | 3.7μs | 3.34ns | 12.9ns | 0.166 | 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.93μs | 6.13ns | 23.7ns | 0 | 0 | 0 | 952 B |
| master | ExecuteAsync |
netcoreapp3.1 | 2.49μs | 9ns | 34.9ns | 0 | 0 | 0 | 952 B |
| master | ExecuteAsync |
net472 | 2.65μs | 1.75ns | 6.78ns | 0.133 | 0 | 0 | 915 B |
| #7835 | ExecuteAsync |
net6.0 | 1.85μs | 7.08ns | 24.5ns | 0 | 0 | 0 | 952 B |
| #7835 | ExecuteAsync |
netcoreapp3.1 | 2.42μs | 9.63ns | 37.3ns | 0 | 0 | 0 | 952 B |
| #7835 | ExecuteAsync |
net472 | 2.54μs | 4.12ns | 16ns | 0.139 | 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.91μs | 20.1ns | 75.3ns | 0 | 0 | 0 | 2.36 KB |
| master | SendAsync |
netcoreapp3.1 | 8.64μs | 26ns | 101ns | 0 | 0 | 0 | 2.9 KB |
| master | SendAsync |
net472 | 12.1μs | 8.02ns | 31.1ns | 0.483 | 0 | 0 | 3.18 KB |
| #7835 | SendAsync |
net6.0 | 6.87μs | 6.23ns | 24.1ns | 0 | 0 | 0 | 2.36 KB |
| #7835 | SendAsync |
netcoreapp3.1 | 8.67μs | 23.8ns | 92.1ns | 0 | 0 | 0 | 2.9 KB |
| #7835 | SendAsync |
net472 | 12.5μs | 7.26ns | 27.2ns | 0.498 | 0 | 0 | 3.18 KB |
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7835
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1
255.46 KB
275.94 KB
20.48 KB
8.02%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1
42.7 KB
44.2 KB
1.5 KB
3.52%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 255.46 KB | 275.94 KB | 20.48 KB | 8.02% |
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 | 42.7 KB | 44.2 KB | 1.5 KB | 3.52% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StringConcatBenchmark |
net6.0 | 46.1μs | 270ns | 2.22μs | 0 | 0 | 0 | 43.64 KB |
| master | StringConcatBenchmark |
netcoreapp3.1 | 50.1μs | 291ns | 2.29μs | 0 | 0 | 0 | 42.7 KB |
| master | StringConcatBenchmark |
net472 | 56.7μs | 297ns | 1.39μs | 0 | 0 | 0 | 57.34 KB |
| master | StringConcatAspectBenchmark |
net6.0 | 490μs | 2.3μs | 9.22μs | 0 | 0 | 0 | 278.04 KB |
| master | StringConcatAspectBenchmark |
netcoreapp3.1 | 506μs | 2.28μs | 8.54μs | 0 | 0 | 0 | 255.46 KB |
| master | StringConcatAspectBenchmark |
net472 | 410μs | 2.31μs | 15μs | 0 | 0 | 0 | 278.53 KB |
| #7835 | StringConcatBenchmark |
net6.0 | 46.8μs | 210ns | 756ns | 0 | 0 | 0 | 43.51 KB |
| #7835 | StringConcatBenchmark |
netcoreapp3.1 | 49.3μs | 245ns | 2.22μs | 0 | 0 | 0 | 44.2 KB |
| #7835 | StringConcatBenchmark |
net472 | 56.6μs | 139ns | 520ns | 0 | 0 | 0 | 57.34 KB |
| #7835 | StringConcatAspectBenchmark |
net6.0 | 460μs | 888ns | 3.08μs | 0 | 0 | 0 | 276.99 KB |
| #7835 | StringConcatAspectBenchmark |
netcoreapp3.1 | 542μs | 2.58μs | 9.99μs | 0 | 0 | 0 | 275.94 KB |
| #7835 | StringConcatAspectBenchmark |
net472 | 414μs | 2.33μs | 15.3μ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.59μs | 14.1ns | 68.9ns | 0 | 0 | 0 | 1.7 KB |
| master | EnrichedLog |
netcoreapp3.1 | 3.65μs | 14.7ns | 56.9ns | 0 | 0 | 0 | 1.7 KB |
| master | EnrichedLog |
net472 | 4.19μs | 3.84ns | 14.9ns | 0.248 | 0 | 0 | 1.64 KB |
| #7835 | EnrichedLog |
net6.0 | 2.59μs | 14ns | 72.7ns | 0 | 0 | 0 | 1.7 KB |
| #7835 | EnrichedLog |
netcoreapp3.1 | 3.67μs | 14.2ns | 54.9ns | 0 | 0 | 0 | 1.7 KB |
| #7835 | EnrichedLog |
net472 | 3.98μs | 3.96ns | 14.8ns | 0.258 | 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 | 124μs | 55.1ns | 199ns | 0 | 0 | 0 | 4.31 KB |
| master | EnrichedLog |
netcoreapp3.1 | 129μs | 208ns | 807ns | 0 | 0 | 0 | 4.31 KB |
| master | EnrichedLog |
net472 | 169μs | 296ns | 1.14μs | 0 | 0 | 0 | 4.52 KB |
| #7835 | EnrichedLog |
net6.0 | 123μs | 66ns | 247ns | 0 | 0 | 0 | 4.31 KB |
| #7835 | EnrichedLog |
netcoreapp3.1 | 128μs | 198ns | 741ns | 0 | 0 | 0 | 4.31 KB |
| #7835 | EnrichedLog |
net472 | 169μs | 52.2ns | 195ns | 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.16μs | 21.6ns | 80.7ns | 0 | 0 | 0 | 2.26 KB |
| master | EnrichedLog |
netcoreapp3.1 | 6.77μs | 15.2ns | 58.7ns | 0 | 0 | 0 | 2.26 KB |
| master | EnrichedLog |
net472 | 7.55μs | 7.72ns | 29.9ns | 0.301 | 0 | 0 | 2.08 KB |
| #7835 | EnrichedLog |
net6.0 | 5.15μs | 25.3ns | 101ns | 0 | 0 | 0 | 2.26 KB |
| #7835 | EnrichedLog |
netcoreapp3.1 | 6.79μs | 6.44ns | 23.2ns | 0 | 0 | 0 | 2.26 KB |
| #7835 | EnrichedLog |
net472 | 7.61μs | 9.36ns | 36.3ns | 0.305 | 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 | 9.28ns | 35.9ns | 0 | 0 | 0 | 1.2 KB |
| master | SendReceive |
netcoreapp3.1 | 2.61μs | 4.31ns | 16.7ns | 0 | 0 | 0 | 1.2 KB |
| master | SendReceive |
net472 | 3.11μs | 1.28ns | 4.62ns | 0.185 | 0 | 0 | 1.2 KB |
| #7835 | SendReceive |
net6.0 | 2.02μs | 8.09ns | 31.3ns | 0 | 0 | 0 | 1.2 KB |
| #7835 | SendReceive |
netcoreapp3.1 | 2.65μs | 12.1ns | 47ns | 0 | 0 | 0 | 1.2 KB |
| #7835 | SendReceive |
net472 | 3.02μs | 3.76ns | 14.5ns | 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.33μs | 10.9ns | 42.3ns | 0 | 0 | 0 | 1.58 KB |
| master | EnrichedLog |
netcoreapp3.1 | 5.75μs | 8.04ns | 30.1ns | 0 | 0 | 0 | 1.63 KB |
| master | EnrichedLog |
net472 | 6.96μs | 6.28ns | 24.3ns | 0.313 | 0 | 0 | 2.03 KB |
| #7835 | EnrichedLog |
net6.0 | 4.26μs | 3.75ns | 14.5ns | 0 | 0 | 0 | 1.58 KB |
| #7835 | EnrichedLog |
netcoreapp3.1 | 5.74μs | 8.24ns | 31.9ns | 0 | 0 | 0 | 1.63 KB |
| #7835 | EnrichedLog |
net472 | 6.87μs | 4.45ns | 16.6ns | 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 | 786ns | 3.71ns | 14.4ns | 0 | 0 | 0 | 576 B |
| master | StartFinishSpan |
netcoreapp3.1 | 1.01μs | 4.75ns | 18.4ns | 0 | 0 | 0 | 576 B |
| master | StartFinishSpan |
net472 | 943ns | 0.246ns | 0.92ns | 0.09 | 0 | 0 | 578 B |
| master | StartFinishScope |
net6.0 | 906ns | 4.96ns | 30.6ns | 0 | 0 | 0 | 696 B |
| master | StartFinishScope |
netcoreapp3.1 | 1.19μs | 6.13ns | 31.8ns | 0 | 0 | 0 | 697 B |
| master | StartFinishScope |
net472 | 1.16μs | 0.86ns | 3.33ns | 0.104 | 0 | 0 | 658 B |
| #7835 | StartFinishSpan |
net6.0 | 775ns | 3.83ns | 15.8ns | 0 | 0 | 0 | 576 B |
| #7835 | StartFinishSpan |
netcoreapp3.1 | 965ns | 5.26ns | 30.7ns | 0 | 0 | 0 | 576 B |
| #7835 | StartFinishSpan |
net472 | 952ns | 0.275ns | 0.993ns | 0.0905 | 0 | 0 | 578 B |
| #7835 | StartFinishScope |
net6.0 | 920ns | 4.76ns | 23.3ns | 0 | 0 | 0 | 696 B |
| #7835 | StartFinishScope |
netcoreapp3.1 | 1.17μs | 6.34ns | 33.5ns | 0 | 0 | 0 | 696 B |
| #7835 | StartFinishScope |
net472 | 1.11μs | 0.172ns | 0.664ns | 0.0999 | 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 | 5.65ns | 28.2ns | 0 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
netcoreapp3.1 | 1.47μs | 6.59ns | 23.8ns | 0 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
net472 | 1.46μs | 0.443ns | 1.6ns | 0.102 | 0 | 0 | 658 B |
| #7835 | RunOnMethodBegin |
net6.0 | 1.1μs | 0.487ns | 1.89ns | 0 | 0 | 0 | 696 B |
| #7835 | RunOnMethodBegin |
netcoreapp3.1 | 1.41μs | 6.71ns | 26ns | 0 | 0 | 0 | 696 B |
| #7835 | RunOnMethodBegin |
net472 | 1.46μs | 0.314ns | 1.22ns | 0.102 | 0 | 0 | 658 B |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
...adog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/HandlerWrapperSetHandlerIntegration.cs
Outdated
Show resolved
Hide resolved
...adog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/HandlerWrapperSetHandlerIntegration.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/SDK/ILambdaContext.cs
Outdated
Show resolved
Hide resolved
...adog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/HandlerWrapperSetHandlerIntegration.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs
Show resolved
Hide resolved
tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
8a247db to
b5116dd
Compare
tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Lucas Pimentel <lucas.pimentel@datadoghq.com>
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'll add some non-blocking comments for things we can clean up later in a separate PR.
| if (state != null) | ||
| { | ||
| request.Headers.Set("lambda-runtime-aws-request-id", (string)state); | ||
| } |
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.
Although it should never happen, if state is ever a different type from string, this casting operation will throw an exception. It would be safer to check that state is a string, not simply not null
The more modern way of doing this in C# is by combining the type check with a string variable declaration and removing the (string) casting operator:
if (state is string requestId)
{
request.Headers.Set("lambda-runtime-aws-request-id", requestId);
}| } | ||
|
|
||
| internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) | ||
| internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, object state, bool isError, string data) |
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 general we should try to avoid using the object type when passing arguments around. C# developers like the strong type safety that the language provides and treating anything as object disable that language feature.
To avoid that, we should cast the state to CallTargetState as early as possible and pass it around as a CallTargetState, not as object.
| var request = requestBuilder.GetStartInvocationRequest(); | ||
| WriteRequestPayload(request, data); | ||
| WriteRequestHeaders(request, context); | ||
| WriteRequestHeaders(request, context?.ClientContext?.Custom); |
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 either context or context.ClientContext or context.ClientContext.Custom are null, we will effectively call WriteRequestHeaders(request, null).
Does WriteRequestHeaders() handle null gracefully? Or should we check for null before calling?
if (context?.ClientContext?.Custom is { } c)
{
WriteRequestHeaders(request, c);
}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.
WriteRequestHeaders starts by checking if context is null (if it is null it does nothing). This was how it was structured prior to my changes, so I am choosing to leave it that way.
| } | ||
|
|
||
| WebRequest ILambdaExtensionRequest.GetEndInvocationRequest(Scope scope, bool isError) | ||
| WebRequest ILambdaExtensionRequest.GetEndInvocationRequest(Scope scope, object state, bool isError) |
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.
Here's a good example of why not to not use object in method signature if we can avoid it: in some methods state is a CallTargetState, but here it's a string. It's easy to get them confused and try to cast to the wrong type, which will throw an exception at run time.
Or if someone mistakenly checks if (state is CallTargetState) before casting, it will always be false (because this one is a string) and we'll never hit the code path below that adds the request header.
| LambdaCommon.Log("DelegateWrapper Running OnDelegateBegin"); | ||
|
|
||
| Scope scope; | ||
| object requestid = null; |
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.
[naming conventions]
We should use requestId instead of requestid (this is the nit-pickiest comment in this PR, I promise)
[type safety]
requestid can be string since ILambdaContext.AwsRequestId is declared as string. We want to avoid using object as much as possible.
| } | ||
|
|
||
| internal static async Task EndInvocationAsync(string returnValue, Exception exception, Scope scope, ILambdaExtensionRequest requestBuilder) | ||
| internal static async Task EndInvocationAsync(string returnValue, Exception exception, object stateObject, ILambdaExtensionRequest requestBuilder) |
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.
Same comment here about the using object as parameter type.
| internal static async Task EndInvocationAsync(string returnValue, Exception exception, Scope scope, ILambdaExtensionRequest requestBuilder) | ||
| internal static async Task EndInvocationAsync(string returnValue, Exception exception, object stateObject, ILambdaExtensionRequest requestBuilder) | ||
| { | ||
| var state = (CallTargetState)stateObject!; |
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.
This is a dangerous cast that will throw an exception if stateObject is null or otherwise not a CallTargetState. Since stateObject is declared as object, it could be anything, so we should always be defensive and check first:
if (state is CallTargetState)
{
...Even better would be to use CallTargetState as the parameter type instead of object. Then the type is guaranteed and we won't need to either check or cast here.
Summary of changes
Adds
lambda-runtime-aws-request-idheader in start invocation and end invocation requests from tracer to extension. Assigns header to the aws request id of the span.Reason for change
Allows the lambda extension to identify which start/end invocation requests belong to which invocation if multiple Lambda requests are being served concurrently.
Implementation details
Uses the LambdaContext from the
IInvocationRequest proxyinstanceto get the aws-request-id inSendStartInvocationand adds it to request headers inWriteRequestHeaders. Saves the aws-request-id as part of the state passed fromOnDelegateBegintoEndInvocationAsync.EndInvocationAsyncpasses the state down toGetEndInvocationRequest, which adds it to the request headers.Updates the LambdaCommonTests and LambdaRequestBuilderTests to match new function signatures with state passed down and to test for the assignment to the lambda-runtime-aws-request-id.
Test coverage
Added unit test in LambdaRequestBuilderTests to check for
lambda-runtime-aws-request-idheader and value in end invocation request. Manually tested for headers in start and end invocation using custom dd-trace-dotnet layerarn:aws:lambda:sa-east-1:425362996713:layer:dotnet-request-id-header-rithika:2and custom lambda extensionarn:aws:lambda:sa-east-1:425362996713:layer:Debug-extension-rithika:1which prints the start and end invocation headers. Sample trace with logs of the headers hereOther details