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.

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

[IAST] CallSite with generics support #5913

Merged
merged 4 commits into from
Aug 23, 2024

Conversation

daniel-romano-DD
Copy link
Contributor

@daniel-romano-DD daniel-romano-DD commented Aug 16, 2024

Summary of changes

Added support for aspects with generics

Reason for change

Some aspects got parameters with generic arguments. This case was not supported

Implementation details

Updated the SourceGenerator to correctly provide signatures for aspects with generics.
Updated the dataflow to generate MethodSpecs for those aspects with generic arguments.

Test coverage

Added new SourceGenerator tests.
Added instrumentation tests for the new aspects with generics.

Other details

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Aug 16, 2024

Datadog Report

Branch report: dani/iast/callsite_generics
Commit report: dc23c20
Test service: dd-trace-dotnet

✅ 0 Failed, 414823 Passed, 3180 Skipped, 35h 53m 52.45s Total Time

@andrewlock
Copy link
Member

andrewlock commented Aug 16, 2024

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).

@andrewlock
Copy link
Member

andrewlock commented Aug 16, 2024

Benchmarks Report for appsec 🐌

Benchmarks for #5913 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.226
  • 1 benchmarks are slower, with geometric mean 1.330
  • 1 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 #5913

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net6.0 1.330 142.14 188.98

