-
Notifications
You must be signed in to change notification settings - Fork 142
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
[IAST] Move analyzers init to an explicit call #5920
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 344564 Passed, 1780 Skipped, 15h 7m 14.82s Total Time |
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). |
Benchmarks Report for appsec 🐌Benchmarks for #5920 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.Asm.AppSecBodyBenchmark - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1 | 1.134 | 210.62 | 238.88 |
Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472 | 1.172 | 4,439.31 | 3,788.06 | |
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net472 | 1.157 | 196.29 | 169.61 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | AllCycleSimpleBody |
net6.0 | 74.3μs | 109ns | 422ns | 0.0749 | 0 | 0 | 6 KB |
master | AllCycleSimpleBody |
netcoreapp3.1 | 62μs | 76.1ns | 295ns | 0.0939 | 0 | 0 | 6.95 KB |
master | AllCycleSimpleBody |
net472 | 49.4μs | 55.2ns | 214ns | 1.31 | 0 | 0 | 8.34 KB |
master | AllCycleMoreComplexBody |
net6.0 | 78.8μs | 76.9ns | 298ns | 0.118 | 0 | 0 | 9.51 KB |
master | AllCycleMoreComplexBody |
netcoreapp3.1 | 69.2μs | 67ns | 260ns | 0.138 | 0 | 0 | 10.36 KB |
master | AllCycleMoreComplexBody |
net472 | 55.9μs | 65.8ns | 246ns | 1.87 | 0.0279 | 0 | 11.85 KB |
master | ObjectExtractorSimpleBody |
net6.0 | 140ns | 0.117ns | 0.452ns | 0.00397 | 0 | 0 | 280 B |
master | ObjectExtractorSimpleBody |
netcoreapp3.1 | 210ns | 0.136ns | 0.526ns | 0.0037 | 0 | 0 | 272 B |
master | ObjectExtractorSimpleBody |
net472 | 196ns | 0.132ns | 0.511ns | 0.0446 | 0 | 0 | 281 B |
master | ObjectExtractorMoreComplexBody |
net6.0 | 3.06μs | 1.67ns | 6.24ns | 0.0535 | 0 | 0 | 3.78 KB |
master | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 3.96μs | 1.82ns | 7.04ns | 0.0495 | 0 | 0 | 3.69 KB |
master | ObjectExtractorMoreComplexBody |
net472 | 4.44μs | 2.34ns | 8.75ns | 0.602 | 0.00666 | 0 | 3.8 KB |
#5920 | AllCycleSimpleBody |
net6.0 | 72.7μs | 79.9ns | 299ns | 0.0731 | 0 | 0 | 6.01 KB |
#5920 | AllCycleSimpleBody |
netcoreapp3.1 | 62.2μs | 81.9ns | 306ns | 0.0929 | 0 | 0 | 6.95 KB |
#5920 | AllCycleSimpleBody |
net472 | 49μs | 52.6ns | 189ns | 1.3 | 0 | 0 | 8.34 KB |
#5920 | AllCycleMoreComplexBody |
net6.0 | 78.8μs | 79.4ns | 297ns | 0.121 | 0 | 0 | 9.51 KB |
#5920 | AllCycleMoreComplexBody |
netcoreapp3.1 | 70.5μs | 89.6ns | 347ns | 0.105 | 0 | 0 | 10.36 KB |
#5920 | AllCycleMoreComplexBody |
net472 | 55.8μs | 43.2ns | 167ns | 1.86 | 0.0282 | 0 | 11.85 KB |
#5920 | ObjectExtractorSimpleBody |
net6.0 | 142ns | 0.188ns | 0.702ns | 0.00394 | 0 | 0 | 280 B |
#5920 | ObjectExtractorSimpleBody |
netcoreapp3.1 | 239ns | 0.163ns | 0.586ns | 0.00373 | 0 | 0 | 272 B |
#5920 | ObjectExtractorSimpleBody |
net472 | 169ns | 0.34ns | 1.32ns | 0.0446 | 0 | 0 | 281 B |
#5920 | ObjectExtractorMoreComplexBody |
net6.0 | 3.04μs | 2.12ns | 7.94ns | 0.054 | 0 | 0 | 3.78 KB |
#5920 | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 3.84μs | 2.73ns | 10.6ns | 0.0499 | 0 | 0 | 3.69 KB |
#5920 | ObjectExtractorMoreComplexBody |
net472 | 3.79μs | 2.58ns | 9.64ns | 0.603 | 0.00569 | 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 | 38.1μs | 22.9ns | 88.8ns | 0.454 | 0 | 0 | 32.4 KB |
master | EncodeArgs |
netcoreapp3.1 | 54.7μs | 31.7ns | 123ns | 0.433 | 0 | 0 | 32.4 KB |
master | EncodeArgs |
net472 | 65.9μs | 29.5ns | 114ns | 5.15 | 0.0664 | 0 | 32.5 KB |
master | EncodeLegacyArgs |
net6.0 | 80.5μs | 172ns | 667ns | 0.0399 | 0 | 0 | 2.14 KB |
master | EncodeLegacyArgs |
netcoreapp3.1 | 108μs | 109ns | 420ns | 0 | 0 | 0 | 2.14 KB |
master | EncodeLegacyArgs |
net472 | 151μs | 33.6ns | 126ns | 0.302 | 0 | 0 | 2.15 KB |
#5920 | EncodeArgs |
net6.0 | 37.9μs | 18ns | 69.7ns | 0.456 | 0 | 0 | 32.4 KB |
#5920 | EncodeArgs |
netcoreapp3.1 | 54.9μs | 15.1ns | 54.4ns | 0.438 | 0 | 0 | 32.4 KB |
#5920 | EncodeArgs |
net472 | 66.7μs | 42.3ns | 164ns | 5.16 | 0.0666 | 0 | 32.5 KB |
#5920 | EncodeLegacyArgs |
net6.0 | 77.2μs | 422ns | 2.43μs | 0 | 0 | 0 | 2.14 KB |
#5920 | EncodeLegacyArgs |
netcoreapp3.1 | 105μs | 254ns | 982ns | 0 | 0 | 0 | 2.15 KB |
#5920 | EncodeLegacyArgs |
net472 | 152μs | 54ns | 209ns | 0.305 | 0 | 0 | 2.15 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 | 188μs | 67.1ns | 251ns | 0 | 0 | 0 | 2.42 KB |
master | RunWafRealisticBenchmark |
netcoreapp3.1 | 197μs | 124ns | 447ns | 0 | 0 | 0 | 2.37 KB |
master | RunWafRealisticBenchmark |
net472 | 211μs | 49.4ns | 178ns | 0.316 | 0 | 0 | 2.43 KB |
master | RunWafRealisticBenchmarkWithAttack |
net6.0 | 122μs | 105ns | 407ns | 0 | 0 | 0 | 1.46 KB |
master | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 131μs | 70ns | 271ns | 0 | 0 | 0 | 1.45 KB |
master | RunWafRealisticBenchmarkWithAttack |
net472 | 140μs | 38.4ns | 144ns | 0.21 | 0 | 0 | 1.48 KB |
#5920 | RunWafRealisticBenchmark |
net6.0 | 185μs | 32.2ns | 120ns | 0 | 0 | 0 | 2.42 KB |
#5920 | RunWafRealisticBenchmark |
netcoreapp3.1 | 196μs | 110ns | 426ns | 0 | 0 | 0 | 2.37 KB |
#5920 | RunWafRealisticBenchmark |
net472 | 209μs | 51.8ns | 194ns | 0.314 | 0 | 0 | 2.43 KB |
#5920 | RunWafRealisticBenchmarkWithAttack |
net6.0 | 122μs | 83.6ns | 324ns | 0 | 0 | 0 | 1.46 KB |
#5920 | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 130μs | 52.5ns | 189ns | 0 | 0 | 0 | 1.45 KB |
#5920 | RunWafRealisticBenchmarkWithAttack |
net472 | 139μs | 45.7ns | 171ns | 0.209 | 0 | 0 | 1.48 KB |
Benchmarks.Trace.Iast.StringAspectsBenchmark - Faster 🎉 Same allocations ✔️
Faster 🎉 in #5920
Benchmark
base/diff
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1
1.192
63,200.00
53,000.00
bimodal
Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 | 1.192 | 63,200.00 | 53,000.00 | bimodal |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StringConcatBenchmark |
net6.0 | 59.9μs | 752ns | 7.44μs | 0 | 0 | 0 | 43.44 KB |
master | StringConcatBenchmark |
netcoreapp3.1 | 64.1μs | 911ns | 8.93μs | 0 | 0 | 0 | 42.64 KB |
master | StringConcatBenchmark |
net472 | 37μs | 120ns | 432ns | 0 | 0 | 0 | 59.04 KB |
master | StringConcatAspectBenchmark |
net6.0 | 319μs | 1.76μs | 11.2μs | 0 | 0 | 0 | 264.42 KB |
master | StringConcatAspectBenchmark |
netcoreapp3.1 | 336μs | 1.21μs | 4.37μs | 0 | 0 | 0 | 255.34 KB |
master | StringConcatAspectBenchmark |
net472 | 277μs | 5.54μs | 54μs | 0 | 0 | 0 | 278.53 KB |
#5920 | StringConcatBenchmark |
net6.0 | 61.1μs | 730ns | 7.23μs | 0 | 0 | 0 | 43.44 KB |
#5920 | StringConcatBenchmark |
netcoreapp3.1 | 53.1μs | 293ns | 1.73μs | 0 | 0 | 0 | 42.64 KB |
#5920 | StringConcatBenchmark |
net472 | 37μs | 85.8ns | 321ns | 0 | 0 | 0 | 59.04 KB |
#5920 | StringConcatAspectBenchmark |
net6.0 | 305μs | 1.6μs | 10.8μs | 0 | 0 | 0 | 264.1 KB |
#5920 | StringConcatAspectBenchmark |
netcoreapp3.1 | 336μs | 1.76μs | 12.7μs | 0 | 0 | 0 | 254.33 KB |
#5920 | StringConcatAspectBenchmark |
net472 | 292μs | 5.62μs | 54.8μs | 0 | 0 | 0 | 278.53 KB |
## Summary of changes Make IAST analyzers initialization explicit, and call it only in `Instrumentation.Initialize` to avoid multiple instances scenarios ## Reason for change A crash under these circumstances has been detected ## Implementation details Move IAST analyzers init inside `Instrumentation.Initialize` only to make sure they are initialized only once ## Test coverage ## Other details <!-- Fixes #{issue} --> <!--⚠️ Note: where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. -->
## Summary of changes Make IAST analyzers initialization explicit, and call it only in `Instrumentation.Initialize` to avoid multiple instances scenarios ## Reason for change A crash under these circumstances has been detected ## Implementation details Move IAST analyzers init inside `Instrumentation.Initialize` only to make sure they are initialized only once ## Test coverage ## Other details Backport of #5920 Co-authored-by: Daniel Romano <[email protected]>
Summary of changes
Make IAST analyzers initialization explicit, and call it only in
Instrumentation.Initialize
to avoid multiple instances scenariosReason for change
A crash under these circumstances has been detected
Implementation details
Move IAST analyzers init inside
Instrumentation.Initialize
only to make sure they are initialized only onceTest coverage
Other details