Skip to content
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

[IAST] Add Stack trace to vuln location #5997

Merged
merged 10 commits into from
Sep 13, 2024
Merged

[IAST] Add Stack trace to vuln location #5997

merged 10 commits into from
Sep 13, 2024

Conversation

daniel-romano-DD
Copy link
Contributor

@daniel-romano-DD daniel-romano-DD commented Sep 5, 2024 •
edited
Loading

Summary of changes

Add full stack trace to vulnerability location following this RFC

Reason for change

A stack trace of where the vulnerability is located can help the client to locate it in order to fix it.

Implementation details

The stacks are added in the MetaStruct _dd.stack tag under the vulnerability section. A stack Id has been added to the vulnerability location. This Id matches the stack Id on the MetaStruct dictionary.

Test coverage

Added integration tests

Other details

@andrewlock
Copy link
Member

andrewlock commented Sep 5, 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 (5997) - mean (70ms)  : 67, 72
     .   : milestone, 70,
    master - mean (69ms)  : 67, 71
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (5997) - mean (1,111ms)  : 1090, 1132
     .   : milestone, 1111,
    master - mean (1,118ms)  : 1090, 1147
     .   : milestone, 1118,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5997) - mean (109ms)  : 104, 113
     .   : milestone, 109,
    master - mean (109ms)  : 105, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (5997) - mean (812ms)  : 794, 830
     .   : milestone, 812,
    master - mean (809ms)  : 789, 829
     .   : milestone, 809,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5997) - mean (92ms)  : 89, 95
     .   : milestone, 92,
    master - mean (92ms)  : 89, 95
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (5997) - mean (771ms)  : 749, 793
     .   : milestone, 771,
    master - mean (768ms)  : 748, 788
     .   : milestone, 768,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5997) - mean (192ms)  : 187, 198
     .   : milestone, 192,
    master - mean (190ms)  : 187, 193
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (5997) - mean (1,195ms)  : 1168, 1222
     .   : milestone, 1195,
    master - mean (1,197ms)  : 1169, 1224
     .   : milestone, 1197,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5997) - mean (277ms)  : 272, 281
     .   : milestone, 277,
    master - mean (276ms)  : 272, 281
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (5997) - mean (973ms)  : 953, 993
     .   : milestone, 973,
    master - mean (971ms)  : 951, 992
     .   : milestone, 971,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5997) - mean (264ms)  : 261, 268
     .   : milestone, 264,
    master - mean (265ms)  : 261, 270
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (5997) - mean (951ms)  : 921, 981
     .   : milestone, 951,
    master - mean (952ms)  : 931, 973
     .   : milestone, 952,
Loading

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Sep 5, 2024 •
edited
Loading

Datadog Report

Branch report: dani/iast/stack_traces_location
Commit report: 120d53b
Test service: dd-trace-dotnet

✅ 0 Failed, 471080 Passed, 3258 Skipped, 31h 42m 13.18s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

  • SnapshotBiggerThanMaxSize_TwoLevel_OneSliced - Datadog.Trace.Tests.Debugger.SnapshotSlicerTests - Last Failure

    Expand for error 
     Unexpected end of content while loading JObject. Path 'debugger', line 1, position 7755.

@andrewlock
Copy link
Member

andrewlock commented Sep 5, 2024 •
edited
Loading

Benchmarks Report for appsec 🐌

Benchmarks for #5997 compared to master:

  • All benchmarks have the same speed
  • 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.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 70.4μs 109ns 422ns 0.0703 0 0 6.01 KB
