Skip to content

[ASM]Fix InvalidOperationException in httpContext.Items#6017

Merged
NachoEchevarria merged 6 commits into
masterfrom
nacho/FixInvalidOperationExceptionInIsBlocked
Sep 12, 2024
Merged

[ASM]Fix InvalidOperationException in httpContext.Items#6017
NachoEchevarria merged 6 commits into
masterfrom
nacho/FixInvalidOperationExceptionInIsBlocked

Conversation

@NachoEchevarria

@NachoEchevarria NachoEchevarria commented Sep 10, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary of changes

This PR fixes the InvalidOperationException that is thrown in the ASM code.
Some logs can be found here.

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

@datadog-ddstaging

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

Copy link
Copy Markdown

Datadog Report

Branch report: nacho/FixInvalidOperationExceptionInIsBlocked
Commit report: 3765cf7
Test service: dd-trace-dotnet

✅ 0 Failed, 363715 Passed, 2070 Skipped, 16h 9m 3.84s Total Time

@andrewlock

andrewlock commented Sep 10, 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 (6017) - mean (69ms)  : 66, 73
     .   : milestone, 69,
    master - mean (70ms)  : 66, 73
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6017) - mean (1,115ms)  : 1093, 1137
     .   : milestone, 1115,
    master - mean (1,115ms)  : 1101, 1128
     .   : milestone, 1115,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6017) - mean (109ms)  : 104, 114
     .   : milestone, 109,
    master - mean (108ms)  : 105, 111
     .   : milestone, 108,

    section CallTarget+Inlining+NGEN
    This PR (6017) - mean (815ms)  : 800, 830
     .   : milestone, 815,
    master - mean (815ms)  : 801, 829
     .   : milestone, 815,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6017) - mean (92ms)  : 89, 96
     .   : milestone, 92,
    master - mean (93ms)  : 89, 97
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (6017) - mean (770ms)  : 750, 790
     .   : milestone, 770,
    master - mean (767ms)  : 750, 783
     .   : milestone, 767,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6017) - mean (190ms)  : 187, 194
     .   : milestone, 190,
    master - mean (192ms)  : 188, 196
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (6017) - mean (1,196ms)  : 1167, 1225
     .   : milestone, 1196,
    master - mean (1,195ms)  : 1169, 1220
     .   : milestone, 1195,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6017) - mean (276ms)  : 271, 281
     .   : milestone, 276,
    master - mean (277ms)  : 272, 282
     .   : milestone, 277,

    section CallTarget+Inlining+NGEN
    This PR (6017) - mean (976ms)  : 952, 999
     .   : milestone, 976,
    master - mean (975ms)  : 952, 998
     .   : milestone, 975,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6017) - mean (266ms)  : 260, 271
     .   : milestone, 266,
    master - mean (265ms)  : 261, 268
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6017) - mean (953ms)  : 926, 979
     .   : milestone, 953,
    master - mean (961ms)  : 937, 985
     .   : milestone, 961,
Loading

@andrewlock

andrewlock commented Sep 10, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Benchmarks Report for appsec 🐌

