[ASM] Add event rules version in telemetry #6208

NachoEchevarria · 2024-10-28T13:31:35Z

Summary of changes

In the WAF related telemetry metrics, we are not currently sending the rules version, only the WAF version. All the other libraries already send that and we have been requested to send this metric as well.

For now, the tag is only sent in non-RASP ASM metrics. RASP metrics will include this tag in the near future but the it is still not defined.

Reason for change

Implementation details

Test coverage

Other details

github-actions · 2024-10-28T13:32:26Z

Snapshots difference summary

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

3 occurrences of :

+      _dd.appsec.event_rules.version: 1.13.1,

1 occurrences of :

+      _dd.appsec.event_rules.version: 2.22.222,

1 occurrences of :

+      _dd.appsec.event_rules.version: 3.33.333,

1 occurrences of :

+      _dd.appsec.event_rules.version: 18.18.18,

datadog-ddstaging · 2024-10-28T13:48:10Z

Datadog Report

Branch report: nacho/AddEventRulesVersionTelemetry
Commit report: 454d17a
Test service: dd-trace-dotnet

✅ 0 Failed, 361854 Passed, 2085 Skipped, 14h 58m 38.67s Total Time

andrewlock · 2024-10-28T14:17:27Z

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

    section CallTarget+Inlining+NGEN
    This PR (6208) - mean (1,119ms)  : 1095, 1142
     .   : milestone, 1119,
    master - mean (1,119ms)  : 1098, 1140
     .   : milestone, 1119,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6208) - mean (111ms)  : 107, 115
     .   : milestone, 111,
    master - mean (109ms)  : 107, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (6208) - mean (785ms)  : 767, 803
     .   : milestone, 785,
    master - mean (773ms)  : 753, 794
     .   : milestone, 773,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6208) - mean (94ms)  : 90, 98
     .   : milestone, 94,
    master - mean (93ms)  : 90, 96
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (6208) - mean (732ms)  : 717, 746
     .   : milestone, 732,
    master - mean (733ms)  : 714, 752
     .   : milestone, 733,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6208) - mean (191ms)  : 187, 194
     .   : milestone, 191,
    master - mean (190ms)  : 187, 193
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6208) - mean (1,198ms)  : 1178, 1219
     .   : milestone, 1198,
    master - mean (1,200ms)  : 1167, 1233
     .   : milestone, 1200,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6208) - mean (277ms)  : 272, 281
     .   : milestone, 277,
    master - mean (278ms)  : 271, 284
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (6208) - mean (941ms)  : 924, 958
     .   : milestone, 941,
    master - mean (945ms)  : 923, 967
     .   : milestone, 945,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6208) - mean (265ms)  : 260, 270
     .   : milestone, 265,
    master - mean (265ms)  : 261, 269
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6208) - mean (923ms)  : 904, 942
     .   : milestone, 923,
    master - mean (929ms)  : 911, 948
     .   : milestone, 929,

andrewlock · 2024-10-28T14:43:37Z

Benchmarks Report for appsec 🐌

Benchmarks for #6208 compared to master:

1 benchmarks are faster, with geometric mean 1.194
1 benchmarks are slower, with geometric mean 1.118
2 benchmarks have fewer 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 #6208

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net6.0	1.118	2,953.88	3,301.50