master AllCycleSimpleBody netcoreapp3.1 62.7μs 96.8ns 349ns 0.0943 0 0 6.95 KB
master AllCycleSimpleBody net472 48.9μs 39.4ns 148ns 1.31 0 0 8.34 KB
master AllCycleMoreComplexBody net6.0 76.8μs 111ns 429ns 0.115 0 0 9.51 KB
master AllCycleMoreComplexBody netcoreapp3.1 69.4μs 90.4ns 338ns 0.139 0 0 10.37 KB
master AllCycleMoreComplexBody net472 56.3μs 58.9ns 228ns 1.88 0.028 0 11.85 KB
master ObjectExtractorSimpleBody net6.0 141ns 0.322ns 1.25ns 0.00396 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 197ns 0.326ns 1.26ns 0.00367 0 0 272 B
master ObjectExtractorSimpleBody net472 171ns 0.25ns 0.902ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 3.01μs 2.3ns 8.9ns 0.0533 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 3.97μs 5.43ns 21ns 0.0497 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 3.87μs 3.85ns 13.9ns 0.602 0.00584 0 3.8 KB
#5997 AllCycleSimpleBody net6.0 73.5μs 68.1ns 264ns 0.0732 0 0 6 KB
#5997 AllCycleSimpleBody netcoreapp3.1 62.5μs 60.6ns 235ns 0.0925 0 0 6.95 KB
#5997 AllCycleSimpleBody net472 47.8μs 60.9ns 228ns 1.31 0 0 8.34 KB
#5997 AllCycleMoreComplexBody net6.0 78.8μs 102ns 394ns 0.118 0 0 9.51 KB
#5997 AllCycleMoreComplexBody netcoreapp3.1 69.6μs 80.6ns 279ns 0.138 0 0 10.37 KB
#5997 AllCycleMoreComplexBody net472 54.9μs 63.7ns 247ns 1.86 0.0273 0 11.85 KB
#5997 ObjectExtractorSimpleBody net6.0 144ns 0.716ns 2.95ns 0.00396 0 0 280 B
#5997 ObjectExtractorSimpleBody netcoreapp3.1 197ns 0.279ns 1.08ns 0.0037 0 0 272 B
#5997 ObjectExtractorSimpleBody net472 171ns 0.103ns 0.401ns 0.0446 0 0 281 B
#5997 ObjectExtractorMoreComplexBody net6.0 3.15μs 1.95ns 6.76ns 0.0538 0 0 3.78 KB
#5997 ObjectExtractorMoreComplexBody netcoreapp3.1 4.01μs 4.44ns 16.6ns 0.0498 0 0 3.69 KB
#5997 ObjectExtractorMoreComplexBody net472 3.88μs 2.63ns 10.2ns 0.603 0.0058 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 37.4μs 15.2ns 54.9ns 0.445 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 54.3μs 18.3ns 70.8ns 0.433 0 0 32.4 KB
master EncodeArgs net472 68μs 85.9ns 333ns 5.14 0.0676 0 32.5 KB
master EncodeLegacyArgs net6.0 72μs 27.2ns 105ns 0 0 0 2.14 KB
master EncodeLegacyArgs netcoreapp3.1 105μs 265ns 1.03μs 0 0 0 2.14 KB
master EncodeLegacyArgs net472 155μs 129ns 498ns 0.309 0 0 2.15 KB
#5997 EncodeArgs net6.0 38μs 23.3ns 84ns 0.461 0 0 32.4 KB
#5997 EncodeArgs netcoreapp3.1 54.3μs 27.9ns 101ns 0.431 0 0 32.4 KB
#5997 EncodeArgs net472 67μs 144ns 556ns 5.15 0.0664 0 32.5 KB
#5997 EncodeLegacyArgs net6.0 74.3μs 25ns 96.7ns 0 0 0 2.14 KB
#5997 EncodeLegacyArgs netcoreapp3.1 105μs 311ns 1.2μs 0 0 0 2.14 KB
#5997 EncodeLegacyArgs net472 151μs 75.1ns 291ns 0.302 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 186μs 188ns 729ns 0 0 0 2.42 KB
master RunWafRealisticBenchmark netcoreapp3.1 199μs 236ns 913ns 0 0 0 2.37 KB
master RunWafRealisticBenchmark net472 210μs 195ns 728ns 0.315 0 0 2.43 KB
master RunWafRealisticBenchmarkWithAttack net6.0 123μs 104ns 403ns 0 0 0 1.46 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 129μs 69.4ns 269ns 0 0 0 1.45 KB
master RunWafRealisticBenchmarkWithAttack net472 140μs 101ns 378ns 0.21 0 0 1.48 KB
#5997 RunWafRealisticBenchmark net6.0 185μs 314ns 1.22μs 0 0 0 2.42 KB
#5997 RunWafRealisticBenchmark netcoreapp3.1 196μs 172ns 645ns 0 0 0 2.37 KB
#5997 RunWafRealisticBenchmark net472 208μs 81.3ns 293ns 0.312 0 0 2.43 KB
#5997 RunWafRealisticBenchmarkWithAttack net6.0 122μs 58.6ns 227ns 0 0 0 1.46 KB
#5997 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 130μs 180ns 697ns 0 0 0 1.45 KB
#5997 RunWafRealisticBenchmarkWithAttack net472 140μs 222ns 859ns 0.208 0 0 1.48 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5997

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 59.09 KB 59.74 KB 656 B 1.11%