Benchmarks for #6017 compared to master:

  • 1 benchmarks are slower, with geometric mean 1.163
  • 1 benchmarks have fewer allocations
  • 2 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 #6017

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net472 1.163 167.85 195.25

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 73.3μs 62.1ns 232ns 0.0728 0 0 6.01 KB
master AllCycleSimpleBody netcoreapp3.1 61.7μs 68.8ns 267ns 0.0925 0 0 6.95 KB
master AllCycleSimpleBody net472 48μs 40.9ns 159ns 1.32 0 0 8.34 KB
master AllCycleMoreComplexBody net6.0 77.7μs 64ns 239ns 0.116 0 0 9.51 KB
master AllCycleMoreComplexBody netcoreapp3.1 68.5μs 76.2ns 295ns 0.138 0 0 10.37 KB
master AllCycleMoreComplexBody net472 55.8μs 57.4ns 222ns 1.87 0.0279 0 11.85 KB
master ObjectExtractorSimpleBody net6.0 142ns 0.11ns 0.411ns 0.00394 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 243ns 0.123ns 0.478ns 0.00368 0 0 272 B
master ObjectExtractorSimpleBody net472 168ns 0.179ns 0.694ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 3.1μs 2.25ns 8.43ns 0.0527 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 4.02μs 2.76ns 10.7ns 0.0485 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 3.94μs 1.6ns 6ns 0.601 0.00591 0 3.8 KB
#6017 AllCycleSimpleBody net6.0 74.8μs 104ns 390ns 0.0741 0 0 6.01 KB
#6017 AllCycleSimpleBody netcoreapp3.1 63.2μs 82.8ns 321ns 0.0945 0 0 6.95 KB
#6017 AllCycleSimpleBody net472 50.2μs 52.7ns 197ns 1.32 0 0 8.34 KB
#6017 AllCycleMoreComplexBody net6.0 81.3μs 111ns 428ns 0.122 0 0 9.51 KB
#6017 AllCycleMoreComplexBody netcoreapp3.1 69.4μs 127ns 491ns 0.138 0 0 10.36 KB
#6017 AllCycleMoreComplexBody net472 57.9μs 73.8ns 286ns 1.86 0.029 0 11.85 KB
#6017 ObjectExtractorSimpleBody net6.0 147ns 0.316ns 1.23ns 0.00394 0 0 280 B
#6017 ObjectExtractorSimpleBody netcoreapp3.1 256ns 0.245ns 0.949ns 0.00377 0 0 272 B
#6017 ObjectExtractorSimpleBody net472 195ns 0.261ns 0.977ns 0.0445 0 0 281 B
#6017 ObjectExtractorMoreComplexBody net6.0 3.15μs 1.36ns 4.9ns 0.0536 0 0 3.78 KB
#6017 ObjectExtractorMoreComplexBody netcoreapp3.1 4.36μs 9.62ns 37.2ns 0.0495 0 0 3.69 KB
#6017 ObjectExtractorMoreComplexBody net472 4.07μs 7.4ns 28.7ns 0.603 0.00607 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 39μs 22.4ns 86.7ns 0.464 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 53.9μs 17.1ns 63.9ns 0.43 0 0 32.4 KB
master EncodeArgs net472 66.1μs 18ns 67.2ns 5.16 0.0661 0 32.5 KB
master EncodeLegacyArgs net6.0 73.8μs 68.4ns 265ns 0 0 0 2.14 KB
master EncodeLegacyArgs netcoreapp3.1 103μs 41.1ns 159ns 0 0 0 2.14 KB
master EncodeLegacyArgs net472 153μs 38.7ns 145ns 0.306 0 0 2.15 KB
#6017 EncodeArgs net6.0 38.6μs 19.8ns 76.7ns 0.444 0 0 32.4 KB
#6017 EncodeArgs netcoreapp3.1 54.3μs 36.8ns 142ns 0.434 0 0 32.4 KB
#6017 EncodeArgs net472 66.9μs 57.9ns 224ns 5.16 0.0675 0 32.5 KB
#6017 EncodeLegacyArgs net6.0 78.4μs 19.9ns 71.9ns 0 0 0 2.14 KB
#6017 EncodeLegacyArgs netcoreapp3.1 106μs 53.3ns 206ns 0 0 0 2.14 KB
#6017 EncodeLegacyArgs net472 153μs 103ns 400ns 0.306 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 184μs 64.6ns 250ns 0 0 0 2.42 KB
master RunWafRealisticBenchmark netcoreapp3.1 197μs 256ns 993ns 0 0 0 2.37 KB
master RunWafRealisticBenchmark net472 209μs 133ns 498ns 0.313 0 0 2.43 KB
master RunWafRealisticBenchmarkWithAttack net6.0 122μs 133ns 514ns 0 0 0 1.46 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 130μs 285ns 1.11μs 0 0 0 1.45 KB
master RunWafRealisticBenchmarkWithAttack net472 142μs 218ns 843ns 0.212 0 0 1.48 KB
#6017 RunWafRealisticBenchmark net6.0 187μs 121ns 467ns 0 0 0 2.42 KB
#6017 RunWafRealisticBenchmark netcoreapp3.1 200μs 298ns 1.15μs 0 0 0 2.37 KB
#6017 RunWafRealisticBenchmark net472 211μs 120ns 466ns 0.314 0 0 2.43 KB
#6017 RunWafRealisticBenchmarkWithAttack net6.0 123μs 79ns 306ns 0 0 0 1.46 KB
#6017 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 132μs 97.2ns 377ns 0 0 0 1.45 KB
#6017 RunWafRealisticBenchmarkWithAttack net472 141μs 103ns 398ns 0.21 0 0 1.48 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6017

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 264.38 KB 324.54 KB 60.15 KB 22.75%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 55.75 KB 59.39 KB 3.64 KB 6.53%

Fewer allocations 🎉 in #6017

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 263.51 KB 262 KB -1.51 KB -0.57%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 60μs 698ns 6.91μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 61.3μs 794ns 7.86μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 36.7μs 86.1ns 322ns 0 0 0 55.75 KB
master StringConcatAspectBenchmark net6.0 307μs 1.54μs 9.64μs 0 0 0 264.38 KB
master StringConcatAspectBenchmark netcoreapp3.1 342μs 1.88μs 11.6μs 0 0 0 263.51 KB
master StringConcatAspectBenchmark net472 274μs 5.34μs 51.5μs 0 0 0 278.53 KB
#6017 StringConcatBenchmark net6.0 64.4μs 999ns 9.94μs 0 0 0 43.44 KB
#6017 StringConcatBenchmark netcoreapp3.1 54.7μs 304ns 1.87μs 0 0 0 42.64 KB
#6017 StringConcatBenchmark net472 37μs 103ns 370ns 0 0 0 59.39 KB
#6017 StringConcatAspectBenchmark net6.0 303μs 5.36μs 52.2μs 0 0 0 324.54 KB
#6017 StringConcatAspectBenchmark netcoreapp3.1 353μs 3.22μs 31.2μs 0 0 0 262 KB
#6017 StringConcatAspectBenchmark net472 290μs 7.08μs 69.1μs 0 0 0 278.53 KB

@andrewlock

andrewlock commented Sep 10, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Benchmarks Report for tracer 🐌

Benchmarks for #6017 compared to master:

  • 3 benchmarks are faster, with geometric mean 1.127
  • 1 benchmarks are slower, with geometric mean 1.155
  • 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.72μs 44.4ns 336ns 0.0162 0.0081 0 5.43 KB
