Skip to content

[IAST] Small string cache bugfix #5064

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jan 22, 2024
Merged

[IAST] Small string cache bugfix #5064

merged 7 commits into from
Jan 22, 2024

Conversation

daniel-romano-DD
Copy link
Contributor

Summary of changes

Avoid tainting of framework cached digit input strings

Reason for change

Framework caches and reuses ToString() results for short (0-9) digits (pre DotNet 8.0) or 0-299 from DotNet 8.0 onwards. This has caused many false possitives, as any digit in an input would cause the use of the same digit in a sink to raise a false possitive.
It happened in a "Select TOP (value)" where the value was the same as the content of a header (small number).

Implementation details

Filter and avoid tainting of digit input strings (TaintedObjects.TaintInputString())

PrivateCorelib implementation
image

Test coverage

Added unit test to check the behavior in pre and post DotNet 8.0

andrewlock
andrewlock reviewed Jan 16, 2024
View reviewed changes
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Jan 16, 2024 •
edited
Loading

Datadog Report

Branch report: dani/asm/small_string_cache_bugfix
Commit report: df837b9
Test service: dd-trace-dotnet

❌ 21 Failed (0 Known Flaky), 310651 Passed, 1507 Skipped, 1h 0m 44.13s Wall Time

❌ Failed Tests (21)

This report shows up to 5 failed tests.

  • CompilerGeneratedClassTest - Datadog.Trace.Tests.Debugger.SymbolsTests.SymbolExtractorTest

  • Test - Datadog.Trace.Tests.Debugger.SymbolsTests.SymbolExtractorTest

  • Test - Datadog.Trace.Tests.Debugger.SymbolsTests.SymbolExtractorTest

  • Test - Datadog.Trace.Tests.Debugger.SymbolsTests.SymbolExtractorTest

  • Test - Datadog.Trace.Tests.Debugger.SymbolsTests.SymbolExtractorTest

@andrewlock
Copy link
Member

andrewlock commented Jan 16, 2024 •
edited
Loading

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:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

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 (5064) - mean (72ms)  : 60, 85
     .   : milestone, 72,
    master - mean (73ms)  : 63, 84
     .   : milestone, 73,

    section CallTarget+Inlining+NGEN
    This PR (5064) - mean (960ms)  : 934, 985
     .   : milestone, 960,
    master - mean (1,028ms)  : 1003, 1053
     .   : milestone, 1028,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5064) - mean (107ms)  : 104, 109
     .   : milestone, 107,
    master - mean (106ms)  : 102, 110
     .   : milestone, 106,

    section CallTarget+Inlining+NGEN
    This PR (5064) - mean (707ms)  : 689, 725
     .   : milestone, 707,
    master - mean (731ms)  : 714, 748
     .   : milestone, 731,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5064) - mean (90ms)  : 88, 93
     .   : milestone, 90,
    master - mean (90ms)  : 87, 93
     .   : milestone, 90,

    section CallTarget+Inlining+NGEN
    This PR (5064) - mean (663ms)  : 643, 682
     .   : milestone, 663,
    master - mean (693ms)  : 669, 716
     .   : milestone, 693,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5064) - mean (188ms)  : 186, 191
     .   : milestone, 188,
    master - mean (189ms)  : 185, 192
     .   : milestone, 189,

    section CallTarget+Inlining+NGEN
    This PR (5064) - mean (1,067ms)  : 1044, 1091
     .   : milestone, 1067,
    master - mean (1,135ms)  : 1117, 1154
     .   : milestone, 1135,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5064) - mean (272ms)  : 268, 275
     .   : milestone, 272,
    master - mean (272ms)  : 267, 277
     .   : milestone, 272,

    section CallTarget+Inlining+NGEN
    This PR (5064) - mean (1,053ms)  : 1034, 1073
     .   : milestone, 1053,
    master - mean (1,086ms)  : 1064, 1109
     .   : milestone, 1086,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5064) - mean (262ms)  : 258, 265
     .   : milestone, 262,
    master - mean (261ms)  : 258, 265
     .   : milestone, 261,

    section CallTarget+Inlining+NGEN
    This PR (5064) - mean (1,018ms)  : 993, 1042
     .   : milestone, 1018,
    master - mean (1,053ms)  : 1033, 1073
     .   : milestone, 1053,
Loading

@andrewlock
Copy link
Member