Fewer allocations 🎉 in #5997

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 254.74 KB 253.42 KB -1.32 KB -0.52%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 255.86 KB 254.33 KB -1.54 KB -0.60%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 64μs 776ns 7.68μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 53.3μs 290ns 1.64μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 40μs 118ns 427ns 0 0 0 59.09 KB
master StringConcatAspectBenchmark net6.0 327μs 1.85μs 12μs 0 0 0 255.86 KB
master StringConcatAspectBenchmark netcoreapp3.1 360μs 1.93μs 10.9μs 0 0 0 254.74 KB
master StringConcatAspectBenchmark net472 288μs 5.7μs 54.4μs 0 0 0 278.53 KB
#5997 StringConcatBenchmark net6.0 60.8μs 594ns 5.7μs 0 0 0 43.44 KB
#5997 StringConcatBenchmark netcoreapp3.1 60.4μs 777ns 7.5μs 0 0 0 42.64 KB
#5997 StringConcatBenchmark net472 37.3μs 101ns 380ns 0 0 0 59.74 KB
#5997 StringConcatAspectBenchmark net6.0 308μs 1.76μs 12.8μs 0 0 0 254.33 KB
#5997 StringConcatAspectBenchmark netcoreapp3.1 334μs 1.42μs 4.91μs 0 0 0 253.42 KB
#5997 StringConcatAspectBenchmark net472 283μs 6.08μs 59.9μs 0 0 0 278.53 KB

@andrewlock
Copy link
Member

andrewlock commented Sep 5, 2024 •
edited
Loading

Benchmarks Report for tracer 🐌

Benchmarks for #5997 compared to master:

  • 2 benchmarks are slower, with geometric mean 1.170
  • All benchmarks have the same 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 7.78μs 44.5ns 321ns 0.0155 0.00776 0 5.43 KB