master StartStopWithChild netcoreapp3.1 9.56μs 51.6ns 278ns 0.0193 0.00965 0 5.62 KB
master StartStopWithChild net472 16.1μs 41.6ns 161ns 1.02 0.296 0.0961 6.07 KB
#6017 StartStopWithChild net6.0 7.85μs 44.7ns 332ns 0.0199 0.00796 0 5.43 KB
#6017 StartStopWithChild netcoreapp3.1 9.86μs 53.8ns 295ns 0.0193 0.00963 0 5.62 KB
#6017 StartStopWithChild net472 15.8μs 52ns 201ns 1.02 0.315 0.0944 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 470μs 225ns 871ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 645μs 338ns 1.27μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 831μs 470ns 1.82μs 0.411 0 0 3.3 KB
#6017 WriteAndFlushEnrichedTraces net6.0 486μs 280ns 1.05μs 0 0 0 2.7 KB
#6017 WriteAndFlushEnrichedTraces netcoreapp3.1 630μs 325ns 1.26μs 0 0 0 2.7 KB
#6017 WriteAndFlushEnrichedTraces net472 848μs 713ns 2.76μ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.14μs 9.9μs 0.194 0 0 18.45 KB
master SendRequest netcoreapp3.1 219μs 1.25μs 9.68μs 0.233 0 0 20.61 KB
master SendRequest net472 0.0281ns 0.0116ns 0.0567ns 0 0 0 0 b
#6017 SendRequest net6.0 199μs 1.05μs 7.6μs 0.194 0 0 18.45 KB
#6017 SendRequest netcoreapp3.1 225μs 1.29μs 10.7μs 0.21 0 0 20.61 KB
#6017 SendRequest net472 0.000614ns 0.000378ns 0.00141ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6017

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 41.57 KB 41.79 KB 218 B 0.52%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 561μs 2.72μs 11.9μs 0.283 0 0 41.57 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 672μs 2.66μs 9.95μs 0.349 0 0 41.57 KB
master WriteAndFlushEnrichedTraces net472 871μs 4.63μs 24.9μs 8.87 2.53 0.422 53.33 KB
#6017 WriteAndFlushEnrichedTraces net6.0 573μs 2.01μs 7.53μs 0.568 0 0 41.68 KB
#6017 WriteAndFlushEnrichedTraces netcoreapp3.1 694μs 3.45μs 15.4μs 0.343 0 0 41.79 KB
#6017 WriteAndFlushEnrichedTraces net472 879μs 2.55μs 9.53μs 8.42 2.66 0.443 53.36 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.37μs 1.33ns 4.98ns 0.014 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.84μs 1.76ns 6.6ns 0.0137 0 0 1.02 KB
master ExecuteNonQuery net472 2.04μs 1.96ns 7.33ns 0.156 0 0 987 B
#6017 ExecuteNonQuery net6.0 1.29μs 1.3ns 5.04ns 0.0141 0 0 1.02 KB
#6017 ExecuteNonQuery netcoreapp3.1 1.73μs 1.01ns 3.79ns 0.013 0 0 1.02 KB
#6017 ExecuteNonQuery net472 2.04μs 1.42ns 5.31ns 0.156 0 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6017

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net6.0 1.155 1,146.16 1,323.87

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.15μs 0.737ns 2.76ns 0.0137 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.55μs 0.739ns 2.86ns 0.0132 0 0 976 B
master CallElasticsearch net472 2.45μs 2.6ns 10.1ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.34μs 1.53ns 5.93ns 0.0134 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.54μs 1.1ns 4.13ns 0.0139 0 0 1.02 KB
master CallElasticsearchAsync net472 2.45μs 1.7ns 6.59ns 0.167 0 0 1.05 KB
#6017 CallElasticsearch net6.0 1.32μs 0.739ns 2.77ns 0.0136 0 0 976 B
#6017 CallElasticsearch netcoreapp3.1 1.51μs 0.676ns 2.62ns 0.0129 0 0 976 B
#6017 CallElasticsearch net472 2.41μs 2.46ns 9.53ns 0.158 0 0 995 B
#6017 CallElasticsearchAsync net6.0 1.21μs 0.498ns 1.8ns 0.0135 0 0 952 B
#6017 CallElasticsearchAsync netcoreapp3.1 1.7μs 0.967ns 3.62ns 0.0135 0 0 1.02 KB
#6017 CallElasticsearchAsync net472 2.62μs 2.39ns 8.95ns 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.3μs 0.775ns 3ns 0.013 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.63μs 2.21ns 8.57ns 0.0124 0 0 952 B
master ExecuteAsync net472 1.78μs 0.664ns 2.39ns 0.145 0 0 915 B
#6017 ExecuteAsync net6.0 1.31μs 1.55ns 5.8ns 0.0132 0 0 952 B
#6017 ExecuteAsync netcoreapp3.1 1.7μs 0.928ns 3.59ns 0.0129 0 0 952 B
#6017 ExecuteAsync net472 1.74μs 0.47ns 1.76ns 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.27μs 1.32ns 4.75ns 0.0299 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.04μs 2.25ns 8.7ns 0.0377 0 0 2.76 KB
master SendAsync net472 7.77μs 1.65ns 6.4ns 0.498 0 0 3.15 KB
#6017 SendAsync net6.0 4.19μs 1.37ns 4.73ns 0.0314 0 0 2.22 KB
#6017 SendAsync netcoreapp3.1 5.13μs 0.997ns 3.45ns 0.0364 0 0 2.76 KB
#6017 SendAsync net472 7.73μs 2.25ns 8.73ns 0.496 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.51μs 0.694ns 2.59ns 0.023 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.2μs 1.86ns 6.96ns 0.0224 0 0 1.64 KB
master EnrichedLog net472 2.81μs 2.45ns 9.16ns 0.249 0 0 1.57 KB
#6017 EnrichedLog net6.0 1.64μs 0.827ns 3.2ns 0.0233 0 0 1.64 KB
#6017 EnrichedLog netcoreapp3.1 2.27μs 0.893ns 3.34ns 0.0215 0 0 1.64 KB
#6017 EnrichedLog net472 2.66μs 2ns 7.22ns 0.25 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 118μs 153ns 594ns 0.0578 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 120μs 288ns 1.12μs 0 0 0 4.28 KB
master EnrichedLog net472 148μs 137ns 529ns 0.663 0.221 0 4.46 KB
#6017 EnrichedLog net6.0 115μs 171ns 664ns 0.0585 0 0 4.28 KB
#6017 EnrichedLog netcoreapp3.1 119μs 232ns 897ns 0.0594 0 0 4.28 KB
#6017 EnrichedLog net472 149μs 234ns 907ns 0.678 0.226 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.14μs 0.559ns 2.02ns 0.0308 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.13μs 1.27ns 4.93ns 0.0288 0 0 2.2 KB
master EnrichedLog net472 4.89μs 1.39ns 5.38ns 0.321 0 0 2.02 KB
#6017 EnrichedLog net6.0 2.92μs 1.2ns 4.66ns 0.0307 0 0 2.2 KB
#6017 EnrichedLog netcoreapp3.1 4.17μs 2.15ns 8.33ns 0.0293 0 0 2.2 KB
#6017 EnrichedLog net472 4.87μs 1.68ns 6.51ns 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.32μs 0.418ns 1.56ns 0.0158 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.79μs 0.502ns 1.81ns 0.0153 0 0 1.14 KB
master SendReceive net472 2.16μs 1.93ns 7.24ns 0.184 0.00109 0 1.16 KB
#6017 SendReceive net6.0 1.28μs 0.48ns 1.86ns 0.0162 0 0 1.14 KB
#6017 SendReceive netcoreapp3.1 1.76μs 0.927ns 3.34ns 0.0149 0 0 1.14 KB
#6017 SendReceive net472 2.16μs 1.18ns 4.55ns 0.183 0 0 1.16 KB
Benchmarks.Trace.SerilogBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6017

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SerilogBenchmark.EnrichedLog‑net6.0 1.132 2,930.18 2,588.95

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.93μs 0.521ns 1.95ns 0.022 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.89μs 1.93ns 7.46ns 0.0214 0 0 1.65 KB
master EnrichedLog net472 4.34μs 2.47ns 9.56ns 0.322 0 0 2.04 KB
#6017 EnrichedLog net6.0 2.59μs 0.914ns 3.54ns 0.0221 0 0 1.6 KB
#6017 EnrichedLog netcoreapp3.1 3.92μs 1.05ns 3.92ns 0.0216 0 0 1.65 KB
#6017 EnrichedLog net472 4.35μs 3.44ns 13.3ns 0.323 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6017

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.114 811.79 728.71

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 392ns 0.242ns 0.936ns 0.00813 0 0 576 B
master StartFinishSpan netcoreapp3.1 577ns 1.62ns 5.85ns 0.00797 0 0 576 B
master StartFinishSpan net472 655ns 0.893ns 3.46ns 0.0917 0 0 578 B
master StartFinishScope net6.0 538ns 0.392ns 1.52ns 0.00965 0 0 696 B
master StartFinishScope netcoreapp3.1 812ns 0.641ns 2.48ns 0.00937 0 0 696 B
master StartFinishScope net472 820ns 0.766ns 2.97ns 0.104 0 0 658 B
#6017 StartFinishSpan net6.0 399ns 0.312ns 1.21ns 0.00802 0 0 576 B
#6017 StartFinishSpan netcoreapp3.1 625ns 0.501ns 1.87ns 0.0079 0 0 576 B
#6017 StartFinishSpan net472 626ns 0.289ns 1.08ns 0.0917 0 0 578 B
#6017 StartFinishScope net6.0 542ns 0.644ns 2.5ns 0.00971 0 0 696 B
#6017 StartFinishScope netcoreapp3.1 728ns 0.527ns 2.04ns 0.00919 0 0 696 B
#6017 StartFinishScope net472 903ns 0.6ns 2.33ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6017

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑netcoreapp3.1 1.135 928.01 817.70

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 587ns 0.573ns 2.22ns 0.00977 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 927ns 0.975ns 3.77ns 0.00922 0 0 696 B
master RunOnMethodBegin net472 1.09μs 1.44ns 5.59ns 0.104 0 0 658 B
#6017 RunOnMethodBegin net6.0 647ns 0.608ns 2.35ns 0.00982 0 0 696 B
#6017 RunOnMethodBegin netcoreapp3.1 818ns 0.608ns 2.36ns 0.00942 0 0 696 B
#6017 RunOnMethodBegin net472 1.09μs 1.14ns 4.43ns 0.105 0 0 658 B