Faster 🎉 in #5913

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1 1.226 240.78 196.35

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 72.9μs 108ns 418ns 0.0727 0 0 6 KB
master AllCycleSimpleBody netcoreapp3.1 61.9μs 70.5ns 264ns 0.0924 0 0 6.95 KB
master AllCycleSimpleBody net472 48.8μs 70.2ns 272ns 1.31 0 0 8.34 KB
master AllCycleMoreComplexBody net6.0 78.3μs 116ns 449ns 0.116 0 0 9.51 KB
master AllCycleMoreComplexBody netcoreapp3.1 69.1μs 93ns 360ns 0.14 0 0 10.36 KB
master AllCycleMoreComplexBody net472 55.5μs 58.9ns 220ns 1.88 0.0277 0 11.85 KB
master ObjectExtractorSimpleBody net6.0 142ns 0.131ns 0.491ns 0.00394 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 242ns 0.554ns 2.07ns 0.00365 0 0 272 B
master ObjectExtractorSimpleBody net472 167ns 0.123ns 0.475ns 0.0446 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 2.96μs 1.86ns 6.95ns 0.0534 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 4.17μs 1.68ns 6.51ns 0.05 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 3.75μs 2.37ns 8.87ns 0.602 0.00568 0 3.8 KB
#5913 AllCycleSimpleBody net6.0 73.1μs 96.3ns 373ns 0.0733 0 0 6.01 KB
#5913 AllCycleSimpleBody netcoreapp3.1 63.4μs 125ns 483ns 0.0635 0 0 6.95 KB
#5913 AllCycleSimpleBody net472 48.8μs 104ns 404ns 1.32 0 0 8.34 KB
#5913 AllCycleMoreComplexBody net6.0 79.3μs 103ns 401ns 0.118 0 0 9.51 KB
#5913 AllCycleMoreComplexBody netcoreapp3.1 70.1μs 107ns 414ns 0.14 0 0 10.36 KB
#5913 AllCycleMoreComplexBody net472 56μs 46.4ns 174ns 1.87 0.028 0 11.85 KB
#5913 ObjectExtractorSimpleBody net6.0 189ns 0.072ns 0.269ns 0.00395 0 0 280 B
#5913 ObjectExtractorSimpleBody netcoreapp3.1 196ns 0.123ns 0.461ns 0.00375 0 0 272 B
#5913 ObjectExtractorSimpleBody net472 166ns 0.158ns 0.611ns 0.0446 0 0 281 B
#5913 ObjectExtractorMoreComplexBody net6.0 3.08μs 2.02ns 7.84ns 0.0538 0 0 3.78 KB
#5913 ObjectExtractorMoreComplexBody netcoreapp3.1 3.9μs 2.82ns 10.6ns 0.0509 0 0 3.69 KB
#5913 ObjectExtractorMoreComplexBody net472 3.8μs 2.14ns 8.29ns 0.602 0.0057 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 36.7μs 16ns 62.1ns 0.454 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 53.8μs 13.2ns 47.6ns 0.431 0 0 32.4 KB
master EncodeArgs net472 66.1μs 65.9ns 255ns 5.13 0.0658 0 32.5 KB
master EncodeLegacyArgs net6.0 71.3μs 43.7ns 151ns 0 0 0 2.14 KB
master EncodeLegacyArgs netcoreapp3.1 108μs 313ns 1.21μs 0 0 0 2.14 KB
master EncodeLegacyArgs net472 151μs 59.2ns 229ns 0.302 0 0 2.15 KB
#5913 EncodeArgs net6.0 37.2μs 30.4ns 118ns 0.445 0 0 32.4 KB
#5913 EncodeArgs netcoreapp3.1 54.5μs 34.8ns 135ns 0.434 0 0 32.4 KB
#5913 EncodeArgs net472 65.1μs 27.4ns 106ns 5.14 0.065 0 32.5 KB
#5913 EncodeLegacyArgs net6.0 72.7μs 37.1ns 144ns 0 0 0 2.14 KB
#5913 EncodeLegacyArgs netcoreapp3.1 102μs 45.4ns 170ns 0 0 0 2.14 KB
#5913 EncodeLegacyArgs net472 151μs 59.7ns 231ns 0.301 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 134ns 519ns 0 0 0 2.42 KB
master RunWafRealisticBenchmark netcoreapp3.1 196μs 122ns 442ns 0 0 0 2.37 KB
master RunWafRealisticBenchmark net472 209μs 91.6ns 355ns 0.312 0 0 2.43 KB
master RunWafRealisticBenchmarkWithAttack net6.0 122μs 27.2ns 98ns 0 0 0 1.46 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 131μs 70.8ns 274ns 0 0 0 1.45 KB
master RunWafRealisticBenchmarkWithAttack net472 139μs 30.1ns 113ns 0.208 0 0 1.48 KB
#5913 RunWafRealisticBenchmark net6.0 184μs 68.1ns 264ns 0 0 0 2.42 KB
#5913 RunWafRealisticBenchmark netcoreapp3.1 195μs 106ns 410ns 0 0 0 2.37 KB
#5913 RunWafRealisticBenchmark net472 208μs 55.1ns 206ns 0.312 0 0 2.43 KB
#5913 RunWafRealisticBenchmarkWithAttack net6.0 121μs 26.3ns 102ns 0 0 0 1.46 KB
#5913 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 131μs 143ns 554ns 0 0 0 1.45 KB
#5913 RunWafRealisticBenchmarkWithAttack net472 139μs 49ns 190ns 0.208 0 0 1.48 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5913

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 254.75 KB 256.82 KB 2.06 KB 0.81%

Fewer allocations 🎉 in #5913

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 59.07 KB 58.57 KB -504 B -0.85%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 61.1μs 528ns 5.06μs 0 0 0 43.44 KB
master StringConcatBenchmark netcoreapp3.1 61.9μs 750ns 7.5μs 0 0 0 42.64 KB
master StringConcatBenchmark net472 37μs 44.7ns 161ns 0 0 0 59.07 KB
master StringConcatAspectBenchmark net6.0 310μs 1.67μs 12.9μs 0 0 0 254.75 KB
master StringConcatAspectBenchmark netcoreapp3.1 328μs 1.85μs 12.4μs 0 0 0 252.77 KB
master StringConcatAspectBenchmark net472 281μs 6.39μs 62.2μs 0 0 0 278.53 KB
#5913 StringConcatBenchmark net6.0 60.4μs 701ns 6.94μs 0 0 0 43.44 KB
#5913 StringConcatBenchmark netcoreapp3.1 55.7μs 344ns 3.32μs 0 0 0 42.64 KB
#5913 StringConcatBenchmark net472 37.4μs 137ns 514ns 0 0 0 58.57 KB
#5913 StringConcatAspectBenchmark net6.0 317μs 1.44μs 10.7μs 0 0 0 256.82 KB
#5913 StringConcatAspectBenchmark netcoreapp3.1 328μs 1.64μs 10.3μs 0 0 0 252.57 KB
#5913 StringConcatAspectBenchmark net472 302μs 7.7μs 75.9μs 0 0 0 278.53 KB