master StartStopWithChild netcoreapp3.1 9.61μs 51.4ns 272ns 0.0194 0.0097 0 5.62 KB
master StartStopWithChild net472 16μs 47.3ns 183ns 1.01 0.29 0.102 6.06 KB
#5997 StartStopWithChild net6.0 7.73μs 37.6ns 222ns 0.0155 0.00776 0 5.43 KB
#5997 StartStopWithChild netcoreapp3.1 10.1μs 50.5ns 231ns 0.0204 0.00509 0 5.62 KB
#5997 StartStopWithChild net472 16.3μs 64.1ns 248ns 1 0.293 0.087 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 468μs 172ns 644ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 629μs 237ns 854ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 824μs 253ns 979ns 0.417 0 0 3.3 KB
#5997 WriteAndFlushEnrichedTraces net6.0 478μs 258ns 966ns 0 0 0 2.7 KB
#5997 WriteAndFlushEnrichedTraces netcoreapp3.1 632μs 332ns 1.29μs 0 0 0 2.7 KB
#5997 WriteAndFlushEnrichedTraces net472 831μs 394ns 1.47μs 0.417 0 0 3.3 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 197μs 1.13μs 8.27μs 0.211 0 0 18.45 KB
master SendRequest netcoreapp3.1 218μs 1.21μs 7.35μs 0.212 0 0 20.61 KB
master SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#5997 SendRequest net6.0 203μs 1.18μs 11.1μs 0.193 0 0 18.45 KB
#5997 SendRequest netcoreapp3.1 221μs 1.23μs 7.6μs 0.213 0 0 20.61 KB
#5997 SendRequest net472 0.00088ns 0.000457ns 0.00177ns 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 588μs 3.27μs 19.3μs 0.571 0 0 41.75 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 699μs 3.56μs 15.9μs 0.342 0 0 41.87 KB
master WriteAndFlushEnrichedTraces net472 885μs 4.34μs 17.4μs 8.3 2.62 0.437 53.32 KB
#5997 WriteAndFlushEnrichedTraces net6.0 567μs 2.83μs 12.3μs 0.571 0 0 41.66 KB
#5997 WriteAndFlushEnrichedTraces netcoreapp3.1 678μs 1.61μs 5.58μs 0.336 0 0 41.91 KB
#5997 WriteAndFlushEnrichedTraces net472 906μs 4.48μs 21.9μs 8.3 2.62 0.437 53.32 KB
Benchmarks.Trace.DbCommandBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5997

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0 1.124 1,227.81 1,380.09

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.23μs 1.54ns 5.95ns 0.0141 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.8μs 3.03ns 11.7ns 0.0142 0 0 1.02 KB
master ExecuteNonQuery net472 2.09μs 1.77ns 6.86ns 0.156 0 0 987 B
#5997 ExecuteNonQuery net6.0 1.38μs 1.38ns 5.18ns 0.0145 0 0 1.02 KB
#5997 ExecuteNonQuery netcoreapp3.1 1.72μs 1.12ns 4.34ns 0.0137 0 0 1.02 KB
#5997 ExecuteNonQuery net472 2.19μs 1.34ns 5ns 0.156 0 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.32μs 0.602ns 2.33ns 0.0138 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.55μs 1.31ns 5.09ns 0.0133 0 0 976 B
master CallElasticsearch net472 2.48μs 2.74ns 10.6ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.32μs 0.579ns 2.24ns 0.0131 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.62μs 1.77ns 6.64ns 0.0139 0 0 1.02 KB
master CallElasticsearchAsync net472 2.51μs 1.22ns 4.74ns 0.166 0 0 1.05 KB
#5997 CallElasticsearch net6.0 1.21μs 0.288ns 1.08ns 0.0139 0 0 976 B
#5997 CallElasticsearch netcoreapp3.1 1.58μs 1.21ns 4.53ns 0.0127 0 0 976 B
#5997 CallElasticsearch net472 2.57μs 1.14ns 4.26ns 0.157 0 0 995 B
#5997 CallElasticsearchAsync net6.0 1.39μs 1.78ns 6.66ns 0.0132 0 0 952 B
#5997 CallElasticsearchAsync netcoreapp3.1 1.68μs 1.67ns 6.25ns 0.0142 0 0 1.02 KB
#5997 CallElasticsearchAsync net472 2.57μs 1.86ns 6.97ns 0.167 0 0 1.05 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.26μs 0.998ns 3.86ns 0.0133 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.64μs 0.701ns 2.62ns 0.0124 0 0 952 B
master ExecuteAsync net472 1.79μs 0.83ns 3.1ns 0.145 0 0 915 B
#5997 ExecuteAsync net6.0 1.26μs 0.582ns 2.18ns 0.0133 0 0 952 B
#5997 ExecuteAsync netcoreapp3.1 1.66μs 3.95ns 15.3ns 0.0131 0 0 952 B
#5997 ExecuteAsync net472 1.74μs 0.977ns 3.78ns 0.145 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 4.26μs 2ns 7.47ns 0.0299 0 0 2.22 KB
master SendAsync netcoreapp3.1 4.97μs 1.76ns 6.82ns 0.0348 0 0 2.76 KB
master SendAsync net472 7.67μs 2.62ns 10.2ns 0.499 0 0 3.15 KB
#5997 SendAsync net6.0 4.18μs 1.13ns 4.24ns 0.0296 0 0 2.22 KB
#5997 SendAsync netcoreapp3.1 5.16μs 2.62ns 10.2ns 0.0362 0 0 2.76 KB
#5997 SendAsync net472 7.79μs 1.03ns 3.87ns 0.5 0 0 3.15 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.55μs 0.622ns 2.33ns 0.0233 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.2μs 0.491ns 1.7ns 0.022 0 0 1.64 KB
master EnrichedLog net472 2.44μs 0.935ns 3.5ns 0.249 0 0 1.57 KB
#5997 EnrichedLog net6.0 1.45μs 0.705ns 2.64ns 0.0232 0 0 1.64 KB
#5997 EnrichedLog netcoreapp3.1 2.34μs 1.23ns 4.43ns 0.022 0 0 1.64 KB
#5997 EnrichedLog net472 2.56μs 0.536ns 1.93ns 0.249 0 0 1.57 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 114μs 195ns 755ns 0.057 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 119μs 211ns 816ns 0.0589 0 0 4.28 KB
master EnrichedLog net472 146μs 164ns 635ns 0.655 0.218 0 4.46 KB
#5997 EnrichedLog net6.0 115μs 75ns 280ns 0.057 0 0 4.28 KB
#5997 EnrichedLog netcoreapp3.1 117μs 141ns 547ns 0.0582 0 0 4.28 KB
#5997 EnrichedLog net472 145μs 171ns 663ns 0.651 0.217 0 4.46 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 3.18μs 1.06ns 4.11ns 0.0303 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.25μs 6.55ns 25.4ns 0.0282 0 0 2.2 KB
master EnrichedLog net472 4.86μs 5.54ns 21.5ns 0.32 0 0 2.02 KB
#5997 EnrichedLog net6.0 3.07μs 0.964ns 3.73ns 0.0306 0 0 2.2 KB
#5997 EnrichedLog netcoreapp3.1 4.22μs 1.86ns 7.22ns 0.0296 0 0 2.2 KB
#5997 EnrichedLog net472 4.84μs 2.88ns 10.8ns 0.319 0 0 2.02 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.4μs 1.52ns 5.87ns 0.0159 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.71μs 1.48ns 5.71ns 0.0152 0 0 1.14 KB
master SendReceive net472 2.04μs 0.914ns 3.54ns 0.183 0.00102 0 1.16 KB
#5997 SendReceive net6.0 1.31μs 0.876ns 3.39ns 0.0158 0 0 1.14 KB
#5997 SendReceive netcoreapp3.1 1.81μs 0.553ns 2.14ns 0.0153 0 0 1.14 KB
#5997 SendReceive net472 2.02μs 0.843ns 3.27ns 0.183 0 0 1.16 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.71μs 1.08ns 4.2ns 0.0217 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.92μs 1.49ns 5.16ns 0.0215 0 0 1.65 KB
master EnrichedLog net472 4.36μs 1.68ns 6.3ns 0.322 0 0 2.04 KB
#5997 EnrichedLog net6.0 2.74μs 0.744ns 2.88ns 0.0221 0 0 1.6 KB
#5997 EnrichedLog netcoreapp3.1 3.76μs 1.51ns 5.85ns 0.0207 0 0 1.65 KB
#5997 EnrichedLog net472 4.29μs 1.03ns 3.72ns 0.323 0 0 2.04 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 392ns 0.281ns 1.09ns 0.00807 0 0 576 B
master StartFinishSpan netcoreapp3.1 605ns 3.16ns 14.8ns 0.00796 0 0 576 B
master StartFinishSpan net472 640ns 0.631ns 2.44ns 0.0918 0 0 578 B
master StartFinishScope net6.0 504ns 0.414ns 1.6ns 0.00987 0 0 696 B
master StartFinishScope netcoreapp3.1 664ns 0.564ns 2.18ns 0.00962 0 0 696 B
master StartFinishScope net472 840ns 0.7ns 2.71ns 0.105 0 0 658 B
#5997 StartFinishSpan net6.0 398ns 0.295ns 1.14ns 0.00803 0 0 576 B
#5997 StartFinishSpan netcoreapp3.1 554ns 0.581ns 2.17ns 0.00759 0 0 576 B
#5997 StartFinishSpan net472 657ns 0.945ns 3.66ns 0.0917 0 0 578 B
#5997 StartFinishScope net6.0 556ns 0.589ns 2.28ns 0.00984 0 0 696 B
#5997 StartFinishScope netcoreapp3.1 663ns 0.747ns 2.69ns 0.00965 0 0 696 B
#5997 StartFinishScope net472 879ns 0.938ns 3.38ns 0.105 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5997

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.217 593.34 722.06

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 593ns 0.231ns 0.893ns 0.00983 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 886ns 0.639ns 2.39ns 0.00942 0 0 696 B
master RunOnMethodBegin net472 1.15μs 1.02ns 3.93ns 0.104 0 0 658 B
#5997 RunOnMethodBegin net6.0 722ns 0.601ns 2.33ns 0.00969 0 0 696 B
#5997 RunOnMethodBegin netcoreapp3.1 953ns 0.878ns 3.4ns 0.00955 0 0 696 B
#5997 RunOnMethodBegin net472 1.03μs 1.06ns 4.12ns 0.105 0 0 658 B

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 6, 2024 •
edited
Loading