@andrewlock

andrewlock commented Sep 10, 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 (6017) (11.060M)   : 0, 11060233
    master (11.058M)   : 0, 11058270
    benchmarks/2.9.0 (11.185M)   : 0, 11185492

    section Automatic
    This PR (6017) (7.416M)   : 0, 7416047
    master (7.433M)   : 0, 7433068
    benchmarks/2.9.0 (7.935M)   : 0, 7934520

    section Trace stats
    master (7.639M)   : 0, 7639043

    section Manual
    master (11.001M)   : 0, 11001149

    section Manual + Automatic
    This PR (6017) (6.874M)   : 0, 6874043
    master (6.890M)   : 0, 6889686

    section DD_TRACE_ENABLED=0
    master (10.219M)   : 0, 10219072
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6017) (9.556M)   : 0, 9556201
    master (9.512M)   : 0, 9512187
    benchmarks/2.9.0 (9.560M)   : 0, 9559569

    section Automatic
    This PR (6017) (6.603M)   : 0, 6602961
    master (6.599M)   : 0, 6598948

    section Trace stats
    master (6.877M)   : 0, 6877069

    section Manual
    master (9.582M)   : 0, 9582301

    section Manual + Automatic
    This PR (6017) (6.243M)   : 0, 6243046
    master (6.227M)   : 0, 6227256

    section DD_TRACE_ENABLED=0
    master (8.944M)   : 0, 8943787
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6017) (10.210M)   : 0, 10209707
    master (9.874M)   : 0, 9873927
    benchmarks/2.9.0 (10.118M)   : 0, 10117626

    section Automatic
    This PR (6017) (6.807M)   : 0, 6806879
    master (6.609M)   : 0, 6609340
    benchmarks/2.9.0 (7.416M)   : 0, 7415715

    section Trace stats
    master (7.330M)   : 0, 7329885

    section Manual
    master (10.106M)   : 0, 10106430

    section Manual + Automatic
    This PR (6017) (6.359M)   : 0, 6359023
    master (6.122M)   : 0, 6122215

    section DD_TRACE_ENABLED=0
    master (9.295M)   : 0, 9294721