Faster 🎉 in #6208

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472	1.194	4,494.64	3,765.07

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	75.2μs	129ns	498ns	0.0746	0	6.01 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	63μs	80.3ns	300ns	0.0938	0	6.95 KB
master	`AllCycleSimpleBody`	net472	49.9μs	58ns	225ns	1.31	0	8.33 KB
master	`AllCycleMoreComplexBody`	net6.0	79μs	50.2ns	188ns	0.12	0	9.51 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	69.5μs	64.7ns	251ns	0.139	0	10.36 KB
master	`AllCycleMoreComplexBody`	net472	56.9μs	86.4ns	335ns	1.88	0.0285	11.85 KB
master	`ObjectExtractorSimpleBody`	net6.0	145ns	0.179ns	0.692ns	0.00397	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	202ns	0.294ns	1.14ns	0.00367	0	272 B
master	`ObjectExtractorSimpleBody`	net472	167ns	0.0968ns	0.375ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	2.95μs	1.79ns	6.68ns	0.0534	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	4.04μs	1.53ns	5.72ns	0.0505	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	4.5μs	5.33ns	20.6ns	0.602	0.00448	3.8 KB
#6208	`AllCycleSimpleBody`	net6.0	74.5μs	65.4ns	245ns	0.0743	0	6.01 KB
#6208	`AllCycleSimpleBody`	netcoreapp3.1	62μs	53ns	198ns	0.0926	0	6.95 KB
#6208	`AllCycleSimpleBody`	net472	49.7μs	57.4ns	222ns	1.32	0	8.34 KB
#6208	`AllCycleMoreComplexBody`	net6.0	79.8μs	101ns	390ns	0.119	0	9.51 KB
#6208	`AllCycleMoreComplexBody`	netcoreapp3.1	70μs	72.4ns	271ns	0.14	0	10.37 KB
#6208	`AllCycleMoreComplexBody`	net472	55.6μs	122ns	474ns	1.88	0.0277	11.85 KB
#6208	`ObjectExtractorSimpleBody`	net6.0	145ns	0.191ns	0.741ns	0.00395	0	280 B
#6208	`ObjectExtractorSimpleBody`	netcoreapp3.1	206ns	0.393ns	1.52ns	0.00377	0	272 B
#6208	`ObjectExtractorSimpleBody`	net472	167ns	0.181ns	0.675ns	0.0446	0	281 B
#6208	`ObjectExtractorMoreComplexBody`	net6.0	3.3μs	2.29ns	8.26ns	0.0532	0	3.78 KB
#6208	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.91μs	2.02ns	7.54ns	0.0487	0	3.69 KB
#6208	`ObjectExtractorMoreComplexBody`	net472	3.76μs	3.28ns	11.8ns	0.602	0.00563	3.8 KB

Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EncodeArgs`	net6.0	38.4μs	18.6ns	71.9ns	0.46	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	54.2μs	25ns	93.5ns	0.43	0	32.4 KB
master	`EncodeArgs`	net472	65.7μs	45.1ns	175ns	5.13	0.0658	32.5 KB
master	`EncodeLegacyArgs`	net6.0	72μs	96ns	333ns	0.0365	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	106μs	99.4ns	385ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	153μs	93.6ns	363ns	0.306	0	2.15 KB
#6208	`EncodeArgs`	net6.0	37.8μs	23.6ns	91.5ns	0.452	0	32.4 KB
#6208	`EncodeArgs`	netcoreapp3.1	54.3μs	20.2ns	75.5ns	0.432	0	32.4 KB
#6208	`EncodeArgs`	net472	65.9μs	34.3ns	133ns	5.16	0.0658	32.5 KB
#6208	`EncodeLegacyArgs`	net6.0	72.8μs	25.2ns	97.5ns	0	0	2.14 KB
#6208	`EncodeLegacyArgs`	netcoreapp3.1	108μs	386ns	1.5μs	0	0	2.15 KB
#6208	`EncodeLegacyArgs`	net472	155μs	82.1ns	318ns	0.309	0	2.15 KB

Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunWafRealisticBenchmark`	net6.0	184μs	317ns	1.23μs	0	2.44 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	195μs	241ns	901ns	0	2.39 KB
master	`RunWafRealisticBenchmark`	net472	209μs	119ns	461ns	0.311	2.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	72.8ns	282ns	0	1.47 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	129μs	89.6ns	335ns	0	1.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	138μs	48.2ns	167ns	0.207	1.49 KB
#6208	`RunWafRealisticBenchmark`	net6.0	184μs	128ns	479ns	0	2.44 KB
#6208	`RunWafRealisticBenchmark`	netcoreapp3.1	197μs	121ns	451ns	0	2.39 KB
#6208	`RunWafRealisticBenchmark`	net472	209μs	104ns	401ns	0.313	2.46 KB
#6208	`RunWafRealisticBenchmarkWithAttack`	net6.0	123μs	48.9ns	183ns	0	1.47 KB
#6208	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	130μs	135ns	521ns	0	1.46 KB
#6208	`RunWafRealisticBenchmarkWithAttack`	net472	139μs	40.5ns	151ns	0.209	1.49 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #6208

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	59.36 KB	59.01 KB	-352 B	-0.59%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	263.3 KB	253 KB	-10.3 KB	-3.91%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	62.4μs	667ns	6.64μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	53.9μs	287ns	1.49μs	42.64 KB
master	`StringConcatBenchmark`	net472	38.4μs	64.1ns	222ns	59.36 KB
master	`StringConcatAspectBenchmark`	net6.0	309μs	1.7μs	10.7μs	254.19 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	347μs	1.87μs	11.1μs	263.3 KB
master	`StringConcatAspectBenchmark`	net472	288μs	6.44μs	62.5μs	278.53 KB
#6208	`StringConcatBenchmark`	net6.0	59.4μs	728ns	7.2μs	43.44 KB
#6208	`StringConcatBenchmark`	netcoreapp3.1	60.8μs	750ns	7.38μs	42.64 KB
#6208	`StringConcatBenchmark`	net472	37.6μs	88.8ns	320ns	59.01 KB
#6208	`StringConcatAspectBenchmark`	net6.0	318μs	1.77μs	11.8μs	254.76 KB
#6208	`StringConcatAspectBenchmark`	netcoreapp3.1	338μs	1.92μs	15.7μs	253 KB
#6208	`StringConcatAspectBenchmark`	net472	287μs	6.24μs	60.8μs	278.53 KB