@andrewlock
Copy link
Member

andrewlock commented Aug 16, 2024

Benchmarks Report for tracer 🐌

Benchmarks for #5913 compared to master:

  • 3 benchmarks are faster, with geometric mean 1.154
  • 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.73μs 40.4ns 274ns 0.0123 0.0041 0 5.43 KB
master StartStopWithChild netcoreapp3.1 10.3μs 56.8ns 321ns 0.0192 0.00961 0 5.62 KB
master StartStopWithChild net472 16μs 47.4ns 184ns 1.02 0.308 0.0915 6.06 KB
#5913 StartStopWithChild net6.0 7.85μs 44.3ns 349ns 0.0169 0.00422 0 5.42 KB
#5913 StartStopWithChild netcoreapp3.1 9.92μs 52.6ns 268ns 0.0148 0.00495 0 5.61 KB
#5913 StartStopWithChild net472 15.9μs 54.6ns 211ns 1.04 0.335 0.101 6.07 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 455μs 118ns 441ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 631μs 218ns 846ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 833μs 711ns 2.75μs 0.417 0 0 3.3 KB
#5913 WriteAndFlushEnrichedTraces net6.0 462μs 238ns 920ns 0 0 0 2.7 KB
#5913 WriteAndFlushEnrichedTraces netcoreapp3.1 631μs 319ns 1.24μs 0 0 0 2.7 KB
#5913 WriteAndFlushEnrichedTraces net472 838μs 431ns 1.61μs 0.419 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 191μs 1.03μs 6.12μs 0.197 0 0 18.45 KB
master SendRequest netcoreapp3.1 217μs 1.21μs 9.06μs 0.208 0 0 20.61 KB
master SendRequest net472 0.00202ns 0.000754ns 0.00292ns 0 0 0 0 b
#5913 SendRequest net6.0 188μs 1.02μs 5.5μs 0.18 0 0 18.45 KB
#5913 SendRequest netcoreapp3.1 214μs 1.19μs 7.59μs 0.202 0 0 20.61 KB
#5913 SendRequest net472 0.00046ns 0.000368ns 0.00142ns 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 571μs 2.51μs 12μs 0.551 0 0 41.69 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 682μs 3.72μs 20.7μs 0.327 0 0 41.75 KB
master WriteAndFlushEnrichedTraces net472 864μs 3.92μs 15.2μs 8.19 2.59 0.431 53.26 KB
#5913 WriteAndFlushEnrichedTraces net6.0 577μs 2.74μs 11μs 0.563 0 0 41.62 KB
#5913 WriteAndFlushEnrichedTraces netcoreapp3.1 669μs 3.56μs 18.5μs 0.324 0 0 41.79 KB
#5913 WriteAndFlushEnrichedTraces net472 873μs 3.42μs 13.3μs 8.13 2.57 0.428 53.3 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.24μs 0.957ns 3.71ns 0.0142 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.65μs 0.997ns 3.86ns 0.0138 0 0 1.02 KB
master ExecuteNonQuery net472 2μs 2.16ns 8.38ns 0.156 0 0 987 B
#5913 ExecuteNonQuery net6.0 1.24μs 1.23ns 4.75ns 0.0143 0 0 1.02 KB
#5913 ExecuteNonQuery netcoreapp3.1 1.67μs 1.22ns 4.55ns 0.0134 0 0 1.02 KB
#5913 ExecuteNonQuery net472 2μs 2.26ns 8.76ns 0.156 0 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5913

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync‑net6.0 1.190 1,421.00 1,194.29

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.21μs 0.824ns 3.19ns 0.0133 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.5μs 0.716ns 2.68ns 0.0127 0 0 976 B
master CallElasticsearch net472 2.42μs 1.1ns 4.24ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.42μs 0.934ns 3.62ns 0.0136 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.69μs 0.684ns 2.56ns 0.0134 0 0 1.02 KB
master CallElasticsearchAsync net472 2.57μs 1.83ns 6.84ns 0.167 0 0 1.05 KB
#5913 CallElasticsearch net6.0 1.26μs 0.863ns 3.34ns 0.0133 0 0 976 B
#5913 CallElasticsearch netcoreapp3.1 1.54μs 0.689ns 2.67ns 0.0134 0 0 976 B
#5913 CallElasticsearch net472 2.53μs 1.55ns 6ns 0.158 0.00124 0 995 B
#5913 CallElasticsearchAsync net6.0 1.19μs 0.703ns 2.63ns 0.013 0 0 952 B
#5913 CallElasticsearchAsync netcoreapp3.1 1.58μs 1.22ns 4.55ns 0.0134 0 0 1.02 KB
#5913 CallElasticsearchAsync net472 2.64μs 2.02ns 7.83ns 0.167 0.00131 0 1.05 KB
Benchmarks.Trace.GraphQLBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5913

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0 1.120 1,320.28 1,178.64

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.32μs 0.442ns 1.59ns 0.0133 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.71μs 1.52ns 5.89ns 0.0128 0 0 952 B
master ExecuteAsync net472 1.8μs 0.931ns 3.48ns 0.145 0 0 915 B
#5913 ExecuteAsync net6.0 1.18μs 0.876ns 3.28ns 0.013 0 0 952 B
#5913 ExecuteAsync netcoreapp3.1 1.6μs 0.568ns 2.2ns 0.0122 0 0 952 B
#5913 ExecuteAsync net472 1.76μs 0.232ns 0.836ns 0.145 0.000879 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.2μs 2.11ns 8.15ns 0.0296 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.1μs 2.12ns 7.93ns 0.0356 0 0 2.76 KB
master SendAsync net472 7.86μs 2.11ns 8.18ns 0.497 0 0 3.15 KB
#5913 SendAsync net6.0 4.23μs 2ns 7.2ns 0.0296 0 0 2.22 KB
#5913 SendAsync netcoreapp3.1 5.08μs 1.83ns 7.07ns 0.0357 0 0 2.76 KB
#5913 SendAsync net472 7.74μs 2.58ns 10ns 0.499 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.63μs 0.621ns 2.32ns 0.0228 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.26μs 0.504ns 1.82ns 0.0226 0 0 1.64 KB
master EnrichedLog net472 2.58μs 1.88ns 7.03ns 0.249 0 0 1.57 KB
#5913 EnrichedLog net6.0 1.48μs 0.871ns 3.14ns 0.0229 0 0 1.64 KB
#5913 EnrichedLog netcoreapp3.1 2.39μs 1ns 3.89ns 0.0216 0 0 1.64 KB
#5913 EnrichedLog net472 2.6μs 1.22ns 4.57ns 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 113μs 113ns 424ns 0.0569 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 120μs 295ns 1.14μs 0.0594 0 0 4.28 KB
master EnrichedLog net472 148μs 94.7ns 354ns 0.672 0.224 0 4.46 KB
#5913 EnrichedLog net6.0 114μs 171ns 638ns 0.0581 0 0 4.28 KB
#5913 EnrichedLog netcoreapp3.1 120μs 507ns 1.9μs 0.0578 0 0 4.28 KB
#5913 EnrichedLog net472 149μs 269ns 1.04μs 0.667 0.222 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.11μs 0.914ns 3.3ns 0.0312 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.25μs 0.892ns 3.34ns 0.0297 0 0 2.2 KB
master EnrichedLog net472 4.9μs 1.79ns 6.69ns 0.32 0 0 2.02 KB
#5913 EnrichedLog net6.0 3.07μs 0.773ns 2.99ns 0.0307 0 0 2.2 KB
#5913 EnrichedLog netcoreapp3.1 4.15μs 1.63ns 5.88ns 0.029 0 0 2.2 KB
#5913 EnrichedLog net472 4.75μs 3.11ns 12ns 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.38μs 0.839ns 3.25ns 0.0159 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.72μs 0.789ns 2.95ns 0.0155 0 0 1.14 KB
master SendReceive net472 2.08μs 1.85ns 6.92ns 0.183 0 0 1.16 KB
#5913 SendReceive net6.0 1.44μs 1.01ns 3.92ns 0.0158 0 0 1.14 KB
#5913 SendReceive netcoreapp3.1 1.78μs 0.715ns 2.58ns 0.0152 0 0 1.14 KB
#5913 SendReceive net472 2.25μs 1.74ns 6.75ns 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.86μs 0.894ns 3.46ns 0.0215 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.89μs 1.7ns 6.58ns 0.0214 0 0 1.65 KB
master EnrichedLog net472 4.4μs 1.83ns 7.1ns 0.323 0 0 2.04 KB
#5913 EnrichedLog net6.0 2.73μs 0.864ns 3.35ns 0.0219 0 0 1.6 KB
#5913 EnrichedLog netcoreapp3.1 3.96μs 1.41ns 5.28ns 0.0217 0 0 1.65 KB
#5913 EnrichedLog net472 4.29μs 1.41ns 5.26ns 0.322 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5913

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.154 820.55 711.28

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 403ns 0.297ns 1.15ns 0.00808 0 0 576 B
master StartFinishSpan netcoreapp3.1 587ns 1.77ns 6.85ns 0.00761 0 0 576 B
master StartFinishSpan net472 650ns 0.664ns 2.57ns 0.0917 0 0 578 B
master StartFinishScope net6.0 544ns 0.352ns 1.36ns 0.00978 0 0 696 B
master StartFinishScope netcoreapp3.1 820ns 0.41ns 1.53ns 0.00942 0 0 696 B
master StartFinishScope net472 842ns 0.458ns 1.65ns 0.104 0 0 658 B
#5913 StartFinishSpan net6.0 399ns 1.74ns 6.74ns 0.00807 0 0 576 B
#5913 StartFinishSpan netcoreapp3.1 555ns 0.313ns 1.21ns 0.00779 0 0 576 B
#5913 StartFinishSpan net472 594ns 0.457ns 1.77ns 0.0916 0 0 578 B
#5913 StartFinishScope net6.0 489ns 0.322ns 1.25ns 0.00978 0 0 696 B
#5913 StartFinishScope netcoreapp3.1 711ns 0.133ns 0.481ns 0.00956 0 0 696 B
#5913 StartFinishScope net472 861ns 1.28ns 4.96ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 662ns 0.521ns 2.02ns 0.00956 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 948ns 0.482ns 1.87ns 0.00946 0 0 696 B
master RunOnMethodBegin net472 1.17μs 0.853ns 3.3ns 0.104 0 0 658 B
#5913 RunOnMethodBegin net6.0 718ns 0.501ns 1.94ns 0.00984 0 0 696 B
#5913 RunOnMethodBegin netcoreapp3.1 886ns 2.23ns 8.33ns 0.00933 0 0 696 B
#5913 RunOnMethodBegin net472 1.13μs 0.322ns 1.25ns 0.104 0 0 658 B

