-
Notifications
You must be signed in to change notification settings - Fork 147
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
[ASM] Avoid null context repetitive log messages #6607
Conversation
Datadog ReportBranch report: ❌ 2 Failed (0 Known Flaky), 246160 Passed, 3051 Skipped, 33h 7m 43.82s Total Time ❌ Failed Tests (2)
⌛ Performance Regressions vs Default Branch (1)
|
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.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6607) - mean (69ms) : 66, 71
. : milestone, 69,
master - mean (69ms) : 65, 74
. : milestone, 69,
section CallTarget+Inlining+NGEN
This PR (6607) - mean (982ms) : 955, 1010
. : milestone, 982,
master - mean (981ms) : 959, 1003
. : milestone, 981,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6607) - mean (107ms) : 105, 110
. : milestone, 107,
master - mean (108ms) : 106, 111
. : milestone, 108,
section CallTarget+Inlining+NGEN
This PR (6607) - mean (675ms) : 661, 688
. : milestone, 675,
master - mean (676ms) : 660, 691
. : milestone, 676,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6607) - mean (91ms) : 89, 93
. : milestone, 91,
master - mean (91ms) : 89, 93
. : milestone, 91,
section CallTarget+Inlining+NGEN
This PR (6607) - mean (622ms) : 606, 637
. : milestone, 622,
master - mean (637ms) : 614, 660
. : milestone, 637,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6607) - mean (190ms) : 185, 195
. : milestone, 190,
master - mean (190ms) : 185, 195
. : milestone, 190,
section CallTarget+Inlining+NGEN
This PR (6607) - mean (1,090ms) : 1064, 1116
. : milestone, 1090,
master - mean (1,087ms) : 1064, 1110
. : milestone, 1087,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6607) - mean (277ms) : 273, 281
. : milestone, 277,
master - mean (276ms) : 272, 281
. : milestone, 276,
section CallTarget+Inlining+NGEN
This PR (6607) - mean (864ms) : 842, 887
. : milestone, 864,
master - mean (865ms) : 832, 897
. : milestone, 865,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6607) - mean (265ms) : 261, 269
. : milestone, 265,
master - mean (264ms) : 260, 268
. : milestone, 264,
section CallTarget+Inlining+NGEN
This PR (6607) - mean (840ms) : 812, 869
. : milestone, 840,
master - mean (846ms) : 807, 885
. : milestone, 846,
|
Benchmarks Report for tracer 🐌Benchmarks for #6607 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 ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 | 1.340 | 400.98 | 537.35 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StartFinishSpan |
net6.0 | 401ns | 0.491ns | 1.84ns | 0.008 | 0 | 0 | 576 B |
master | StartFinishSpan |
netcoreapp3.1 | 607ns | 0.872ns | 3.26ns | 0.00765 | 0 | 0 | 576 B |
master | StartFinishSpan |
net472 | 601ns | 1.03ns | 4ns | 0.0916 | 0 | 0 | 578 B |
master | StartFinishScope |
net6.0 | 520ns | 1.08ns | 4.18ns | 0.00982 | 0 | 0 | 696 B |
master | StartFinishScope |
netcoreapp3.1 | 704ns | 1.14ns | 4.42ns | 0.00955 | 0 | 0 | 696 B |
master | StartFinishScope |
net472 | 802ns | 2.6ns | 10.1ns | 0.104 | 0 | 0 | 658 B |
#6607 | StartFinishSpan |
net6.0 | 537ns | 0.939ns | 3.64ns | 0.00811 | 0 | 0 | 576 B |
#6607 | StartFinishSpan |
netcoreapp3.1 | 547ns | 1.23ns | 4.76ns | 0.00773 | 0 | 0 | 576 B |
#6607 | StartFinishSpan |
net472 | 600ns | 0.877ns | 3.4ns | 0.0916 | 0 | 0 | 578 B |
#6607 | StartFinishScope |
net6.0 | 558ns | 0.895ns | 3.47ns | 0.00987 | 0 | 0 | 696 B |
#6607 | StartFinishScope |
netcoreapp3.1 | 699ns | 1.52ns | 5.9ns | 0.00936 | 0 | 0 | 696 B |
#6607 | StartFinishScope |
net472 | 786ns | 1.73ns | 6.68ns | 0.104 | 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 | 578ns | 0.723ns | 2.8ns | 0.00991 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
netcoreapp3.1 | 897ns | 1.35ns | 5.05ns | 0.00921 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
net472 | 1.05μs | 2.36ns | 9.15ns | 0.104 | 0 | 0 | 658 B |
#6607 | RunOnMethodBegin |
net6.0 | 618ns | 0.689ns | 2.67ns | 0.00983 | 0 | 0 | 696 B |
#6607 | RunOnMethodBegin |
netcoreapp3.1 | 974ns | 1.64ns | 6.36ns | 0.00925 | 0 | 0 | 696 B |
#6607 | RunOnMethodBegin |
net472 | 1.07μs | 1.71ns | 6.64ns | 0.104 | 0 | 0 | 658 B |
Benchmarks Report for appsec 🐌Benchmarks for #6607 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 - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Iast.StringAspectsBenchmark - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 | 1.793 | 320,850.00 | 575,400.00 | bimodal |
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 | 1.202 | 53,900.00 | 64,800.00 | bimodal |
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 | 255.53 KB | 317.02 KB | 61.49 KB | 24.06% |
Benchmark | Base Allocated | Diff Allocated | Change | Change % |
---|---|---|---|---|
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 253.9 KB | 252.25 KB | -1.66 KB | -0.65% |
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 | 59.89 KB | 59.07 KB | -816 B | -1.36% |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StringConcatBenchmark |
net6.0 | 58.2μs | 730ns | 7.23μs | 0 | 0 | 0 | 43.44 KB |
master | StringConcatBenchmark |
netcoreapp3.1 | 54.9μs | 303ns | 2.78μs | 0 | 0 | 0 | 42.64 KB |
master | StringConcatBenchmark |
net472 | 37.4μs | 196ns | 920ns | 0 | 0 | 0 | 59.89 KB |
master | StringConcatAspectBenchmark |
net6.0 | 326μs | 1.87μs | 14μs | 0 | 0 | 0 | 255.53 KB |
master | StringConcatAspectBenchmark |
netcoreapp3.1 | 355μs | 2.04μs | 14.7μs | 0 | 0 | 0 | 253.9 KB |
master | StringConcatAspectBenchmark |
net472 | 249μs | 1.12μs | 4.03μs | 0 | 0 | 0 | 278.53 KB |
#6607 | StringConcatBenchmark |
net6.0 | 60.2μs | 737ns | 7.3μs | 0 | 0 | 0 | 43.44 KB |
#6607 | StringConcatBenchmark |
netcoreapp3.1 | 66.1μs | 914ns | 9.05μs | 0 | 0 | 0 | 42.64 KB |
#6607 | StringConcatBenchmark |
net472 | 37.2μs | 110ns | 398ns | 0 | 0 | 0 | 59.07 KB |
#6607 | StringConcatAspectBenchmark |
net6.0 | 543μs | 9.33μs | 91.8μs | 0 | 0 | 0 | 317.02 KB |
#6607 | StringConcatAspectBenchmark |
netcoreapp3.1 | 336μs | 1.9μs | 15.6μs | 0 | 0 | 0 | 252.25 KB |
#6607 | StringConcatAspectBenchmark |
net472 | 299μs | 7.32μs | 71μs | 0 | 0 | 0 | 278.53 KB |
Thanks for you reviews! |
Summary of changes
Within a customer, we are displaying a warning message when calling RASP with a null context. The problem is that the message is sent too many times, flooding the log.
In this PR, we have lowered the log level of the warning message after the first call.
Also, we are displaying invalid aspect class in dataflow.cpp. The reason is that RASP is enabled and IAST disabled, so some aspects are not valid within this context. While this is a normal situation, the level of the log message might suggest that there is a anomalous situation.
Reason for change
Implementation details
Test coverage
Other details