andrewlock · 2024-10-28T14:54:57Z

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 (6208) (11.276M)   : 0, 11276193
    master (10.998M)   : 0, 10997755
    benchmarks/2.9.0 (11.081M)   : 0, 11080577

    section Automatic
    This PR (6208) (7.532M)   : 0, 7532073
    master (7.284M)   : 0, 7284408
    benchmarks/2.9.0 (7.732M)   : 0, 7732233

    section Trace stats
    master (7.634M)   : 0, 7633666

    section Manual
    master (11.000M)   : 0, 11000072

    section Manual + Automatic
    This PR (6208) (6.845M)   : 0, 6844693
    master (6.816M)   : 0, 6815738

    section DD_TRACE_ENABLED=0
    master (10.254M)   : 0, 10254139

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6208) (9.714M)   : 0, 9714420
    master (9.323M)   : 0, 9323082
    benchmarks/2.9.0 (9.798M)   : 0, 9798067

    section Automatic
    This PR (6208) (6.478M)   : 0, 6477518
    master (6.301M)   : 0, 6301489

    section Trace stats
    master (6.819M)   : 0, 6818552

    section Manual
    master (9.497M)   : 0, 9496751

    section Manual + Automatic
    This PR (6208) (5.908M)   : 0, 5907787
    master (6.146M)   : 0, 6145780

    section DD_TRACE_ENABLED=0
    master (8.875M)   : 0, 8874896

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6208) (9.915M)   : 0, 9914646
    master (9.897M)   : 0, 9897302
    benchmarks/2.9.0 (10.067M)   : 0, 10067315

    section Automatic
    This PR (6208) (6.427M)   : 0, 6426828
    master (6.508M)   : 0, 6508447
    benchmarks/2.9.0 (7.552M)   : 0, 7552193

    section Trace stats
    master (7.282M)   : 0, 7282022

    section Manual
    master (9.753M)   : 0, 9752972

    section Manual + Automatic
    This PR (6208) (5.957M)   : 0, 5956501
    master (6.109M)   : 0, 6109286

    section DD_TRACE_ENABLED=0
    master (9.327M)   : 0, 9326631

