Skip to content

[ASM] Fix exception when accessing ReportedExternalWafsRequestHeaders #6030

Merged
NachoEchevarria merged 4 commits into
masterfrom
nacho/ReportedExternalWafsRequestHeaders
Oct 14, 2024
Merged

[ASM] Fix exception when accessing ReportedExternalWafsRequestHeaders #6030
NachoEchevarria merged 4 commits into
masterfrom
nacho/ReportedExternalWafsRequestHeaders

Conversation

@NachoEchevarria

@NachoEchevarria NachoEchevarria commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary of changes

This PR is closely related to this one

In this case, the error is thrown when accessing the key ReportedExternalWafsRequestHeaders

No other httpRequest.Items problematic accesses have been found.

This error can happen only in netcore versions of the framework. When we access httpcontext.Items, the exception is thrown if the key is not found. httpcontext.Items is defined as a IDictionary<object, object>. Usually, it will be a Microsoft.AspNetCore.Http.ItemsDictionary, which does not throw an exception when trying to retrieve a key that is not stored, but other custom implementations such as Dictionary<object, object> will throw it. It seems that in one customer, we are receiving a context in which Items is a Dictionary.

This might be due to custom middlewares, third party extensions, use of custom http contexts, etc.

Reason for change

Implementation details

Test coverage

Other details

@andrewlock

andrewlock commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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 (6030) - mean (70ms)  : 68, 72
     .   : milestone, 70,
    master - mean (71ms)  : 68, 74
     .   : milestone, 71,

    section CallTarget+Inlining+NGEN
    This PR (6030) - mean (1,115ms)  : 1042, 1188
     .   : milestone, 1115,
    master - mean (1,109ms)  : 1084, 1134
     .   : milestone, 1109,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6030) - mean (110ms)  : 107, 113
     .   : milestone, 110,
    master - mean (109ms)  : 107, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (6030) - mean (785ms)  : 739, 830
     .   : milestone, 785,
    master - mean (777ms)  : 762, 792
     .   : milestone, 777,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6030) - mean (93ms)  : 91, 95
     .   : milestone, 93,
    master - mean (93ms)  : 92, 95
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (6030) - mean (781ms)  : 562, 1000
     .   : milestone, 781,
    master - mean (731ms)  : 716, 747
     .   : milestone, 731,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6030) - mean (190ms)  : 186, 194
     .   : milestone, 190,
    master - mean (191ms)  : 188, 194
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6030) - mean (1,198ms)  : 1171, 1225
     .   : milestone, 1198,
    master - mean (1,199ms)  : 1171, 1227
     .   : milestone, 1199,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6030) - mean (276ms)  : 271, 280
     .   : milestone, 276,
    master - mean (276ms)  : 271, 281
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (6030) - mean (940ms)  : 922, 958
     .   : milestone, 940,
    master - mean (939ms)  : 921, 957
     .   : milestone, 939,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6030) - mean (264ms)  : 261, 268
     .   : milestone, 264,
    master - mean (265ms)  : 261, 269
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6030) - mean (928ms)  : 901, 954
     .   : milestone, 928,
    master - mean (927ms)  : 908, 947
     .   : milestone, 927,
Loading

@datadog-ddstaging

datadog-ddstaging Bot commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: nacho/ReportedExternalWafsRequestHeaders
Commit report: db69852
Test service: dd-trace-dotnet

✅ 0 Failed, 365281 Passed, 2076 Skipped, 16h 37m 44.43s Total Time

@NachoEchevarria NachoEchevarria changed the title Fix [ASM] Fix exception when accessing ReportedExternalWafsRequestHeaders Sep 13, 2024
@andrewlock

andrewlock commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Benchmarks Report for tracer 🐌

Benchmarks for #6030 compared to master:

  • 3 benchmarks are faster, with geometric mean 1.162
  • 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 7.76μs 41.8ns 217ns 0.0202 0.00807 0 5.42 KB