Snapshots difference summary

The following differences have been observed in committed snapshots. It is meant to help the reviewer.
The diff is simplistic, so please check some files anyway while we improve it.

1 occurrences of :

+      _dd.iast.json:
[...]
+  "vulnerabilities": [
+    {
+      "type": "STACKTRACE_LEAK",
+      "hash": 1099366274,
+      "location": {
+        "spanId": XXX,
+        "path": "Samples.Security.AspNetCore5.Controllers.IastController",
+        "method": "StackTraceLeak"
+      },
+      "evidence": {
+        "value": "Samples.Security.AspNetCore5,SystemException"
+      }
+    }
+  ]
[...]
+      _dd.iast.telemetry.executed.sink.header_injection: 1.0,
+      _dd.iast.telemetry.executed.sink.hsts_header_missing: 1.0,
+      _dd.iast.telemetry.executed.sink.stacktrace_leak: 1.0,
+      _dd.iast.telemetry.executed.sink.unvalidated_redirect: 1.0,
+      _dd.iast.telemetry.executed.sink.xcontenttype_header_missing: 1.0,

1 occurrences of :

-      _sampling_priority_v1: 1.0
+      _sampling_priority_v1: 2.0

26 occurrences of :

+      appsec: ,

311 occurrences of :

+    },
+    MetaStruct: {
+      appsec:

23 occurrences of :

+        "stackId": "1"

5 occurrences of :

+    },
+    MetaStruct: {
+      _dd.stack:

6 occurrences of :

+    },
+    MetaStruct: {
+      appsec: ,
+      _dd.stack:

3 occurrences of :

+        "stackId": "2"

@andrewlock
Copy link
Member

andrewlock commented Sep 6, 2024 •
edited
Loading

Throughput/Crank Report ⚡

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 (5997) (11.207M)   : 0, 11207456
    master (11.191M)   : 0, 11191009
    benchmarks/2.9.0 (11.235M)   : 0, 11234930

    section Automatic
    This PR (5997) (7.446M)   : 0, 7445598
    master (7.480M)   : 0, 7479864
    benchmarks/2.9.0 (7.902M)   : 0, 7901974

    section Trace stats
    master (7.854M)   : 0, 7854112

    section Manual
    master (11.251M)   : 0, 11250786

    section Manual + Automatic
    This PR (5997) (6.886M)   : 0, 6886230
    master (6.974M)   : 0, 6974322

    section DD_TRACE_ENABLED=0
    master (10.260M)   : 0, 10260342
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5997) (9.633M)   : 0, 9632921
    master (9.407M)   : 0, 9407221
    benchmarks/2.9.0 (9.468M)   : 0, 9467847

    section Automatic
    This PR (5997) (6.581M)   : 0, 6580748
    master (6.409M)   : 0, 6408651

    section Trace stats
    master (6.945M)   : 0, 6945033

    section Manual
    master (9.485M)   : 0, 9484960

    section Manual + Automatic
    This PR (5997) (6.082M)   : 0, 6082118
    master (6.153M)   : 0, 6152889

    section DD_TRACE_ENABLED=0
    master (8.880M)   : 0, 8879609
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5997) (10.246M)   : 0, 10245694
    master (10.042M)   : 0, 10041833
    benchmarks/2.9.0 (10.191M)   : 0, 10191280

    section Automatic
    This PR (5997) (6.743M)   : 0, 6742808
    master (6.584M)   : 0, 6583747
    benchmarks/2.9.0 (7.389M)   : 0, 7388665

    section Trace stats
    master (7.349M)   : 0, 7348894

    section Manual
    master (10.065M)   : 0, 10064803

    section Manual + Automatic
    This PR (5997) (6.280M)   : 0, 6280349
    master (6.022M)   : 0, 6022361

    section DD_TRACE_ENABLED=0
    master (9.438M)   : 0, 9437949