@daniel-romano-DD daniel-romano-DD changed the title [IAST] CallSite undefined generics support [IAST] CallSite with generics support Aug 21, 2024
@andrewlock
Copy link
Member

andrewlock commented Aug 21, 2024

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 (5913) (11.859M)   : 0, 11859050
    master (11.606M)   : 0, 11606357
    benchmarks/2.9.0 (11.883M)   : 0, 11882814

    section Automatic
    This PR (5913) (7.736M)   : 0, 7735768
    master (7.756M)   : 0, 7755721
    benchmarks/2.9.0 (8.446M)   : 0, 8445613

    section Trace stats
    master (8.110M)   : 0, 8109885

    section Manual
    master (11.466M)   : 0, 11465663

    section Manual + Automatic
    This PR (5913) (7.169M)   : 0, 7169172
    master (7.192M)   : 0, 7192253

    section DD_TRACE_ENABLED=0
    master (10.665M)   : 0, 10665360

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5913) (9.686M)   : 0, 9685827
    benchmarks/2.9.0 (9.760M)   : 0, 9759697

    section Automatic
    This PR (5913) (6.711M)   : 0, 6711300

    section Manual + Automatic
    This PR (5913) (6.224M)   : 0, 6223690

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5913) (10.183M)   : 0, 10183003
    master (10.088M)   : 0, 10088205

    section Automatic
    This PR (5913) (6.856M)   : 0, 6855877
    master (6.620M)   : 0, 6619922

    section Trace stats
    master (7.305M)   : 0, 7304619

    section Manual
    master (9.981M)   : 0, 9980905

    section Manual + Automatic
    This PR (5913) (6.496M)   : 0, 6496428
    master (6.066M)   : 0, 6066177

    section DD_TRACE_ENABLED=0
    master (9.345M)   : 0, 9344849