master StartStopWithChild netcoreapp3.1 9.93μs 56.1ns 372ns 0.0187 0.00933 0 5.61 KB
master StartStopWithChild net472 16.3μs 64.5ns 250ns 1.03 0.312 0.0903 6.08 KB
#6030 StartStopWithChild net6.0 7.94μs 45.5ns 331ns 0.0157 0.00786 0 5.43 KB
#6030 StartStopWithChild netcoreapp3.1 10.1μs 57ns 378ns 0.0247 0.00987 0 5.62 KB
#6030 StartStopWithChild net472 16.6μs 76.4ns 305ns 1.01 0.286 0.09 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 467μs 219ns 849ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 650μs 229ns 857ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 835μs 452ns 1.75μs 0.414 0 0 3.3 KB
#6030 WriteAndFlushEnrichedTraces net6.0 469μs 491ns 1.9μs 0 0 0 2.7 KB
#6030 WriteAndFlushEnrichedTraces netcoreapp3.1 661μs 225ns 869ns 0 0 0 2.7 KB
#6030 WriteAndFlushEnrichedTraces net472 836μs 327ns 1.18μ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 193μs 1.04μs 5.82μs 0.191 0 0 18.45 KB
master SendRequest netcoreapp3.1 213μs 854ns 3.19μs 0.207 0 0 20.61 KB
master SendRequest net472 0.00144ns 0.000398ns 0.00154ns 0 0 0 0 b
#6030 SendRequest net6.0 192μs 1.07μs 6.57μs 0.181 0 0 18.45 KB
#6030 SendRequest netcoreapp3.1 210μs 1.18μs 7.35μs 0.229 0 0 20.61 KB
#6030 SendRequest net472 0.000306ns 0.000178ns 0.000664ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6030

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 41.44 KB 41.78 KB 334 B 0.81%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 566μs 2.05μs 7.68μs 0.563 0 0 41.68 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 676μs 3.41μs 19μs 0.332 0 0 41.44 KB
master WriteAndFlushEnrichedTraces net472 852μs 2.91μs 10.5μs 8.08 2.55 0.425 53.32 KB
#6030 WriteAndFlushEnrichedTraces net6.0 553μs 3.01μs 17.8μs 0.581 0 0 41.64 KB
#6030 WriteAndFlushEnrichedTraces netcoreapp3.1 662μs 1.95μs 7.54μs 0.322 0 0 41.78 KB
#6030 WriteAndFlushEnrichedTraces net472 876μs 3.29μs 12.7μs 8.68 2.6 0.434 53.27 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.26μs 0.591ns 2.29ns 0.0145 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.77μs 1.73ns 6.71ns 0.0133 0 0 1.02 KB
master ExecuteNonQuery net472 2.06μs 1.61ns 6.24ns 0.156 0 0 987 B
#6030 ExecuteNonQuery net6.0 1.27μs 1.61ns 6.24ns 0.0138 0 0 1.02 KB
#6030 ExecuteNonQuery netcoreapp3.1 1.73μs 1.35ns 5.06ns 0.0136 0 0 1.02 KB
#6030 ExecuteNonQuery net472 2.01μs 2.34ns 8.44ns 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.21μs 0.627ns 2.43ns 0.0133 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.51μs 0.75ns 2.91ns 0.0133 0 0 976 B
master CallElasticsearch net472 2.4μs 0.999ns 3.87ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.25μs 0.663ns 2.57ns 0.0133 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.59μs 1.18ns 4.24ns 0.0133 0 0 1.02 KB
master CallElasticsearchAsync net472 2.46μs 1.37ns 5.13ns 0.167 0 0 1.05 KB
#6030 CallElasticsearch net6.0 1.24μs 0.516ns 1.93ns 0.0138 0 0 976 B
#6030 CallElasticsearch netcoreapp3.1 1.58μs 1.37ns 5.11ns 0.0128 0 0 976 B
#6030 CallElasticsearch net472 2.42μs 1.15ns 4.31ns 0.158 0 0 995 B
#6030 CallElasticsearchAsync net6.0 1.23μs 0.525ns 2.03ns 0.0135 0 0 952 B
#6030 CallElasticsearchAsync netcoreapp3.1 1.62μs 1.47ns 5.48ns 0.0137 0 0 1.02 KB
#6030 CallElasticsearchAsync net472 2.6μs 1.22ns 4.72ns 0.166 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.25μs 0.823ns 3.08ns 0.0132 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.63μs 4.27ns 16.5ns 0.0122 0 0 952 B
master ExecuteAsync net472 1.73μs 0.72ns 2.79ns 0.145 0 0 915 B
#6030 ExecuteAsync net6.0 1.21μs 1.16ns 4.5ns 0.0134 0 0 952 B
#6030 ExecuteAsync netcoreapp3.1 1.66μs 0.9ns 3.49ns 0.0125 0 0 952 B
#6030 ExecuteAsync net472 1.72μs 0.509ns 1.83ns 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.16μs 3.67ns 13.7ns 0.0313 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.07μs 2.01ns 7.51ns 0.038 0 0 2.76 KB
master SendAsync net472 7.74μs 5.7ns 22.1ns 0.496 0 0 3.15 KB
#6030 SendAsync net6.0 4.17μs 5.78ns 22.4ns 0.0312 0 0 2.22 KB
#6030 SendAsync netcoreapp3.1 5.15μs 2.43ns 9.4ns 0.0361 0 0 2.76 KB
#6030 SendAsync net472 7.77μs 2.43ns 9.41ns 0.497 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.52μs 1.1ns 4.27ns 0.0228 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.5μs 2.25ns 8.41ns 0.0224 0 0 1.64 KB
master EnrichedLog net472 2.54μs 1.24ns 4.62ns 0.249 0 0 1.57 KB
#6030 EnrichedLog net6.0 1.49μs 0.683ns 2.56ns 0.0231 0 0 1.64 KB
#6030 EnrichedLog netcoreapp3.1 2.18μs 11.9ns 69.6ns 0.0224 0 0 1.64 KB
#6030 EnrichedLog net472 2.62μs 1.13ns 4.37ns 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 116μs 148ns 575ns 0.058 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 120μs 200ns 749ns 0 0 0 4.28 KB
master EnrichedLog net472 147μs 99.3ns 372ns 0.658 0.219 0 4.46 KB
#6030 EnrichedLog net6.0 113μs 98.8ns 370ns 0.0564 0 0 4.28 KB
#6030 EnrichedLog netcoreapp3.1 117μs 184ns 688ns 0 0 0 4.28 KB
#6030 EnrichedLog net472 147μs 126ns 487ns 0.656 0.219 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.21μs 9.32ns 36.1ns 0.0301 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.16μs 2.83ns 11ns 0.029 0 0 2.2 KB
master EnrichedLog net472 4.92μs 5.75ns 22.3ns 0.32 0 0 2.02 KB
#6030 EnrichedLog net6.0 3.15μs 4.05ns 15.7ns 0.0301 0 0 2.2 KB
#6030 EnrichedLog netcoreapp3.1 4.23μs 0.973ns 3.64ns 0.0297 0 0 2.2 KB
#6030 EnrichedLog net472 4.84μs 1.36ns 5.28ns 0.32 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.38μs 0.464ns 1.74ns 0.0158 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.66μs 0.713ns 2.76ns 0.0149 0 0 1.14 KB
master SendReceive net472 2.15μs 1.43ns 5.34ns 0.183 0.00109 0 1.16 KB
#6030 SendReceive net6.0 1.51μs 0.903ns 3.5ns 0.0158 0 0 1.14 KB
#6030 SendReceive netcoreapp3.1 1.73μs 1.67ns 6.48ns 0.0154 0 0 1.14 KB
#6030 SendReceive net472 2.11μs 1.31ns 5.06ns 0.183 0.00105 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.85μs 1.03ns 3.99ns 0.0229 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.88μs 2.5ns 9.7ns 0.0216 0 0 1.65 KB
master EnrichedLog net472 4.56μs 0.811ns 2.92ns 0.322 0 0 2.04 KB
#6030 EnrichedLog net6.0 2.76μs 0.54ns 2.09ns 0.0221 0 0 1.6 KB
#6030 EnrichedLog netcoreapp3.1 3.97μs 2.25ns 8.73ns 0.0218 0 0 1.65 KB
#6030 EnrichedLog net472 4.42μs 2.22ns 8.61ns 0.322 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6030

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 1.241 493.90 398.08
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0 1.127 535.55 475.17

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 494ns 0.291ns 1.09ns 0.00801 0 0 576 B
master StartFinishSpan netcoreapp3.1 568ns 0.588ns 2.2ns 0.00795 0 0 576 B
master StartFinishSpan net472 668ns 0.53ns 2.05ns 0.0916 0 0 578 B
master StartFinishScope net6.0 535ns 0.23ns 0.893ns 0.0098 0 0 696 B
master StartFinishScope netcoreapp3.1 714ns 0.329ns 1.28ns 0.00929 0 0 696 B
master StartFinishScope net472 869ns 0.51ns 1.91ns 0.104 0 0 658 B
#6030 StartFinishSpan net6.0 398ns 0.311ns 1.21ns 0.00803 0 0 576 B
#6030 StartFinishSpan netcoreapp3.1 550ns 0.425ns 1.59ns 0.00771 0 0 576 B
#6030 StartFinishSpan net472 671ns 0.474ns 1.71ns 0.0917 0 0 578 B
#6030 StartFinishScope net6.0 475ns 0.182ns 0.705ns 0.00978 0 0 696 B
#6030 StartFinishScope netcoreapp3.1 717ns 1.2ns 4.65ns 0.0094 0 0 696 B
#6030 StartFinishScope net472 868ns 0.784ns 2.93ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6030

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.123 737.31 656.80

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 738ns 0.532ns 2.06ns 0.00985 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 996ns 0.348ns 1.25ns 0.00966 0 0 696 B
master RunOnMethodBegin net472 1.14μs 0.707ns 2.64ns 0.104 0 0 658 B
#6030 RunOnMethodBegin net6.0 657ns 0.189ns 0.734ns 0.00982 0 0 696 B
#6030 RunOnMethodBegin netcoreapp3.1 921ns 0.606ns 2.27ns 0.00944 0 0 696 B
#6030 RunOnMethodBegin net472 1.15μs 0.556ns 2.08ns 0.105 0 0 658 B