Loading

@daniel-romano-DD daniel-romano-DD force-pushed the dani/iast/stack_traces_location branch 2 times, most recently from bbc8c49 to 4115f09 Compare September 11, 2024 21:43
@daniel-romano-DD daniel-romano-DD marked this pull request as ready for review September 11, 2024 21:43
e-n-0
e-n-0 approved these changes Sep 12, 2024
View reviewed changes
Copy link
Member

@e-n-0 e-n-0 left a comment

Choose a reason for hiding this comment

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

Nice work 😄 LGTM

@daniel-romano-DD
Initial tests
a70efef 
Snapshots update

Fixed snapshots

Snapshots
@daniel-romano-DD
Vuln stack implementation and tests
3db4974 
Fix test compilation
@daniel-romano-DD
Removed unneeded stack id when stack reporting is disabled
d2f5ba0
@daniel-romano-DD
Fixed test
0509650 
Fixed test

Fixed tests

Fixed snapshot

Fix tests

Fixed mvc5 tests

Fixed test

Fix netfw snapshots

Fix AspNetCore2 tests

Snapshots fixed

Fixed missing snapshot

Fixed snapshots
@daniel-romano-DD
Fixed flaky test
7da3788 
Snapshot fixed
andrewlock
andrewlock reviewed Sep 13, 2024
View reviewed changes
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

I haven't reviewed all the tests, but there's a couple of things I think we should tweak, mostly around moving ASM/IAST-specific calls out of TraceContext, and handling null reference types more cleanly

tracer/src/Datadog.Trace/TraceContext.cs
{
if (Volatile.Read(ref _appSecRequestContext) is null)
{
Interlocked.CompareExchange(ref _appSecRequestContext, new(), null);
}

_appSecRequestContext!.AddWafSecurityEvents(events);
return _appSecRequestContext!;
Copy link
Member

Choose a reason for hiding this comment

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

Wait, why do we need the ! 🤔 Is it just different TFMs causing the issue, or is there a flow control issue. AFAICT, it should be guaranteed non null, because Interlocked.CompareExchange ensures that?

Copy link
Member

Choose a reason for hiding this comment

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

As discussed, it's an early TFM issue, fine to ignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, without it it does not compile...

tracer/src/Datadog.Trace/TraceContext.cs Show resolved Hide resolved
tracer/src/Datadog.Trace/TraceContext.cs Show resolved Hide resolved
tracer/src/Datadog.Trace/Iast/IastRequestContext.cs Outdated Show resolved Hide resolved
tracer/src/Datadog.Trace/Iast/Location.cs Outdated Show resolved Hide resolved
tracer/src/Datadog.Trace/Iast/StackWalker.cs Outdated Show resolved Hide resolved
tracer/src/Datadog.Trace/Iast/StackWalker.cs Outdated Show resolved Hide resolved
tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs Outdated Show resolved Hide resolved
tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs Show resolved Hide resolved
andrewlock
andrewlock approved these changes Sep 13, 2024
View reviewed changes
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM once it's all passing 🙂

@daniel-romano-DD daniel-romano-DD merged commit e716a76 into master Sep 13, 2024
70 of 73 checks passed
@daniel-romano-DD daniel-romano-DD deleted the dani/iast/stack_traces_location branch September 13, 2024 18:38
@github-actions github-actions bot added this to the vNext-v3 milestone Sep 13, 2024
@daniel-romano-DD daniel-romano-DD mentioned this pull request Sep 16, 2024
Closed
@daniel-romano-DD daniel-romano-DD mentioned this pull request Oct 4, 2024
Merged
daniel-romano-DD added a commit that referenced this pull request Oct 9, 2024
@daniel-romano-DD
[ASM] Segregate asm and iast contexts functionality (#6118)
8b345de 
## Summary of changes
As suggested in [this PR
comment](#5997 (comment))
by @andrewlock , it is better to remove ASM and IAST specific
functionality from the `TraceContext `to theis respective contexts.

## Reason for change
Code cleanup

## Implementation details

## 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. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewlock andrewlock andrewlock approved these changes

@e-n-0 e-n-0 e-n-0 approved these changes

Assignees
No one assigned
Labels
area:asm-iast
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@daniel-romano-DD @andrewlock @e-n-0