Loading

@NachoEchevarria NachoEchevarria changed the title Fix [ASM]Fix InvalidOperationException in httpContext.Items Sep 11, 2024
@NachoEchevarria
NachoEchevarria marked this pull request as ready for review September 11, 2024 15:23
@NachoEchevarria
NachoEchevarria requested a review from a team as a code owner September 11, 2024 15:23
@robertpi

robertpi commented Sep 12, 2024

Copy link
Copy Markdown
Member

Good catch! Thank you.

@NachoEchevarria

NachoEchevarria commented Sep 12, 2024

Copy link
Copy Markdown
Collaborator Author

Thanks for the reviews!

@NachoEchevarria
NachoEchevarria merged commit 4afaa3b into master Sep 12, 2024
@NachoEchevarria
NachoEchevarria deleted the nacho/FixInvalidOperationExceptionInIsBlocked branch September 12, 2024 13:59
@github-actions github-actions Bot added this to the vNext-v3 milestone Sep 12, 2024
@NachoEchevarria NachoEchevarria mentioned this pull request Sep 13, 2024
Merged
NachoEchevarria added a commit that referenced this pull request Oct 14, 2024
@NachoEchevarria
[ASM] Fix exception when accessing ReportedExternalWafsRequestHeaders (…
a721f7b 
…#6030)

## Summary of changes

This PR is closely related to [this
one](#6017)

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
<!-- 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. -->
@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

@robertpi robertpi robertpi approved these changes
@dromanol dromanol dromanol approved these changes

Assignees

No one assigned

Labels

area:asm identified-by:telemetry

Projects

None yet

Milestone

3.3.0

Development

Successfully merging this pull request may close these issues.

4 participants

@NachoEchevarria @andrewlock @robertpi @dromanol