@andrewlock

andrewlock commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Benchmarks Report for appsec 🐌

Benchmarks for #6030 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.248
  • 1 benchmarks are slower, with geometric mean 1.274
  • 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 - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6030

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net472 1.274 163.65 208.48

Faster 🎉 in #6030

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472 1.248 4,672.82 3,742.92

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 71.4μs 78.9ns 306ns 0.072 0 0 6 KB
master AllCycleSimpleBody netcoreapp3.1 62.9μs 40.9ns 148ns 0.0941 0 0 6.95 KB
master AllCycleSimpleBody net472 48.5μs 25ns 93.6ns 1.31 0 0 8.34 KB
master AllCycleMoreComplexBody net6.0 76.9μs 71ns 266ns 0.115 0 0 9.51 KB
master AllCycleMoreComplexBody netcoreapp3.1 69μs 64.2ns 249ns 0.138 0 0 10.36 KB
master AllCycleMoreComplexBody net472 54.7μs 26.7ns 99.8ns 1.86 0.0274 0 11.85 KB
master ObjectExtractorSimpleBody net6.0 140ns 0.129ns 0.483ns 0.00392 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 195ns 0.334ns 1.29ns 0.0037 0 0 272 B
master ObjectExtractorSimpleBody net472 164ns 0.29ns 1.12ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 3.15μs 1.12ns 4.05ns 0.0536 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 3.89μs 3.27ns 12.7ns 0.0506 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 4.67μs 3.58ns 13.9ns 0.603 0.0046 0 3.8 KB
#6030 AllCycleSimpleBody net6.0 72μs 49.9ns 180ns 0.0713 0 0 6.01 KB
#6030 AllCycleSimpleBody netcoreapp3.1 62.3μs 50.4ns 188ns 0.0933 0 0 6.95 KB
#6030 AllCycleSimpleBody net472 48.5μs 33.9ns 127ns 1.31 0 0 8.34 KB
#6030 AllCycleMoreComplexBody net6.0 77.5μs 139ns 540ns 0.115 0 0 9.51 KB
#6030 AllCycleMoreComplexBody netcoreapp3.1 69.2μs 93.7ns 363ns 0.139 0 0 10.36 KB
#6030 AllCycleMoreComplexBody net472 56.2μs 27.1ns 101ns 1.86 0.0282 0 11.85 KB
#6030 ObjectExtractorSimpleBody net6.0 141ns 0.435ns 1.69ns 0.00397 0 0 280 B
#6030 ObjectExtractorSimpleBody netcoreapp3.1 198ns 0.334ns 1.29ns 0.00375 0 0 272 B
#6030 ObjectExtractorSimpleBody net472 208ns 0.122ns 0.438ns 0.0446 0 0 281 B
#6030 ObjectExtractorMoreComplexBody net6.0 3.09μs 1.7ns 6.38ns 0.0526 0 0 3.78 KB
#6030 ObjectExtractorMoreComplexBody netcoreapp3.1 3.83μs 2.4ns 8.99ns 0.0495 0 0 3.69 KB
#6030 ObjectExtractorMoreComplexBody net472 3.74μs 2.58ns 9.3ns 0.602 0.00561 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 19ns 73.6ns 0.455 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 54.4μs 17.8ns 69ns 0.433 0 0 32.4 KB
master EncodeArgs net472 65.8μs 37.3ns 144ns 5.14 0.0659 0 32.5 KB
master EncodeLegacyArgs net6.0 75.8μs 418ns 2.51μs 0 0 0 2.14 KB
master EncodeLegacyArgs netcoreapp3.1 104μs 426ns 1.65μs 0 0 0 2.15 KB
master EncodeLegacyArgs net472 151μs 103ns 400ns 0.3 0 0 2.15 KB
#6030 EncodeArgs net6.0 38.4μs 19.4ns 72.7ns 0.442 0 0 32.4 KB
#6030 EncodeArgs netcoreapp3.1 53.8μs 28ns 108ns 0.429 0 0 32.4 KB
#6030 EncodeArgs net472 66.2μs 26.6ns 103ns 5.14 0.0663 0 32.5 KB
#6030 EncodeLegacyArgs net6.0 72.3μs 78.8ns 273ns 0 0 0 2.14 KB
#6030 EncodeLegacyArgs netcoreapp3.1 105μs 102ns 354ns 0 0 0 2.14 KB
#6030 EncodeLegacyArgs net472 149μs 71.6ns 268ns 0.298 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 142ns 548ns 0 0 0 2.44 KB
master RunWafRealisticBenchmark netcoreapp3.1 198μs 261ns 1.01μs 0 0 0 2.39 KB
master RunWafRealisticBenchmark net472 211μs 168ns 651ns 0.311 0 0 2.46 KB
master RunWafRealisticBenchmarkWithAttack net6.0 122μs 108ns 404ns 0 0 0 1.47 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 130μs 57.4ns 222ns 0 0 0 1.46 KB
master RunWafRealisticBenchmarkWithAttack net472 139μs 32.7ns 127ns 0.208 0 0 1.49 KB
#6030 RunWafRealisticBenchmark net6.0 182μs 57.9ns 224ns 0 0 0 2.44 KB
#6030 RunWafRealisticBenchmark netcoreapp3.1 196μs 187ns 723ns 0 0 0 2.39 KB
#6030 RunWafRealisticBenchmark net472 209μs 74.2ns 267ns 0.312 0 0 2.46 KB
#6030 RunWafRealisticBenchmarkWithAttack net6.0 122μs 105ns 405ns 0 0 0 1.47 KB
#6030 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 130μs 110ns 426ns 0 0 0 1.46 KB
#6030 RunWafRealisticBenchmarkWithAttack net472 139μs 29.1ns 113ns 0.209 0 0 1.49 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6030

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 253.55 KB 255.65 KB 2.1 KB 0.83%