andrewlock · 2024-10-28T15:27:25Z

Benchmarks Report for tracer 🐌

Benchmarks for #6208 compared to master:

1 benchmarks are faster, with geometric mean 1.168
2 benchmarks are slower, with geometric mean 1.141
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.7μs	42.8ns	267ns	0.0162	0.00406	0	5.43 KB
master	`StartStopWithChild`	netcoreapp3.1	9.92μs	53.5ns	288ns	0.0189	0.00943	0	5.61 KB
master	`StartStopWithChild`	net472	16μs	66.6ns	258ns	1.04	0.33	0.0967	6.06 KB
#6208	`StartStopWithChild`	net6.0	7.82μs	43.6ns	283ns	0.0156	0.00778	0	5.42 KB
#6208	`StartStopWithChild`	netcoreapp3.1	9.56μs	48.5ns	237ns	0.0193	0.00964	0	5.62 KB
#6208	`StartStopWithChild`	net472	16.2μs	67.6ns	262ns	1.01	0.293	0.0895	6.06 KB

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	482μs	278ns	1.04μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	637μs	383ns	1.48μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	839μs	253ns	911ns	0.419	3.3 KB
#6208	`WriteAndFlushEnrichedTraces`	net6.0	498μs	371ns	1.39μs	0	2.7 KB
#6208	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	638μs	464ns	1.8μs	0	2.7 KB
#6208	`WriteAndFlushEnrichedTraces`	net472	835μs	523ns	1.96μs	0.417	3.3 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	200μs	1.15μs	9.6μs	0.19	18.45 KB
master	`SendRequest`	netcoreapp3.1	225μs	1.31μs	11.6μs	0.207	20.61 KB
master	`SendRequest`	net472	0.00136ns	0.000644ns	0.00241ns	0	0 b
#6208	`SendRequest`	net6.0	202μs	1.16μs	8.91μs	0.196	18.45 KB
#6208	`SendRequest`	netcoreapp3.1	229μs	1.34μs	12.5μs	0.231	20.61 KB
#6208	`SendRequest`	net472	0.00463ns	0.0017ns	0.00659ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6208

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1	41.64 KB	41.87 KB	237 B	0.57%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	558μs	2.95μs	14.7μs	0.556	0	0	41.45 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	670μs	3.57μs	18.2μs	0.329	0	0	41.64 KB
master	`WriteAndFlushEnrichedTraces`	net472	868μs	2.52μs	9.42μs	8.3	2.62	0.437	53.34 KB
#6208	`WriteAndFlushEnrichedTraces`	net6.0	554μs	2.59μs	10μs	0.539	0	0	41.51 KB
#6208	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	706μs	4.05μs	30.3μs	0.342	0	0	41.87 KB
#6208	`WriteAndFlushEnrichedTraces`	net472	899μs	3.59μs	13.9μs	8.27	2.3	0.46	53.34 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.33μs	1.23ns	4.76ns	0.0145	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.82μs	1.45ns	5.6ns	0.0137	1.02 KB
master	`ExecuteNonQuery`	net472	2.08μs	2.74ns	10.6ns	0.157	987 B
#6208	`ExecuteNonQuery`	net6.0	1.19μs	0.862ns	3.34ns	0.0143	1.02 KB
#6208	`ExecuteNonQuery`	netcoreapp3.1	1.71μs	3.34ns	12.9ns	0.0136	1.02 KB
#6208	`ExecuteNonQuery`	net472	2.06μs	3.02ns	11.7ns	0.156	987 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.22μs	0.585ns	2.19ns	0.014	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.55μs	0.82ns	3.07ns	0.0132	976 B
master	`CallElasticsearch`	net472	2.53μs	2.45ns	9.49ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.23μs	0.381ns	1.43ns	0.0135	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.63μs	0.686ns	2.66ns	0.0139	1.02 KB
master	`CallElasticsearchAsync`	net472	2.59μs	2ns	7.74ns	0.167	1.05 KB
#6208	`CallElasticsearch`	net6.0	1.19μs	0.76ns	2.84ns	0.0137	976 B
#6208	`CallElasticsearch`	netcoreapp3.1	1.57μs	0.744ns	2.79ns	0.0131	976 B
#6208	`CallElasticsearch`	net472	2.47μs	4.73ns	17.7ns	0.157	995 B
#6208	`CallElasticsearchAsync`	net6.0	1.27μs	0.49ns	1.83ns	0.0133	952 B
#6208	`CallElasticsearchAsync`	netcoreapp3.1	1.6μs	0.905ns	3.5ns	0.0136	1.02 KB
#6208	`CallElasticsearchAsync`	net472	2.66μs	5.06ns	19.6ns	0.167	1.05 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.33μs	0.646ns	2.42ns	0.0133	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.71μs	0.61ns	2.28ns	0.0126	952 B
master	`ExecuteAsync`	net472	1.77μs	1.19ns	4.6ns	0.145	915 B
#6208	`ExecuteAsync`	net6.0	1.29μs	1.01ns	3.79ns	0.0135	952 B
#6208	`ExecuteAsync`	netcoreapp3.1	1.6μs	1.39ns	5.2ns	0.0129	952 B
#6208	`ExecuteAsync`	net472	1.73μs	0.444ns	1.72ns	0.145	915 B

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.13μs	1.38ns	5.15ns	0.031	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.08μs	1.79ns	6.93ns	0.0382	2.76 KB
master	`SendAsync`	net472	7.83μs	3.37ns	12.6ns	0.497	3.15 KB
#6208	`SendAsync`	net6.0	4.05μs	1.28ns	4.98ns	0.0325	2.22 KB
#6208	`SendAsync`	netcoreapp3.1	4.98μs	2.23ns	8.65ns	0.0374	2.76 KB
#6208	`SendAsync`	net472	7.84μs	3.67ns	14.2ns	0.496	3.15 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.45μs	0.988ns	3.7ns	0.0233	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.21μs	1.44ns	5.39ns	0.0227	1.64 KB
master	`EnrichedLog`	net472	2.5μs	1.24ns	4.79ns	0.249	1.57 KB
#6208	`EnrichedLog`	net6.0	1.55μs	0.926ns	3.47ns	0.0226	1.64 KB
#6208	`EnrichedLog`	netcoreapp3.1	2.14μs	0.816ns	3.05ns	0.0225	1.64 KB
#6208	`EnrichedLog`	net472	2.6μs	0.482ns	1.74ns	0.249	1.57 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	117μs	162ns	628ns	0	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	122μs	169ns	655ns	0.0622	0	4.28 KB
master	`EnrichedLog`	net472	152μs	241ns	932ns	0.684	0.228	4.46 KB
#6208	`EnrichedLog`	net6.0	118μs	172ns	666ns	0.0585	0	4.28 KB
#6208	`EnrichedLog`	netcoreapp3.1	122μs	217ns	842ns	0.0609	0	4.28 KB
#6208	`EnrichedLog`	net472	153μs	202ns	781ns	0.684	0.228	4.46 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3.02μs	1.18ns	4.57ns	0.0305	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.15μs	1.99ns	7.7ns	0.0292	2.2 KB
master	`EnrichedLog`	net472	4.78μs	1.12ns	3.88ns	0.32	2.02 KB
#6208	`EnrichedLog`	net6.0	3.04μs	1.17ns	4.53ns	0.0304	2.2 KB
#6208	`EnrichedLog`	netcoreapp3.1	4.27μs	1.33ns	5.13ns	0.0297	2.2 KB
#6208	`EnrichedLog`	net472	4.73μs	1.55ns	5.99ns	0.319	2.02 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.3μs	0.596ns	2.31ns	0.016	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.77μs	0.518ns	2.01ns	0.0151	1.14 KB
master	`SendReceive`	net472	2.16μs	1.02ns	3.95ns	0.183	1.16 KB
#6208	`SendReceive`	net6.0	1.26μs	0.61ns	2.36ns	0.0157	1.14 KB
#6208	`SendReceive`	netcoreapp3.1	1.79μs	0.548ns	2.12ns	0.0153	1.14 KB
#6208	`SendReceive`	net472	2.15μs	0.805ns	3.12ns	0.183	1.16 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.82μs	1.1ns	4.28ns	0.0226	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.78μs	1.74ns	6.52ns	0.0209	1.65 KB
master	`EnrichedLog`	net472	4.39μs	1.86ns	6.71ns	0.324	2.04 KB
#6208	`EnrichedLog`	net6.0	2.73μs	0.9ns	3.48ns	0.022	1.6 KB
#6208	`EnrichedLog`	netcoreapp3.1	4.03μs	1.81ns	7.03ns	0.0221	1.65 KB
#6208	`EnrichedLog`	net472	4.3μs	2.07ns	7.74ns	0.323	2.04 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6208

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1	1.144	544.34	622.61
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0	1.138	499.96	569.01

Faster 🎉 in #6208

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.168	471.32	403.36

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	471ns	0.337ns	1.26ns	0.0081	576 B
master	`StartFinishSpan`	netcoreapp3.1	544ns	0.28ns	1.01ns	0.00786	576 B
master	`StartFinishSpan`	net472	662ns	0.729ns	2.82ns	0.0915	578 B
master	`StartFinishScope`	net6.0	500ns	0.388ns	1.5ns	0.0097	696 B
master	`StartFinishScope`	netcoreapp3.1	796ns	3.86ns	15.9ns	0.0095	696 B
master	`StartFinishScope`	net472	909ns	0.415ns	1.55ns	0.104	658 B
#6208	`StartFinishSpan`	net6.0	403ns	0.352ns	1.36ns	0.00808	576 B
#6208	`StartFinishSpan`	netcoreapp3.1	622ns	0.669ns	2.59ns	0.00777	576 B
#6208	`StartFinishSpan`	net472	731ns	0.526ns	2.04ns	0.0916	578 B
#6208	`StartFinishScope`	net6.0	569ns	0.25ns	0.902ns	0.00975	696 B
#6208	`StartFinishScope`	netcoreapp3.1	746ns	0.717ns	2.78ns	0.00924	696 B
#6208	`StartFinishScope`	net472	868ns	0.369ns	1.43ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	669ns	0.569ns	2.2ns	0.00972	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	954ns	0.395ns	1.53ns	0.00913	696 B
master	`RunOnMethodBegin`	net472	1.17μs	0.489ns	1.89ns	0.104	658 B
#6208	`RunOnMethodBegin`	net6.0	682ns	0.28ns	1.08ns	0.00955	696 B
#6208	`RunOnMethodBegin`	netcoreapp3.1	962ns	0.899ns	3.48ns	0.0092	696 B
#6208	`RunOnMethodBegin`	net472	1.13μs	0.983ns	3.81ns	0.104	658 B

andrewlock

I think this highlights a few things:

The existing (pre-this PR) behaviour looks buggy to me - we're overwriting the tags for a bunch of ASM metrics currently AFAICT. The original design assumed that all ASM metrics should have the waf version tagged. That looks fundamentally broken
I'm not a fan of the changes in GetTags() - I think that's piling on the complexity to an already too-complex aspect.

I think we can probably simplify it to so that the entire implementation is more like this:

private string[]? GetTags(string? ns, string[]? metricKeyTags)
{
    if (ns != MetricNamespaceConstants.ASM)
    {
        return metricKeyTags;
    }

    if (metricKeyTags is null)
    {
        return _wafVersionTags ?? _unknownWafVersionTags;
    }

    if (string.Equals(metricKeyTags[0], "waf_version"))
    {
        var wafVersionTag = (_wafVersionTags ?? _unknownWafVersionTags)[0];
        metricKeyTags[0] = wafVersionTag;
    }

    if (metricKeyTags.Length > 1 && string.Equals(metricKeyTags[1], "event_rules_version"))
    {
        var eventRulesTag = (_wafVersionTags ?? _unknownWafVersionTags)[1];
        metricKeyTags[1] = eventRulesTag;
    }

    return metricKeyTags;
}

This solves the existing overwriting issue, and adds support for event_rules. It also means we don't need the additional array fields, we just need

private static readonly string[] _unknownWafVersionTags = { "waf_version:unknown", "event_rules_version:unknown" };
private string[]? _wafVersionTags; // gets updated with both waf and rules version

We should also likely add some unit tests that asserts that:

if a metric uses waf_version then it's always the first element
if a metric uses event_rules_version then it's always the second element
if a metric uses event_rules_version then we always have a waf_version

andrewlock · 2024-10-30T11:16:27Z

tracer/src/Datadog.Trace/Telemetry/Collectors/MetricsTelemetryCollectorBase.cs

@@ -211,21 +214,29 @@ private async Task AggregateMetricsLoopAsync()
        }
    }

-    private string[]? GetTags(string? ns, string[]? metricKeyTags)
+    private string[]? GetTags(string? ns, string[]? metricKeyTags, string name)


I'm not a fan of the changes here - it adds a lot of complexity and assumptions where that's just not necessary IMO.

It assumes that rasp metrics will never record event rules

It assumes that only rasp metrics will not record event rules

It also highlights for me that a bunch of the ASM metrics are already broken. Anything that doesn't have waf_version as the first tag in the metric is broken AFAICT 😬

We include the waf_version in all the ASM metrics, so that would not be a problem. With this new change, all ASM metrics would also include the event rules version but the RASP ones, which include only the WAF version. Anyway, I will test the implementation that you posted in the other comment, which actually decreases complexity. Thanks for the suggestion!

We include the waf_version in all the ASM metrics

That doesn't appear to be the case, e.g. waf.inputtruncated does not have it 😬

Maybe we should also add a test that asserts that all the ASM metrics do have it (if they are all supposed to have it) 🤔

You are right! I will check these metrics. I would say that they should have the WAF version, but I need to check first. Thanks for pointing this out.

tracer/src/Datadog.Trace/Telemetry/Collectors/MetricsTelemetryCollectorBase.cs

NachoEchevarria · 2024-10-30T13:46:57Z

I think this highlights a few things:

The existing (pre-this PR) behaviour looks buggy to me - we're overwriting the tags for a bunch of ASM metrics currently AFAICT. The original design assumed that all ASM metrics should have the waf version tagged. That looks fundamentally broken

I'm not a fan of the changes in GetTags() - I think that's piling on the complexity to an already too-complex aspect.

I think we can probably simplify it to so that the entire implementation is more like this:
private string[]? GetTags(string? ns, string[]? metricKeyTags)
{
    if (ns != MetricNamespaceConstants.ASM)
    {
        return metricKeyTags;
    }

    if (metricKeyTags is null)
    {
        return _wafVersionTags ?? _unknownWafVersionTags;
    }

    if (string.Equals(metricKeyTags[0], "waf_version"))
    {
        var wafVersionTag = (_wafVersionTags ?? _unknownWafVersionTags)[0];
        metricKeyTags[0] = wafVersionTag;
    }

    if (metricKeyTags.Length > 1 && string.Equals(metricKeyTags[1], "event_rules_version"))
    {
        var eventRulesTag = (_wafVersionTags ?? _unknownWafVersionTags)[1];
        metricKeyTags[1] = eventRulesTag;
    }

    return metricKeyTags;
}
This solves the existing overwriting issue, and adds support for event_rules. It also means we don't need the additional array fields, we just need
private static readonly string[] _unknownWafVersionTags = { "waf_version:unknown", "event_rules_version:unknown" };
private string[]? _wafVersionTags; // gets updated with both waf and rules version
We should also likely add some unit tests that asserts that:

if a metric uses waf_version then it's always the first element

if a metric uses event_rules_version then it's always the second element

if a metric uses event_rules_version then we always have a waf_version

Thanks for the feedback! I have updated the code and added the unit tests that you suggested!

andrewlock

Great, thanks! 🙂

NachoEchevarria · 2024-10-30T16:06:21Z

Thanks for the feedback and suggestions!

NachoEchevarria added the area:asm label Oct 28, 2024

NachoEchevarria added 5 commits October 29, 2024 11:57

Add rules version telemetry tag

a9d3e04

Add rules version telemetry

3bdfddb

Update RCM ruleset version

c5032d9

improve GetTags code

eaf6c8e

Update telemetryCollectorBase and tests

ae2648b

NachoEchevarria force-pushed the nacho/AddEventRulesVersionTelemetry branch from 8373f2b to ae2648b Compare October 29, 2024 10:59

NachoEchevarria added 2 commits October 29, 2024 13:00

Remove not needed

42a1a32

Fix tests

d383d31

NachoEchevarria marked this pull request as ready for review October 30, 2024 09:20

NachoEchevarria requested review from a team as code owners October 30, 2024 09:20

e-n-0 approved these changes Oct 30, 2024

View reviewed changes

anna-git approved these changes Oct 30, 2024

View reviewed changes

andrewlock reviewed Oct 30, 2024

View reviewed changes

Add unit tests and refactor

454d17a

andrewlock approved these changes Oct 30, 2024

View reviewed changes

NachoEchevarria merged commit 1d065bd into master Oct 30, 2024
73 of 76 checks passed

NachoEchevarria deleted the nacho/AddEventRulesVersionTelemetry branch October 30, 2024 16:06

github-actions bot added this to the vNext-v3 milestone Oct 30, 2024

[ASM] Add event rules version in telemetry #6208

[ASM] Add event rules version in telemetry #6208

Conversation

NachoEchevarria commented Oct 28, 2024 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

github-actions bot commented Oct 28, 2024

Snapshots difference summary

datadog-ddstaging bot commented Oct 28, 2024 • edited Loading

Datadog Report

andrewlock commented Oct 28, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Oct 28, 2024

Benchmarks Report for appsec 🐌

Benchmark details

Slower ⚠️ in #6208

Faster 🎉 in #6208

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6208

Raw results

andrewlock commented Oct 28, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock commented Oct 28, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

More allocations ⚠️ in #6208

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6208

Faster 🎉 in #6208

Raw results

Raw results

andrewlock left a comment • edited Loading

Choose a reason for hiding this comment

andrewlock Oct 30, 2024

Choose a reason for hiding this comment

NachoEchevarria Oct 30, 2024

Choose a reason for hiding this comment

andrewlock Oct 30, 2024

Choose a reason for hiding this comment

NachoEchevarria Oct 30, 2024

Choose a reason for hiding this comment

NachoEchevarria commented Oct 30, 2024

andrewlock left a comment

Choose a reason for hiding this comment

NachoEchevarria commented Oct 30, 2024

NachoEchevarria commented Oct 28, 2024 •

edited

Loading

datadog-ddstaging bot commented Oct 28, 2024 •

edited

Loading

andrewlock commented Oct 28, 2024 •

edited

Loading

andrewlock commented Oct 28, 2024 •

edited

Loading

andrewlock commented Oct 28, 2024 •

edited

Loading

andrewlock left a comment •

edited

Loading