Loading

@daniel-romano-DD daniel-romano-DD marked this pull request as ready for review August 22, 2024 10:16
@daniel-romano-DD daniel-romano-DD requested review from a team as code owners August 22, 2024 10:16
@@ -43,100 +43,4 @@ std::vector<VulnerabilityType> ParseVulnerabilityTypes(const std::string& txt)
}
return res;
}

SpotInfo::SpotInfo(int line, Aspect* aspect, int id)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why all this code can be removed? Wasn't it being used before, or does it relate to a change in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was not used (legacy code), and this PR made necessary to touch some of those functions, so I preferred to eliminate it.

@@ -68,7 +68,7 @@ public void GivenStringConcatOperations_WhenPerformed_ResultIsOK()
FormatTainted(String.Concat((object)" dummy ", null, testString2, (object)" dummy ")).Should().Be(" dummy :+-abc-+: dummy ");
}

[Fact(Skip = "Aspect disabled until undefined generics are supported")]
[Fact]
Copy link
Member

Choose a reason for hiding this comment

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

nice :)

Copy link
Member

@robertpi robertpi left a comment

Choose a reason for hiding this comment

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

Managed bit look good, can we get someone with better C++ to review as well?

@daniel-romano-DD daniel-romano-DD force-pushed the dani/iast/callsite_generics branch 3 times, most recently from 1f62d4d to ededd84 Compare August 23, 2024 07:34
Copy link
Collaborator