Fewer allocations 🎉 in #6030

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 59.07 KB 57.9 KB -1.18 KB -1.99%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 265.54 KB 253.34 KB -12.2 KB -4.59%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 60μs 782ns 7.78μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 60μs 802ns 7.98μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 37.3μs 50ns 180ns 0 0 0 59.07 KB
master StringConcatAspectBenchmark net6.0 306μs 1.63μs 9.92μs 0 0 0 253.55 KB
master StringConcatAspectBenchmark netcoreapp3.1 339μs 1μs 3.47μs 0 0 0 265.54 KB
master StringConcatAspectBenchmark net472 276μs 5.47μs 53.3μs 0 0 0 278.53 KB
#6030 StringConcatBenchmark net6.0 59.5μs 727ns 7.24μs 0 0 0 43.44 KB
#6030 StringConcatBenchmark netcoreapp3.1 53.2μs 297ns 1.78μs 0 0 0 42.64 KB
#6030 StringConcatBenchmark net472 37.7μs 99.7ns 359ns 0 0 0 57.9 KB
#6030 StringConcatAspectBenchmark net6.0 305μs 5.6μs 54.9μs 0 0 0 255.65 KB
#6030 StringConcatAspectBenchmark netcoreapp3.1 343μs 2.7μs 25.2μs 0 0 0 253.34 KB
#6030 StringConcatAspectBenchmark net472 268μs 6.03μs 58.8μs 0 0 0 278.53 KB

@andrewlock

andrewlock commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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 (6030) (11.066M)   : 0, 11065944
    master (11.013M)   : 0, 11012653
    benchmarks/2.9.0 (11.081M)   : 0, 11080577

    section Automatic
    This PR (6030) (7.317M)   : 0, 7317210
    master (7.257M)   : 0, 7256944
    benchmarks/2.9.0 (7.732M)   : 0, 7732233

    section Trace stats
    master (7.393M)   : 0, 7393311

    section Manual
    master (11.210M)   : 0, 11210238

    section Manual + Automatic
    This PR (6030) (6.695M)   : 0, 6695249
    master (6.902M)   : 0, 6901880

    section DD_TRACE_ENABLED=0
    master (10.246M)   : 0, 10245809
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6030) (9.640M)   : 0, 9639956
    master (9.699M)   : 0, 9699374
    benchmarks/2.9.0 (9.798M)   : 0, 9798067

    section Automatic
    This PR (6030) (6.595M)   : 0, 6595159
    master (6.629M)   : 0, 6628939

    section Trace stats
    master (6.955M)   : 0, 6954928

    section Manual
    master (9.662M)   : 0, 9662390

    section Manual + Automatic
    This PR (6030) (5.900M)   : 0, 5899593
    master (6.096M)   : 0, 6095670

    section DD_TRACE_ENABLED=0
    master (8.710M)   : 0, 8710481
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6030) (10.165M)   : 0, 10165305
    master (10.071M)   : 0, 10071070
    benchmarks/2.9.0 (10.067M)   : 0, 10067315

    section Automatic
    This PR (6030) (6.787M)   : 0, 6787253
    master (6.612M)   : 0, 6612085
    benchmarks/2.9.0 (7.552M)   : 0, 7552193

    section Trace stats
    master (7.286M)   : 0, 7285519

    section Manual
    master (10.026M)   : 0, 10025950

    section Manual + Automatic
    This PR (6030) (6.213M)   : 0, 6213214
    master (6.346M)   : 0, 6345551

    section DD_TRACE_ENABLED=0
    master (9.203M)   : 0, 9203356
Loading

@NachoEchevarria
NachoEchevarria marked this pull request as ready for review October 10, 2024 12:38
@NachoEchevarria
NachoEchevarria requested a review from a team as a code owner October 10, 2024 12:38
e-n-0
e-n-0 approved these changes Oct 11, 2024
View reviewed changes

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice 😄

anna-git
anna-git approved these changes Oct 11, 2024
View reviewed changes

@anna-git anna-git left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

interesting

@NachoEchevarria
NachoEchevarria merged commit a721f7b into master Oct 14, 2024
@NachoEchevarria
NachoEchevarria deleted the nacho/ReportedExternalWafsRequestHeaders branch October 14, 2024 07:22
@github-actions github-actions Bot added this to the vNext-v3 milestone Oct 14, 2024
@e-n-0 e-n-0 mentioned this pull request Jan 27, 2025
Merged
e-n-0 added a commit that referenced this pull request Feb 5, 2025
@e-n-0
[ASM] Fix errors exceptions on HttpContext get_Items (#6592)
ca322c4 
## Summary of changes

Handle the errors when trying to access the Items features of the
`HttpContext`.

All usage of Context.Items int the SecurityCoordinator has been changed
so the errors are caught.

Should fix these example stacktraces:

```
System.NullReferenceException
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)
```

```
System.ObjectDisposedException
   at Microsoft.AspNetCore.Http.Features.FeatureReferences`1.ThrowContextDisposed()
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityReporter.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)
```


## Reason for change

A `ObjectDisposedException` and a `NullReferenceException` can happen in
concurrent access to the `HttpContext`.
This situation of race condition can happen while the request is
finished while async jobs still run in the background and the waf for
RASP is run.

The HttpContext can be disposed from different manner by the user. You
can find more information in this
[PR](#6529).

## Implementation details

A new boolean have been added that reflect the state if the Items threw
an exception, thus making the code not access the HttpContext anymore in
the `TryReport` method.

## Test coverage

Added a unit test verifying that our code doesn't throw exceptions when
the `Items` of the `HttpContext` isn't available.

## Other details

Related PRs:
- #6529
- #6030
- #6017
chojomok pushed a commit that referenced this pull request Jul 15, 2025
@e-n-0 @chojomok
[ASM] Fix errors exceptions on HttpContext get_Items (#6592)
38d12e6 
## Summary of changes

Handle the errors when trying to access the Items features of the
`HttpContext`.

All usage of Context.Items int the SecurityCoordinator has been changed
so the errors are caught.

Should fix these example stacktraces:

```
System.NullReferenceException
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)
```

```
System.ObjectDisposedException
   at Microsoft.AspNetCore.Http.Features.FeatureReferences`1.ThrowContextDisposed()
   at Microsoft.AspNetCore.Http.DefaultHttpContext.get_Items()
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.HttpTransport.get_ReportedExternalWafsRequestHeaders()
   at Datadog.Trace.AppSec.Coordinator.SecurityReporter.TryReport(IResult result, Boolean blocked, Nullable`1 status)
   at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.ReportAndBlock(IResult result)
   at Datadog.Trace.AppSec.Rasp.RaspModule.RunWafRasp(Dictionary`2 arguments, Span rootSpan, String address)
   at Datadog.Trace.AppSec.VulnerabilitiesModule.OnSSRF(String uriText)
   at Datadog.Trace.Iast.Aspects.System.Net.HttpClientAspect.Review(String parameter)
```


## Reason for change

A `ObjectDisposedException` and a `NullReferenceException` can happen in
concurrent access to the `HttpContext`.
This situation of race condition can happen while the request is
finished while async jobs still run in the background and the waf for
RASP is run.

The HttpContext can be disposed from different manner by the user. You
can find more information in this
[PR](#6529).

## Implementation details

A new boolean have been added that reflect the state if the Items threw
an exception, thus making the code not access the HttpContext anymore in
the `TryReport` method.

## Test coverage

Added a unit test verifying that our code doesn't throw exceptions when
the `Items` of the `HttpContext` isn't available.

## Other details

Related PRs:
- #6529
- #6030
- #6017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Assignees

No one assigned

Labels

area:asm identified-by:telemetry

Projects

None yet

Milestone

3.4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@NachoEchevarria @andrewlock @anna-git @e-n-0