andrewlock commented Jan 16, 2024 •
edited
Loading

Throughput/Crank Report:zap:

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5064) (11.810M)   : 0, 11810456
    master (11.793M)   : 0, 11792846
    benchmarks/2.9.0 (11.757M)   : 0, 11756801

    section Automatic
    This PR (5064) (8.110M)   : 0, 8110037
    master (8.076M)   : 0, 8076448
    benchmarks/2.9.0 (8.573M)   : 0, 8573327

    section Trace stats
    This PR (5064) (8.491M)   : 0, 8490900
    master (8.407M)   : 0, 8406782

    section Manual
    This PR (5064) (10.196M)   : 0, 10195833
    master (10.353M)   : 0, 10352820

    section Manual + Automatic
    This PR (5064) (7.700M)   : 0, 7699890
    master (7.610M)   : 0, 7610023

    section Version Conflict
    This PR (5064) (6.915M)   : 0, 6914757
    master (6.845M)   : 0, 6845162
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5064) (9.492M)   : 0, 9492169
    master (9.498M)   : 0, 9497633
    benchmarks/2.9.0 (9.605M)   : 0, 9605360

    section Automatic
    This PR (5064) (6.563M)   : 0, 6563220
    master (6.538M)   : 0, 6538176

    section Trace stats
    This PR (5064) (6.859M)   : 0, 6859141
    master (6.967M)   : 0, 6966769

    section Manual
    This PR (5064) (8.130M)   : 0, 8129782
    master (8.183M)   : 0, 8183082

    section Manual + Automatic
    This PR (5064) (6.047M)   : 0, 6046946
    master (6.155M)   : 0, 6155194

    section Version Conflict
    This PR (5064) (5.576M)   : 0, 5575896
    master (5.582M)   : 0, 5581839
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5064) (10.126M)   : 0, 10126037
    master (10.290M)   : 0, 10289786
    benchmarks/2.9.0 (9.956M)   : 0, 9956027

    section Automatic
    This PR (5064) (7.124M)   : 0, 7123543
    master (7.052M)   : 0, 7051599
    benchmarks/2.9.0 (7.438M)   : 0, 7437608

    section Trace stats
    This PR (5064) (7.367M)   : 0, 7367484
    master (7.398M)   : 0, 7397958

    section Manual
    This PR (5064) (9.053M)   : 0, 9053213
    master (8.955M)   : 0, 8954877

    section Manual + Automatic
    This PR (5064) (6.924M)   : 0, 6923623
    master (6.754M)   : 0, 6754356

    section Version Conflict
    This PR (5064) (6.115M)   : 0, 6115262
    master (6.121M)   : 0, 6120614
Loading 
gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5064) (7.216M)   : 0, 7216241
    master (7.304M)   : 0, 7303586
    benchmarks/2.9.0 (7.813M)   : 0, 7813475

    section No attack
    This PR (5064) (1.761M)   : 0, 1760845
    master (1.762M)   : 0, 1762048
    benchmarks/2.9.0 (3.251M)   : 0, 3251456

    section Attack
    This PR (5064) (1.427M)   : 0, 1427369
    master (1.413M)   : 0, 1413289
    benchmarks/2.9.0 (2.577M)   : 0, 2577462

    section Blocking
    This PR (5064) (3.146M)   : 0, 3146210
    master (3.108M)   : 0, 3107757

    section IAST default
    This PR (5064) (6.294M)   : 0, 6293682
    master (6.434M)   : 0, 6433540

    section IAST full
    This PR (5064) (5.454M)   : 0, 5454065
    master (5.532M)   : 0, 5532224

    section Base vuln
    This PR (5064) (0.940M)   : 0, 940284
    master (0.929M)   : 0, 928748

    section IAST vuln
    This PR (5064) (0.855M)   : 0, 854867
    master (0.890M)   : 0, 890135
Loading

tonyredondo
tonyredondo reviewed Jan 16, 2024
View reviewed changes
tracer/src/Datadog.Trace/Iast/TaintedObjects.cs
{
if (!string.IsNullOrEmpty(stringToTaint))
if (!IsFiltered(stringToTaint))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are not longer checking if stringToTaint is null in IsFiltered

Copy link
Contributor Author

@daniel-romano-DD daniel-romano-DD Jan 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String should not be null here (#nullable enable, but IsFiltered checks for null also

@andrewlock
Copy link
Member

andrewlock commented Jan 16, 2024 •
edited
Loading

Benchmarks Report 🐌

Benchmarks for #5064 compared to master:

  • 2 benchmarks are slower, with geometric mean 1.143
  • 2 benchmarks have fewer allocations
  • 1 benchmarks have more allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 8.77μs 48.5ns 314ns 0.0225 0.00898 0 7.48 KB
master StartStopWithChild netcoreapp3.1 10.9μs 61ns 381ns 0.0326 0.0163 0 7.58 KB
master StartStopWithChild net472 17.2μs 70.3ns 272ns 1.34 0.342 0.111 7.95 KB
#5064 StartStopWithChild net6.0 8.8μs 46.4ns 245ns 0.018 0.009 0 7.48 KB
#5064 StartStopWithChild netcoreapp3.1 10.6μs 59.7ns 387ns 0.0254 0.0102 0 7.58 KB
#5064 StartStopWithChild net472 17.1μs 27.4ns 98.6ns 1.34 0.333 0.108 7.96 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 455μs 242ns 938ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 629μs 149ns 556ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 815μs 312ns 1.21μs 0.403 0 0 3.3 KB
#5064 WriteAndFlushEnrichedTraces net6.0 466μs 354ns 1.37μs 0 0 0 2.7 KB
#5064 WriteAndFlushEnrichedTraces netcoreapp3.1 634μs 290ns 1.09μs 0 0 0 2.7 KB
#5064 WriteAndFlushEnrichedTraces net472 812μs 167ns 625ns 0.403 0 0 3.3 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 38.8μs 30.9ns 120ns 0.0195 0 0 1.77 KB
master AllCycleSimpleBody netcoreapp3.1 41.3μs 38.2ns 148ns 0.0207 0 0 1.74 KB
master AllCycleSimpleBody net472 44.8μs 18.7ns 72.3ns 0.288 0 0 1.81 KB
master AllCycleMoreComplexBody net6.0 200μs 85.1ns 330ns 0.0995 0 0 9.25 KB
master AllCycleMoreComplexBody netcoreapp3.1 211μs 241ns 932ns 0.106 0 0 9.14 KB
master AllCycleMoreComplexBody net472 225μs 152ns 588ns 1.46 0 0 9.32 KB
master ObjectExtractorSimpleBody net6.0 146ns 0.19ns 0.711ns 0.00396 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 215ns 0.133ns 0.513ns 0.00379 0 0 272 B
master ObjectExtractorSimpleBody net472 165ns 0.316ns 1.22ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 3.05μs 1.79ns 6.44ns 0.0533 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 4.22μs 1.33ns 4.81ns 0.0506 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 3.81μs 5.91ns 22.1ns 0.602 0.00568 0 3.8 KB
#5064 AllCycleSimpleBody net6.0 39.3μs 75.6ns 293ns 0.0197 0 0 1.77 KB
#5064 AllCycleSimpleBody netcoreapp3.1 41.7μs 20.9ns 78.2ns 0.0208 0 0 1.74 KB
#5064 AllCycleSimpleBody net472 45.4μs 21.4ns 82.7ns 0.287 0 0 1.81 KB
#5064 AllCycleMoreComplexBody net6.0 200μs 73.9ns 276ns 0.1 0 0 9.25 KB
#5064 AllCycleMoreComplexBody netcoreapp3.1 212μs 102ns 380ns 0.106 0 0 9.14 KB
#5064 AllCycleMoreComplexBody net472 225μs 66.8ns 250ns 1.46 0 0 9.32 KB
#5064 ObjectExtractorSimpleBody net6.0 138ns 0.228ns 0.883ns 0.00391 0 0 280 B
#5064 ObjectExtractorSimpleBody netcoreapp3.1 199ns 0.177ns 0.685ns 0.00371 0 0 272 B
#5064 ObjectExtractorSimpleBody net472 173ns 0.252ns 0.977ns 0.0446 0 0 281 B
#5064 ObjectExtractorMoreComplexBody net6.0 3.08μs 1.25ns 4.67ns 0.0525 0 0 3.78 KB
#5064 ObjectExtractorMoreComplexBody netcoreapp3.1 3.93μs 1.52ns 5.47ns 0.0511 0 0 3.69 KB
#5064 ObjectExtractorMoreComplexBody net472 3.9μs 2.33ns 8.41ns 0.602 0.00582 0 3.8 KB
Benchmarks.Trace.Asm.AppSecWafBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5064

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWaf(args: NestedMap (10))‑net6.0 1.143 51,858.08 59,266.60

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunWaf(args=NestedMap (10)) net6.0 51.9μs 20.3ns 73.1ns 0.208 0 0 16.06 KB
master RunWaf(args=NestedMap (10)) netcoreapp3.1 70.2μs 389ns 2.4μs 0.196 0 0 16.06 KB
master RunWaf(args=NestedMap (10)) net472 95.8μs 29.1ns 113ns 2.55 0.0962 0 16.14 KB
master RunWafTwice(args=NestedMap (10)) net6.0 57.5μs 11.2ns 42.1ns 0.218 0 0 16.6 KB
master RunWafTwice(args=NestedMap (10)) netcoreapp3.1 70.5μs 342ns 1.41μs 0.21 0 0 16.58 KB
master RunWafTwice(args=NestedMap (10)) net472 109μs 309ns 1.16μs 2.65 0.104 0 16.69 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 114μs 232ns 897ns 0.286 0 0 22.41 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 130μs 652ns 2.99μs 0.254 0 0 22.36 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net472 165μs 562ns 2.1μs 3.56 0.162 0 22.7 KB
master RunWaf(args=NestedMap (100)) net6.0 98.1μs 56.8ns 220ns 0.441 0 0 32.76 KB
master RunWaf(args=NestedMap (100)) netcoreapp3.1 131μs 734ns 5.09μs 0.439 0 0 33.33 KB
master RunWaf(args=NestedMap (100)) net472 192μs 847ns 3.28μs 5.34 0.375 0 33.67 KB
master RunWafTwice(args=NestedMap (100)) net6.0 105μs 56.3ns 195ns 0.47 0 0 33.3 KB
master RunWafTwice(args=NestedMap (100)) netcoreapp3.1 137μs 750ns 4.18μs 0.456 0 0 33.86 KB
master RunWafTwice(args=NestedMap (100)) net472 196μs 56.9ns 213ns 5.37 0.391 0 34.23 KB
master RunWafWithAttack(args=Neste(...)tack) [23]) net6.0 160μs 41.4ns 155ns 0.498 0 0 39.1 KB
master RunWafWithAttack(args=Neste(...)tack) [23]) netcoreapp3.1 197μs 99.9ns 346ns 0.507 0 0 39.63 KB
master RunWafWithAttack(args=Neste(...)tack) [23]) net472 257μs 1.12μs 4.33μs 6.31 0.505 0 40.23 KB
master RunWaf(args=NestedMap (20)) net6.0 104μs 596ns 4.42μs 0.443 0 0 32.18 KB
master RunWaf(args=NestedMap (20)) netcoreapp3.1 133μs 724ns 4.22μs 0.413 0 0 32.3 KB
master RunWaf(args=NestedMap (20)) net472 188μs 106ns 412ns 5.19 0.37 0 32.63 KB
master RunWafTwice(args=NestedMap (20)) net6.0 112μs 344ns 1.33μs 0.449 0 0 32.72 KB
master RunWafTwice(args=NestedMap (20)) netcoreapp3.1 133μs 82.5ns 309ns 0.398 0 0 32.82 KB
master RunWafTwice(args=NestedMap (20)) net472 194μs 136ns 527ns 5.25 0.389 0 33.19 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 168μs 63.2ns 228ns 0.559 0 0 38.53 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 203μs 597ns 2.31μs 0.484 0 0 38.6 KB
master RunWafWithAttack(args=Neste(...)tack) [22]) net472 255μs 537ns 2.08μs 6.12 0.5 0 39.2 KB
#5064 RunWaf(args=NestedMap (10)) net6.0 59.2μs 40.4ns 156ns 0.212 0 0 16.06 KB
#5064 RunWaf(args=NestedMap (10)) netcoreapp3.1 67.8μs 380ns 2.43μs 0.211 0 0 16.06 KB
#5064 RunWaf(args=NestedMap (10)) net472 99.6μs 427ns 1.66μs 2.55 0.0946 0 16.14 KB
#5064 RunWafTwice(args=NestedMap (10)) net6.0 56μs 80.6ns 312ns 0.222 0 0 16.6 KB
#5064 RunWafTwice(args=NestedMap (10)) netcoreapp3.1 72.2μs 364ns 1.63μs 0.208 0 0 16.58 KB
#5064 RunWafTwice(args=NestedMap (10)) net472 105μs 79.1ns 307ns 2.6 0.104 0 16.69 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 113μs 56.4ns 211ns 0.287 0 0 22.41 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 131μs 113ns 437ns 0.257 0 0 22.36 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [22]) net472 170μs 560ns 2.1μs 3.59 0.163 0 22.7 KB
#5064 RunWaf(args=NestedMap (100)) net6.0 104μs 572ns 3.52μs 0.452 0 0 32.76 KB
#5064 RunWaf(args=NestedMap (100)) netcoreapp3.1 136μs 224ns 868ns 0.408 0 0 33.33 KB
#5064 RunWaf(args=NestedMap (100)) net472 187μs 77.3ns 299ns 5.31 0.373 0 33.67 KB
#5064 RunWafTwice(args=NestedMap (100)) net6.0 100μs 63.4ns 246ns 0.424 0 0 33.3 KB
#5064 RunWafTwice(args=NestedMap (100)) netcoreapp3.1 141μs 674ns 2.78μs 0.461 0 0 33.86 KB
#5064 RunWafTwice(args=NestedMap (100)) net472 201μs 91.1ns 353ns 5.41 0.394 0 34.23 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [23]) net6.0 157μs 49.2ns 184ns 0.55 0 0 39.1 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [23]) netcoreapp3.1 204μs 350ns 1.35μs 0.474 0 0 39.63 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [23]) net472 263μs 1.31μs 5.55μs 6.39 0.511 0 40.23 KB
#5064 RunWaf(args=NestedMap (20)) net6.0 100μs 53.9ns 209ns 0.453 0 0 32.18 KB
#5064 RunWaf(args=NestedMap (20)) netcoreapp3.1 127μs 621ns 3.51μs 0.401 0 0 32.3 KB
#5064 RunWaf(args=NestedMap (20)) net472 186μs 144ns 537ns 5.17 0.369 0 32.63 KB
#5064 RunWafTwice(args=NestedMap (20)) net6.0 101μs 59.1ns 229ns 0.456 0 0 32.72 KB
#5064 RunWafTwice(args=NestedMap (20)) netcoreapp3.1 137μs 746ns 4.42μs 0.411 0 0 32.82 KB
#5064 RunWafTwice(args=NestedMap (20)) net472 194μs 63.9ns 230ns 5.26 0.389 0 33.19 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [22]) net6.0 165μs 863ns 4.23μs 0.505 0 0 38.53 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [22]) netcoreapp3.1 199μs 581ns 2.25μs 0.491 0 0 38.6 KB
#5064 RunWafWithAttack(args=Neste(...)tack) [22]) net472 254μs 197ns 761ns 6.17 0.504 0 39.2 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 173μs 187ns 723ns 0.26 0 0 18.25 KB
master SendRequest netcoreapp3.1 197μs 1.09μs 6.52μs 0.193 0 0 20.41 KB
master SendRequest net472 0.000465ns 0.000189ns 0.000733ns 0 0 0 0 b
#5064 SendRequest net6.0 173μs 135ns 505ns 0.172 0 0 18.25 KB
#5064 SendRequest netcoreapp3.1 195μs 352ns 1.36μs 0.194 0 0 20.41 KB
#5064 SendRequest net472 0.000285ns 0.000129ns 0.000483ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 547μs 1μs 3.88μs 0.534 0 0 41.85 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 657μs 951ns 3.68μs 0.334 0 0 41.71 KB
master WriteAndFlushEnrichedTraces net472 839μs 2.91μs 11.3μs 8.28 2.48 0.414 53.25 KB
#5064 WriteAndFlushEnrichedTraces net6.0 562μs 1.02μs 3.95μs 0.576 0 0 41.66 KB
#5064 WriteAndFlushEnrichedTraces netcoreapp3.1 640μs 2.3μs 8.9μs 0.332 0 0 41.71 KB
#5064 WriteAndFlushEnrichedTraces net472 836μs 2.83μs 11μs 8.28 2.48 0.414 53.26 KB
Benchmarks.Trace.DbCommandBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5064

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0 1.143 1,051.20 1,201.57

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.05μs 0.274ns 0.988ns 0.0106 0 0 768 B
master ExecuteNonQuery netcoreapp3.1 1.47μs 0.732ns 2.83ns 0.0103 0 0 768 B
master ExecuteNonQuery net472 1.8μs 0.71ns 2.56ns 0.116 0 0 730 B
#5064 ExecuteNonQuery net6.0 1.2μs 0.442ns 1.71ns 0.0109 0 0 768 B
#5064 ExecuteNonQuery netcoreapp3.1 1.45μs 0.405ns 1.46ns 0.0102 0 0 768 B
#5064 ExecuteNonQuery net472 1.79μs 0.97ns 3.63ns 0.116 0 0 730 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.21μs 1.18ns 4.56ns 0.013 0 0 936 B
master CallElasticsearch netcoreapp3.1 1.56μs 1.72ns 6.45ns 0.0126 0 0 936 B
master CallElasticsearch net472 2.5μs 0.735ns 2.85ns 0.151 0 0 955 B
master CallElasticsearchAsync net6.0 1.3μs 0.558ns 2.16ns 0.0124 0 0 912 B
master CallElasticsearchAsync netcoreapp3.1 1.54μs 0.486ns 1.88ns 0.0131 0 0 984 B
master CallElasticsearchAsync net472 2.77μs 1.15ns 4.44ns 0.16 0 0 1.01 KB
#5064 CallElasticsearch net6.0 1.24μs 0.728ns 2.82ns 0.013 0 0 936 B
#5064 CallElasticsearch netcoreapp3.1 1.57μs 0.717ns 2.68ns 0.0126 0 0 936 B
#5064 CallElasticsearch net472 2.61μs 0.735ns 2.75ns 0.151 0 0 955 B
#5064 CallElasticsearchAsync net6.0 1.3μs 0.362ns 1.35ns 0.0129 0 0 912 B
#5064 CallElasticsearchAsync netcoreapp3.1 1.66μs 0.419ns 1.51ns 0.0134 0 0 984 B
#5064 CallElasticsearchAsync net472 2.62μs 0.866ns 3.35ns 0.16 0 0 1.01 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.48μs 0.646ns 2.24ns 0.0126 0 0 912 B
master ExecuteAsync netcoreapp3.1 1.66μs 1.43ns 5.35ns 0.0124 0 0 912 B
master ExecuteAsync net472 1.83μs 0.483ns 1.81ns 0.139 0 0 875 B
#5064 ExecuteAsync net6.0 1.36μs 0.572ns 1.98ns 0.0126 0 0 912 B
#5064 ExecuteAsync netcoreapp3.1 1.69μs 1.02ns 3.94ns 0.0118 0 0 912 B
#5064 ExecuteAsync net472 1.83μs 0.751ns 2.81ns 0.138 0 0 875 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 4.29μs 7.03ns 25.3ns 0.0299 0 0 2.1 KB
master SendAsync netcoreapp3.1 5.06μs 2.05ns 7.68ns 0.0354 0 0 2.63 KB
master SendAsync net472 7.8μs 3ns 11.6ns 0.523 0 0 3.31 KB
#5064 SendAsync net6.0 4.41μs 4.28ns 16.6ns 0.0287 0 0 2.1 KB
#5064 SendAsync netcoreapp3.1 4.99μs 2.21ns 8.25ns 0.0349 0 0 2.63 KB
#5064 SendAsync net472 7.67μs 5.93ns 22.9ns 0.523 0 0 3.31 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5064

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 204.27 KB 211.98 KB 7.71 KB 3.78%

Fewer allocations 🎉 in #5064

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 61.93 KB 59.04 KB -2.89 KB -4.66%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 212.85 KB 202.7 KB -10.15 KB -4.77%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 59μs 1.16μs 11.3μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 54μs 244ns 1.17μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 37.6μs 77.7ns 269ns 0 0 0 61.93 KB
master StringConcatAspectBenchmark net6.0 276μs 6.83μs 66.5μs 0 0 0 212.85 KB
master StringConcatAspectBenchmark netcoreapp3.1 327μs 6.37μs 59.4μs 0 0 0 204.27 KB
master StringConcatAspectBenchmark net472 223μs 962ns 3.47μs 0 0 0 221.18 KB
#5064 StringConcatBenchmark net6.0 52.2μs 226ns 1.06μs 0 0 0 43.44 KB
#5064 StringConcatBenchmark netcoreapp3.1 61.1μs 777ns 7.61μs 0 0 0 42.64 KB
#5064 StringConcatBenchmark net472 38.7μs 191ns 788ns 0 0 0 59.04 KB
#5064 StringConcatAspectBenchmark net6.0 283μs 4.5μs 41.8μs 0 0 0 202.7 KB
#5064 StringConcatAspectBenchmark netcoreapp3.1 288μs 5.73μs 53.1μs 0 0 0 211.98 KB
#5064 StringConcatAspectBenchmark net472 260μs 6.28μs 60.9μs 0 0 0 221.18 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 1.59μs 0.62ns 2.32ns 0.0223 0 0 1.57 KB
master EnrichedLog netcoreapp3.1 2.2μs 1.64ns 6.14ns 0.0208 0 0 1.57 KB
master EnrichedLog net472 2.58μs 1.36ns 5.26ns 0.238 0 0 1.5 KB
#5064 EnrichedLog net6.0 1.48μs 0.655ns 2.54ns 0.022 0 0 1.57 KB
#5064 EnrichedLog netcoreapp3.1 2.23μs 1.04ns 3.91ns 0.0211 0 0 1.57 KB
#5064 EnrichedLog net472 2.67μs 3.37ns 13ns 0.238 0 0 1.5 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 113μs 130ns 487ns 0.0567 0 0 4.21 KB
master EnrichedLog netcoreapp3.1 119μs 108ns 418ns 0.059 0 0 4.21 KB
master EnrichedLog net472 149μs 119ns 447ns 0.668 0.223 0 4.39 KB
#5064 EnrichedLog net6.0 113μs 60.8ns 235ns 0.0567 0 0 4.21 KB
#5064 EnrichedLog netcoreapp3.1 118μs 102ns 383ns 0.0591 0 0 4.21 KB
#5064 EnrichedLog net472 148μs 80ns 310ns 0.663 0.221 0 4.39 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 2.99μs 0.867ns 3.36ns 0.0299 0 0 2.13 KB
master EnrichedLog netcoreapp3.1 4.28μs 3.46ns 13.4ns 0.0277 0 0 2.13 KB
master EnrichedLog net472 4.86μs 2.06ns 7.98ns 0.308 0 0 1.95 KB
#5064 EnrichedLog net6.0 3.02μs 2.33ns 9.02ns 0.0301 0 0 2.13 KB
#5064 EnrichedLog netcoreapp3.1 4.29μs 3.03ns 11.7ns 0.0274 0 0 2.13 KB
#5064 EnrichedLog net472 4.91μs 1.5ns 5.6ns 0.308 0 0 1.95 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.33μs 0.546ns 2.04ns 0.0153 0 0 1.1 KB
master SendReceive netcoreapp3.1 1.69μs 0.803ns 3.01ns 0.0147 0 0 1.1 KB
master SendReceive net472 2.2μs 3.74ns 14.5ns 0.177 0 0 1.12 KB
#5064 SendReceive net6.0 1.33μs 2.15ns 8.34ns 0.0152 0 0 1.1 KB
#5064 SendReceive netcoreapp3.1 1.75μs 0.85ns 3.29ns 0.0148 0 0 1.1 KB
#5064 SendReceive net472 2.11μs 4.34ns 16.8ns 0.177 0 0 1.12 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 2.72μs 0.774ns 2.9ns 0.0217 0 0 1.53 KB
master EnrichedLog netcoreapp3.1 3.84μs 1.8ns 6.47ns 0.0213 0 0 1.58 KB
master EnrichedLog net472 4.31μs 1.05ns 4.05ns 0.311 0 0 1.97 KB
#5064 EnrichedLog net6.0 2.76μs 0.954ns 3.69ns 0.022 0 0 1.53 KB
#5064 EnrichedLog netcoreapp3.1 3.89μs 1.71ns 6.61ns 0.0214 0 0 1.58 KB
#5064 EnrichedLog net472 4.31μs 1.72ns 6.66ns 0.311 0 0 1.97 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 462ns 0.13ns 0.504ns 0.00742 0 0 536 B
master StartFinishSpan netcoreapp3.1 753ns 0.697ns 2.61ns 0.00719 0 0 536 B
master StartFinishSpan net472 751ns 0.758ns 2.93ns 0.0854 0 0 538 B
master StartFinishScope net6.0 538ns 0.121ns 0.467ns 0.00905 0 0 656 B
master StartFinishScope netcoreapp3.1 926ns 0.686ns 2.66ns 0.00853 0 0 656 B
master StartFinishScope net472 949ns 0.37ns 1.28ns 0.098 0 0 618 B
#5064 StartFinishSpan net6.0 472ns 0.158ns 0.59ns 0.00746 0 0 536 B
#5064 StartFinishSpan netcoreapp3.1 718ns 0.585ns 2.26ns 0.0073 0 0 536 B
#5064 StartFinishSpan net472 724ns 0.594ns 2.3ns 0.0854 0 0 538 B
#5064 StartFinishScope net6.0 586ns 0.141ns 0.51ns 0.00917 0 0 656 B
#5064 StartFinishScope netcoreapp3.1 840ns 1.55ns 6.01ns 0.00881 0 0 656 B
#5064 StartFinishScope net472 909ns 1.02ns 3.95ns 0.098 0 0 618 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 726ns 0.26ns 1.01ns 0.00903 0 0 656 B
master RunOnMethodBegin netcoreapp3.1 1.01μs 0.786ns 2.84ns 0.00901 0 0 656 B
master RunOnMethodBegin net472 1.11μs 0.388ns 1.5ns 0.098 0 0 618 B
#5064 RunOnMethodBegin net6.0 708ns 0.396ns 1.53ns 0.00911 0 0 656 B
#5064 RunOnMethodBegin netcoreapp3.1 990ns 0.334ns 1.29ns 0.00898 0 0 656 B
#5064 RunOnMethodBegin net472 1.13μs 0.858ns 3.32ns 0.0982 0 0 618 B

@daniel-romano-DD daniel-romano-DD marked this pull request as ready for review January 16, 2024 19:10
@daniel-romano-DD daniel-romano-DD requested review from a team as code owners January 16, 2024 19:10
@daniel-romano-DD daniel-romano-DD marked this pull request as draft January 16, 2024 19:11
@daniel-romano-DD daniel-romano-DD marked this pull request as ready for review January 16, 2024 20:48
andrewlock
andrewlock approved these changes Jan 18, 2024
View reviewed changes
tracer/test/Datadog.Trace.Security.Unit.Tests/IAST/Tainted/TaintedObjectsTests.cs

TaintInput(false, string.Empty);

TaintInput(false, "0");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe add a couple of test cases with negative numbers and decimals - they will pass, but guards against future refactoring bugs 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

tracer/test/benchmarks/Benchmarks.Trace/Iast/StringAspectsBenchmark.cs Outdated
@@ -77,7 +77,7 @@ private static List<string> InitTaintedContext(int size, bool initTainted = true
return res;
}

const int Iterations = 100;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to include this change, it will invalidate the benchmarks I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted 👍

@daniel-romano-DD daniel-romano-DD force-pushed the dani/asm/small_string_cache_bugfix branch from 1e14a65 to 8b98d0a Compare January 18, 2024 11:17
@daniel-romano-DD daniel-romano-DD force-pushed the dani/asm/small_string_cache_bugfix branch from 5192ffd to df837b9 Compare January 18, 2024 15:37
tonyredondo
tonyredondo approved these changes Jan 18, 2024
View reviewed changes
Copy link
Member

@tonyredondo tonyredondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@robertpi robertpi merged commit ee340c7 into master Jan 22, 2024
@robertpi robertpi deleted the dani/asm/small_string_cache_bugfix branch January 22, 2024 18:19
@github-actions github-actions bot added this to the vNext milestone Jan 22, 2024
@daniel-romano-DD daniel-romano-DD changed the title [IAST] Dani/asm/small string cache bugfix [IAST] Small string cache bugfix May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonyredondo tonyredondo tonyredondo approved these changes

@andrewlock andrewlock andrewlock approved these changes

@NachoEchevarria NachoEchevarria NachoEchevarria approved these changes

Assignees
No one assigned
Labels
area:asm area:asm-iast type:bug
Projects
None yet
Milestone
2.46.0
Development

Successfully merging this pull request may close these issues.
5 participants
@daniel-romano-DD @andrewlock @tonyredondo @NachoEchevarria @robertpi