@kevingosse kevingosse left a comment

Choose a reason for hiding this comment

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

I trust you on the removed spot stuff 🙈

if (ns.Length > 0) { name = ns + "." + name; }
if (namedType.TypeArguments.Length > 0)
{
name = $"{name}`{namedType.TypeArguments.Length}<{string.Join(",", namedType.TypeArguments.Select(a => GetFullName(a, false)))}>";
Copy link
Collaborator

Choose a reason for hiding this comment

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

The notation is System.Threading.Tasks.Task`1[System.Int32], not System.Threading.Tasks.Task`1<System.Int32>

var valuesConverted = values as IEnumerable<object?>;

if (values is IEnumerable valuesEnumerable)
var result = target!.AppendJoin(separator, values!);
Copy link
Collaborator

Choose a reason for hiding this comment

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

That was the case before, but I'm confused why StringBuilder is marked as nullable in the signature, but then you immediately use !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the string builder is null the original call is going to fail (as intended) and that code will never execute

@@ -206,6 +206,10 @@ namespace iast
{
return _id;
}
SignatureInfo* MethodSpec::GetMethodSpecSignature()
{
return MemberRefInfo::GetSignature();
Copy link
Collaborator

Choose a reason for hiding this comment

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

That really looks like a static method call. God, C++ is so confusing

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, multiple inheritance shenanigans...

@daniel-romano-DD daniel-romano-DD merged commit 95e891a into master Aug 23, 2024
73 of 75 checks passed
@daniel-romano-DD daniel-romano-DD deleted the dani/iast/callsite_generics branch August 23, 2024 15:39
@github-actions github-actions bot added this to the vNext-v3 milestone Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants