Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[IAST] Support for specifying aspect min version #5931

Merged
merged 6 commits into from
Aug 23, 2024
Merged

[IAST] Support for specifying aspect min version #5931

merged 6 commits into from
Aug 23, 2024

Conversation

daniel-romano-DD
Copy link
Contributor

@daniel-romano-DD daniel-romano-DD commented Aug 22, 2024 •
edited
Loading

Summary of changes

Added a mechanism by which an aspect or aspect class can define a minimum native version in order to be applied.

Reason for change

Some aspects defined under new features (like aspects for struct methods, or aspects with generics) can break old native implementations, so, we need a way to avoid those to be sent or applied.

Implementation details

Created FromVersionAttribute versions of all Aspect attributes. These inherit from the original attribute adding a minimum version of the native library from which the aspect will be applied. This version is then added in the end of the aspect line, in a way that makes old native tracers ignore those lines, as it breaks the old specification.
For new tracers who are able to read that version info, a version comparison will determine if that line is accepted or dumped, allowing us to fine tune the aspects set applied.

This aspect attribute which states that this aspect will be only applied if a native tracer with version 3.2.0 or bigger is found
[AspectMethodReplaceFromVersion("3.2.0", "System.String::Concat(System.Collections.Generic.IEnumerable)")]
will generate this aspect line
[AspectMethodReplace(\"System.String::Concat(System.Collections.Generic.IEnumerable)\",\"\",[0],[False],[None],Default,[]);V3.2.0] Concat(System.Collections.Generic.IEnumerable)

Notice that the version goes in the end, and that's because old tracers while parsing the line were looking for the text )]
This will make old unprepared tracers to automatically ditch the line, whereas the new one implemented from this PR, will be able to retrieve, parse and decide what to do with that version info.

Test coverage

Source generator tests have been written to ensure the correct use and aspect generation of version flavor aspect attributes.

Other details

@andrewlock
Copy link
Member

andrewlock commented Aug 22, 2024 •
edited
Loading

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

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

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

@daniel-romano-DD daniel-romano-DD changed the title [IAST] Support for specifying asepect min version [IAST] Support for specifying aspect min version Aug 22, 2024
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Aug 22, 2024 •
edited
Loading

Datadog Report

Branch report: dani/iast/callsite_min_version
Commit report: e912347
Test service: dd-trace-dotnet

✅ 0 Failed, 405439 Passed, 3143 Skipped, 35h 51m 46.17s Total Time

@andrewlock
Copy link
Member

andrewlock commented Aug 23, 2024 •
edited
Loading

Benchmarks Report for appsec 🐌

Benchmarks for #5931 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.159
  • 1 benchmarks are slower, with geometric mean 1.191
  • 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.Asm.AppSecBodyBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5931

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472 1.191 3,744.84 4,460.68

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
#5931 AllCycleSimpleBody net6.0 73.5μs 85.2ns 330ns 0.073 0 0 6.01 KB
#5931 AllCycleSimpleBody netcoreapp3.1 63.9μs 58.3ns 218ns 0.0954 0 0 6.95 KB
#5931 AllCycleSimpleBody net472 49.1μs 54.6ns 211ns 1.3 0 0 8.33 KB
#5931 AllCycleMoreComplexBody net6.0 78.7μs 110ns 428ns 0.117 0 0 9.51 KB
#5931 AllCycleMoreComplexBody netcoreapp3.1 71.1μs 74.7ns 289ns 0.106 0 0 10.36 KB
#5931 AllCycleMoreComplexBody net472 55.8μs 77.6ns 301ns 1.87 0.0279 0 11.85 KB
#5931 ObjectExtractorSimpleBody net6.0 147ns 0.0885ns 0.343ns 0.00391 0 0 280 B
#5931 ObjectExtractorSimpleBody netcoreapp3.1 258ns 0.207ns 0.776ns 0.0038 0 0 272 B
#5931 ObjectExtractorSimpleBody net472 165ns 0.2ns 0.776ns 0.0446 0 0 281 B
#5931 ObjectExtractorMoreComplexBody net6.0 3.04μs 2.17ns 8.42ns 0.0533 0 0 3.78 KB
#5931 ObjectExtractorMoreComplexBody netcoreapp3.1 4.03μs 1.46ns 5.47ns 0.0505 0 0 3.69 KB
#5931 ObjectExtractorMoreComplexBody net472 4.47μs 5.23ns 20.3ns 0.603 0.00667 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
#5931 EncodeArgs net6.0 37.1μs 23.4ns 90.8ns 0.447 0 0 32.4 KB
#5931 EncodeArgs netcoreapp3.1 55.8μs 19.1ns 71.4ns 0.443 0 0 32.4 KB
#5931 EncodeArgs net472 65.2μs 37.7ns 146ns 5.16 0.0649 0 32.5 KB
#5931 EncodeLegacyArgs net6.0 76.4μs 423ns 2.64μs 0 0 0 2.14 KB
#5931 EncodeLegacyArgs netcoreapp3.1 105μs 151ns 586ns 0 0 0 2.14 KB
#5931 EncodeLegacyArgs net472 153μs 97.3ns 377ns 0.309 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
#5931 RunWafRealisticBenchmark net6.0 186μs 142ns 549ns 0 0 0 2.42 KB
#5931 RunWafRealisticBenchmark netcoreapp3.1 196μs 97.7ns 352ns 0 0 0 2.37 KB
#5931 RunWafRealisticBenchmark net472 208μs 42.1ns 152ns 0.312 0 0 2.43 KB
#5931 RunWafRealisticBenchmarkWithAttack net6.0 122μs 53.3ns 206ns 0 0 0 1.46 KB
#5931 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 129μs 121ns 469ns 0 0 0 1.45 KB
#5931 RunWafRealisticBenchmarkWithAttack net472 139μs 22.3ns 80.5ns 0.209 0 0 1.48 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5931

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 1.159 62,400.00 53,850.00 multimodal

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
#5931 StringConcatBenchmark net6.0 59.6μs 741ns 7.41μs 0 0 0 43.44 KB
#5931 StringConcatBenchmark netcoreapp3.1 53.6μs 210ns 786ns 0 0 0 42.64 KB
#5931 StringConcatBenchmark net472 37.6μs 143ns 517ns 0 0 0 59.27 KB
#5931 StringConcatAspectBenchmark net6.0 303μs 1.66μs 10.1μs 0 0 0 254.35 KB
#5931 StringConcatAspectBenchmark netcoreapp3.1 331μs 1.69μs 9.25μs 0 0 0 252.9 KB
#5931 StringConcatAspectBenchmark net472 294μs 7.56μs 73.7μs 0 0 0 278.53 KB

@andrewlock
Copy link
Member

andrewlock commented Aug 23, 2024 •
edited
Loading

Benchmarks Report for tracer 🐌

Benchmarks for #5931 compared to master:

  • 4 benchmarks are faster, with geometric mean 1.126
  • 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
#5931 StartStopWithChild net6.0 7.78μs 43.7ns 306ns 0.0181 0.00724 0 5.43 KB
#5931 StartStopWithChild netcoreapp3.1 10.4μs 56.1ns 337ns 0.0205 0.0103 0 5.61 KB
#5931 StartStopWithChild net472 16.2μs 62.3ns 241ns 1.02 0.308 0.103 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
#5931 WriteAndFlushEnrichedTraces net6.0 477μs 254ns 949ns 0 0 0 2.7 KB
#5931 WriteAndFlushEnrichedTraces netcoreapp3.1 644μs 469ns 1.82μs 0 0 0 2.7 KB
#5931 WriteAndFlushEnrichedTraces net472 842μs 561ns 2.17μ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
#5931 SendRequest net6.0 194μs 1.12μs 9.13μs 0.183 0 0 18.45 KB
#5931 SendRequest netcoreapp3.1 210μs 1.15μs 6.78μs 0.214 0 0 20.61 KB
#5931 SendRequest net472 0.00139ns 0.000589ns 0.00228ns 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
#5931 WriteAndFlushEnrichedTraces net6.0 559μs 2.01μs 9.43μs 0.556 0 0 41.69 KB
#5931 WriteAndFlushEnrichedTraces netcoreapp3.1 675μs 2.63μs 10.2μs 0.345 0 0 41.76 KB
#5931 WriteAndFlushEnrichedTraces net472 864μs 3.19μs 11.9μs 8.13 2.57 0.428 53.31 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
#5931 ExecuteNonQuery net6.0 1.24μs 1.26ns 4.89ns 0.0144 0 0 1.02 KB
#5931 ExecuteNonQuery netcoreapp3.1 1.7μs 0.935ns 3.62ns 0.0134 0 0 1.02 KB
#5931 ExecuteNonQuery net472 2.03μs 3.71ns 14.4ns 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.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
#5931 CallElasticsearch net6.0 1.26μs 1.18ns 4.58ns 0.0132 0 0 976 B
#5931 CallElasticsearch netcoreapp3.1 1.48μs 0.671ns 2.42ns 0.0133 0 0 976 B
#5931 CallElasticsearch net472 2.41μs 0.917ns 3.31ns 0.158 0 0 995 B
#5931 CallElasticsearchAsync net6.0 1.28μs 0.832ns 2.88ns 0.0131 0 0 952 B
#5931 CallElasticsearchAsync netcoreapp3.1 1.62μs 5.23ns 20.3ns 0.0138 0 0 1.02 KB
#5931 CallElasticsearchAsync net472 2.67μs 1.23ns 4.42ns 0.167 0.00134 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.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
#5931 ExecuteAsync net6.0 1.2μs 0.622ns 2.33ns 0.0132 0 0 952 B
#5931 ExecuteAsync netcoreapp3.1 1.56μs 0.868ns 3.13ns 0.0126 0 0 952 B
#5931 ExecuteAsync net472 1.72μs 0.986ns 3.82ns 0.145 0.00086 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
#5931 SendAsync net6.0 4.1μs 0.813ns 3.04ns 0.0306 0 0 2.22 KB
#5931 SendAsync netcoreapp3.1 5.08μs 2.16ns 8.09ns 0.038 0 0 2.76 KB
#5931 SendAsync net472 7.74μs 2.29ns 8.24ns 0.498 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
#5931 EnrichedLog net6.0 1.58μs 0.495ns 1.92ns 0.0231 0 0 1.64 KB
#5931 EnrichedLog netcoreapp3.1 2.18μs 1.4ns 5.24ns 0.0219 0 0 1.64 KB
#5931 EnrichedLog net472 2.63μs 2.7ns 10.1ns 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 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
#5931 EnrichedLog net6.0 116μs 253ns 978ns 0.0584 0 0 4.28 KB
#5931 EnrichedLog netcoreapp3.1 120μs 275ns 1.07μs 0.0597 0 0 4.28 KB
#5931 EnrichedLog net472 149μs 233ns 901ns 0.674 0.225 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
#5931 EnrichedLog net6.0 3.03μs 0.813ns 2.82ns 0.0302 0 0 2.2 KB
#5931 EnrichedLog netcoreapp3.1 4.1μs 1.52ns 5.67ns 0.0288 0 0 2.2 KB
#5931 EnrichedLog net472 4.92μs 5.1ns 19.8ns 0.319 0 0 2.02 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.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
#5931 SendReceive net6.0 1.33μs 1.31ns 5.08ns 0.0161 0 0 1.14 KB
#5931 SendReceive netcoreapp3.1 1.7μs 1.1ns 4.26ns 0.0153 0 0 1.14 KB
#5931 SendReceive net472 2.17μs 1.26ns 4.54ns 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
#5931 EnrichedLog net6.0 2.74μs 0.735ns 2.85ns 0.0219 0 0 1.6 KB
#5931 EnrichedLog netcoreapp3.1 3.83μs 2.27ns 8.79ns 0.0212 0 0 1.65 KB
#5931 EnrichedLog net472 4.32μs 1.91ns 7.14ns 0.322 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5931

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0 1.126 543.93 482.96
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.123 820.55 730.90
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472 1.117 650.93 582.60

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
#5931 StartFinishSpan net6.0 398ns 0.389ns 1.5ns 0.00803 0 0 576 B
#5931 StartFinishSpan netcoreapp3.1 573ns 0.88ns 3.41ns 0.00786 0 0 576 B
#5931 StartFinishSpan net472 582ns 0.681ns 2.64ns 0.0916 0 0 578 B
#5931 StartFinishScope net6.0 482ns 0.387ns 1.5ns 0.00981 0 0 696 B
#5931 StartFinishScope netcoreapp3.1 732ns 0.7ns 2.71ns 0.00917 0 0 696 B
#5931 StartFinishScope net472 839ns 1.03ns 3.98ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5931

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net472 1.139 1,168.62 1,026.14

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
#5931 RunOnMethodBegin net6.0 599ns 1.1ns 4.13ns 0.00985 0 0 696 B
#5931 RunOnMethodBegin netcoreapp3.1 945ns 0.691ns 2.68ns 0.00908 0 0 696 B
#5931 RunOnMethodBegin net472 1.03μs 1.02ns 3.95ns 0.104 0 0 658 B

@daniel-romano-DD daniel-romano-DD marked this pull request as ready for review August 23, 2024 07:05
@daniel-romano-DD daniel-romano-DD requested review from a team as code owners August 23, 2024 07:05
robertpi
robertpi reviewed Aug 23, 2024
View reviewed changes
tracer/src/Datadog.Trace.SourceGenerators/Datadog.Trace.SourceGenerators.csproj
<Compile Include="..\Datadog.Trace\Iast\Dataflow\AspectFilter.cs" Link="AspectsDefinitions\Sources\AspectFilter.cs" />
<EmbeddedResource Include="..\Datadog.Trace\Iast\Dataflow\AspectMethodInsertAfterAttribute.cs" Link="AspectsDefinitions\Sources\AspectMethodInsertAfterAttribute.cs" />
<EmbeddedResource Include="..\Datadog.Trace\Iast\Dataflow\AspectMethodInsertBeforeAttribute.cs" Link="AspectsDefinitions\Sources\AspectMethodInsertBeforeAttribute.cs" />
<EmbeddedResource Include="..\Datadog.Trace\Iast\Dataflow\AspectMethodReplaceAttribute.cs" Link="AspectsDefinitions\Sources\AspectMethodReplaceAttribute.cs" />
Copy link
Member

Choose a reason for hiding this comment

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

Why are these removed from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed in the source generator

robertpi
robertpi reviewed Aug 23, 2024
View reviewed changes
tracer/test/test-applications/integrations/Samples.InstrumentedTests/iast.runsettings Outdated
@@ -1,3 +1,17 @@
<?xml version="1.0" encoding="utf-8" ?>
<RunSettings><RunConfiguration><EnvironmentVariables>
<CORECLR_ENABLE_PROFILING>1</CORECLR_ENABLE_PROFILING>
Copy link
Member

Choose a reason for hiding this comment

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

did you mean to commit this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

robertpi
robertpi approved these changes Aug 23, 2024
View reviewed changes
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.

Couple of questions, but looks good!

andrewlock
andrewlock approved these changes Aug 23, 2024
View reviewed changes
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

LGTM, but if I understand correctly, this is only a version conflict problem, right? And it only occurs when the manual version of Datadog.Trace is greater than the native version?

And I believe that can't happen in v3 (manual will never be higher than native). There's no harm in this obviously, and it's future proofing for "who knows what will happen in the future", but it's probably more important this is back ported to v2 🙂

tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp
if (pos1 == std::string::npos)
{
//Check for version limitation
pos1 = IndexOf(line, WStr(");V"), &offset);
Copy link
Member

Choose a reason for hiding this comment

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

Just to be clear, this is the only place ; can be in the string right? So this is definitely going to get our expected position

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's it

tracer/src/Datadog.Tracer.Native/iast/dataflow_aspects.cpp

DataflowAspectClass::DataflowAspectClass(Dataflow* dataflow, const WSTRING& aspectsAssembly, const WSTRING& line)
{
this->_dataflow = dataflow;
//[AspectClassAttribute("mscorlib,netstandard,System.Private.CoreLib",PROPAGATION,"")] Hdiv.AST.Aspects.Aspects.System_StringAspect
Copy link
Member

Choose a reason for hiding this comment

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

nit: it's kind of handy having this example here when reading the logic below, so might be good to keep this + include an example of the "new" one? Same for below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree

@daniel-romano-DD daniel-romano-DD merged commit d54c629 into master Aug 23, 2024
72 of 75 checks passed
@daniel-romano-DD daniel-romano-DD deleted the dani/iast/callsite_min_version branch August 23, 2024 09:32
@github-actions github-actions bot added this to the vNext-v3 milestone Aug 23, 2024
daniel-romano-DD added a commit that referenced this pull request Aug 23, 2024
@daniel-romano-DD
[IAST] Support for specifying aspect min version (#5931)
6c8db45 
## Summary of changes
Added a mechanism by which an aspect or aspect class can define a
minimum native version in order to be applied.

## Reason for change
Some aspects defined under new features (like aspects for struct
methods, or aspects with generics) can break old native implementations,
so, we need a way to avoid those to be sent or applied.

## Implementation details
Created `FromVersionAttribute` versions of all Aspect attributes. These
inherit from the original attribute adding a **minimum version of the
native library** from which the aspect will be applied. This version is
then added in the end of the aspect line, in a way that makes old native
tracers ignore those lines, as it breaks the old specification.
For new tracers who are able to read that version info, a version
comparison will determine if that line is accepted or dumped, allowing
us to fine tune the aspects set applied.

This aspect attribute which states that this aspect will be only applied
if a native tracer with version `3.2.0` or bigger is found
`[AspectMethodReplaceFromVersion("3.2.0",
"System.String::Concat(System.Collections.Generic.IEnumerable)")]`
will generate this aspect line

`[AspectMethodReplace(\"System.String::Concat(System.Collections.Generic.IEnumerable)\",\"\",[0],[False],[None],Default,[]);V3.2.0]
Concat(System.Collections.Generic.IEnumerable)`

Notice that the version goes in the end, and that's because old tracers
while parsing the line were looking for the text `)] `
This will make old unprepared tracers to automatically ditch the line,
whereas the new one implemented from this PR, will be able to retrieve,
parse and decide what to do with that version info.

## Test coverage
Source generator tests have been written to ensure the correct use and
aspect generation of version flavor aspect attributes.

## Other details


<!-- ⚠️ 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. -->
daniel-romano-DD added a commit that referenced this pull request Aug 23, 2024
@daniel-romano-DD
[IAST] Support for specifying aspect min version (#5931) [-> V2] (#5932)
2bd1621 
## Summary of changes
Added a mechanism by which an aspect or aspect class can define a
minimum native version in order to be applied.

## Reason for change
Some aspects defined under new features (like aspects for struct
methods, or aspects with generics) can break old native implementations,
so, we need a way to avoid those to be sent or applied.

## Implementation details
Created `FromVersionAttribute` versions of all Aspect attributes. These
inherit from the original attribute adding a **minimum version of the
native library** from which the aspect will be applied. This version is
then added in the end of the aspect line, in a way that makes old native
tracers ignore those lines, as it breaks the old specification. For new
tracers who are able to read that version info, a version comparison
will determine if that line is accepted or dumped, allowing us to fine
tune the aspects set applied.

This aspect attribute which states that this aspect will be only applied
if a native tracer with version `3.2.0` or bigger is found
`[AspectMethodReplaceFromVersion("3.2.0",
"System.String::Concat(System.Collections.Generic.IEnumerable)")]` will
generate this aspect line


`[AspectMethodReplace(\"System.String::Concat(System.Collections.Generic.IEnumerable)\",\"\",[0],[False],[None],Default,[]);V3.2.0]
Concat(System.Collections.Generic.IEnumerable)`

Notice that the version goes in the end, and that's because old tracers
while parsing the line were looking for the text `)] ` This will make
old unprepared tracers to automatically ditch the line, whereas the new
one implemented from this PR, will be able to retrieve, parse and decide
what to do with that version info.

## Test coverage
Source generator tests have been written to ensure the correct use and
aspect generation of version flavor aspect attributes.

## Other details


<!-- ⚠️ 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. -->

## Summary of changes

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

@andrewlock andrewlock andrewlock approved these changes

@robertpi robertpi robertpi approved these changes

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

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