[ASM] remote configuration refactoring and simplifying updates #6179

anna-git · 2024-10-22T17:46:53Z

Summary of changes

Main idea is to make Rcm.ConfigurationStatus not RCM specific anymore but more appsec configuration state specific. This way, depending on changes, it holds the new state to move appsec to and most init/update logic is held in it.

This PR can be read by commit, renaming commits can be looked at quickly.

Reason for change

RCM updates were hard to understand as well as the new subscriptions

Implementation details

Make a ConfigurationState holding the current appsec values and taking decisions accordingly.

Test coverage

Other details

datadog-ddstaging · 2024-10-22T18:04:50Z

Datadog Report

Branch report: anna/asm/rcm-refactor
Commit report: 2183bd7
Test service: dd-trace-dotnet

✅ 0 Failed, 366373 Passed, 2093 Skipped, 15h 30m 5.21s Total Time

andrewlock · 2024-10-22T18:17:31Z

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

    section CallTarget+Inlining+NGEN
    This PR (6179) - mean (1,117ms)  : 1095, 1139
     .   : milestone, 1117,
    master - mean (1,118ms)  : 1096, 1140
     .   : milestone, 1118,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6179) - mean (110ms)  : 107, 113
     .   : milestone, 110,
    master - mean (110ms)  : 107, 114
     .   : milestone, 110,

    section CallTarget+Inlining+NGEN
    This PR (6179) - mean (772ms)  : 756, 787
     .   : milestone, 772,
    master - mean (778ms)  : 762, 793
     .   : milestone, 778,

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

    section CallTarget+Inlining+NGEN
    This PR (6179) - mean (728ms)  : 713, 744
     .   : milestone, 728,
    master - mean (733ms)  : 718, 749
     .   : milestone, 733,

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

    section CallTarget+Inlining+NGEN
    This PR (6179) - mean (1,198ms)  : 1172, 1224
     .   : milestone, 1198,
    master - mean (1,201ms)  : 1175, 1228
     .   : milestone, 1201,

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

    section CallTarget+Inlining+NGEN
    This PR (6179) - mean (942ms)  : 926, 958
     .   : milestone, 942,
    master - mean (949ms)  : 927, 971
     .   : milestone, 949,

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

    section CallTarget+Inlining+NGEN
    This PR (6179) - mean (925ms)  : 909, 941
     .   : milestone, 925,
    master - mean (924ms)  : 905, 943
     .   : milestone, 924,

andrewlock · 2024-10-22T23:29:43Z

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 (6179) (11.174M)   : 0, 11173511
    master (11.292M)   : 0, 11291882
    benchmarks/2.9.0 (11.081M)   : 0, 11080577

    section Automatic
    This PR (6179) (7.351M)   : 0, 7350651
    master (7.471M)   : 0, 7470618
    benchmarks/2.9.0 (7.732M)   : 0, 7732233

    section Trace stats
    master (7.854M)   : 0, 7853541

    section Manual
    master (11.178M)   : 0, 11177916

    section Manual + Automatic
    This PR (6179) (6.848M)   : 0, 6848438
    master (6.983M)   : 0, 6983283

    section DD_TRACE_ENABLED=0
    master (10.353M)   : 0, 10352817

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6179) (9.577M)   : 0, 9577151
    master (9.759M)   : 0, 9758573
    benchmarks/2.9.0 (9.798M)   : 0, 9798067

    section Automatic
    This PR (6179) (6.560M)   : 0, 6559827
    master (6.447M)   : 0, 6446582

    section Trace stats
    master (6.536M)   : 0, 6536083

    section Manual
    master (9.551M)   : 0, 9551044

    section Manual + Automatic
    This PR (6179) (6.140M)   : 0, 6140095
    master (6.282M)   : 0, 6282382

    section DD_TRACE_ENABLED=0
    master (8.955M)   : 0, 8955266

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6179) (10.306M)   : 0, 10306315
    master (9.819M)   : 0, 9818915
    benchmarks/2.9.0 (10.067M)   : 0, 10067315

    section Automatic
    This PR (6179) (6.756M)   : 0, 6756026
    master (6.429M)   : 0, 6429392
    benchmarks/2.9.0 (7.552M)   : 0, 7552193

    section Trace stats
    master (7.215M)   : 0, 7214864

    section Manual
    master (9.879M)   : 0, 9879197

    section Manual + Automatic
    This PR (6179) (6.042M)   : 0, 6041552
    master (5.890M)   : 0, 5889975

    section DD_TRACE_ENABLED=0
    master (9.240M)   : 0, 9240298

andrewlock · 2024-10-23T17:55:02Z

Benchmarks Report for appsec 🐌

Benchmarks for #6179 compared to master:

1 benchmarks are slower, with geometric mean 1.240
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 #6179

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1	1.240	201.00	249.19

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
#6179	`AllCycleSimpleBody`	net6.0	72.9μs	71.4ns	277ns	0.0729	0	6 KB
#6179	`AllCycleSimpleBody`	netcoreapp3.1	62.9μs	57.3ns	222ns	0.0939	0	6.95 KB
#6179	`AllCycleSimpleBody`	net472	49.4μs	40.6ns	152ns	1.31	0	8.33 KB
#6179	`AllCycleMoreComplexBody`	net6.0	79.9μs	74.7ns	279ns	0.12	0	9.51 KB
#6179	`AllCycleMoreComplexBody`	netcoreapp3.1	69.4μs	83ns	321ns	0.138	0	10.37 KB
#6179	`AllCycleMoreComplexBody`	net472	57.5μs	66.8ns	259ns	1.86	0.0286	11.85 KB
#6179	`ObjectExtractorSimpleBody`	net6.0	153ns	0.166ns	0.62ns	0.00395	0	280 B
#6179	`ObjectExtractorSimpleBody`	netcoreapp3.1	249ns	0.129ns	0.484ns	0.00362	0	272 B
#6179	`ObjectExtractorSimpleBody`	net472	171ns	0.142ns	0.531ns	0.0446	0	281 B
#6179	`ObjectExtractorMoreComplexBody`	net6.0	3.28μs	2.15ns	8.03ns	0.0538	0	3.78 KB
#6179	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	4.02μs	6.27ns	24.3ns	0.0496	0	3.69 KB
#6179	`ObjectExtractorMoreComplexBody`	net472	4.29μs	4ns	15.5ns	0.601	0.00639	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
#6179	`EncodeArgs`	net6.0	39.6μs	15.8ns	56.9ns	0.454	0	32.4 KB
#6179	`EncodeArgs`	netcoreapp3.1	56.2μs	111ns	428ns	0.437	0	32.4 KB
#6179	`EncodeArgs`	net472	66.9μs	58.8ns	228ns	5.15	0.0669	32.5 KB
#6179	`EncodeLegacyArgs`	net6.0	72.9μs	23ns	89ns	0.0365	0	2.14 KB
#6179	`EncodeLegacyArgs`	netcoreapp3.1	104μs	124ns	480ns	0	0	2.14 KB
#6179	`EncodeLegacyArgs`	net472	153μs	83.2ns	300ns	0.306	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
#6179	`RunWafRealisticBenchmark`	net6.0	184μs	194ns	751ns	0	2.44 KB
#6179	`RunWafRealisticBenchmark`	netcoreapp3.1	198μs	236ns	914ns	0	2.39 KB
#6179	`RunWafRealisticBenchmark`	net472	210μs	131ns	506ns	0.314	2.46 KB
#6179	`RunWafRealisticBenchmarkWithAttack`	net6.0	124μs	134ns	520ns	0	1.47 KB
#6179	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	130μs	228ns	883ns	0	1.46 KB
#6179	`RunWafRealisticBenchmarkWithAttack`	net472	139μs	75.1ns	291ns	0.207	1.49 KB

Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6179

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0	254.19 KB	265.03 KB	10.84 KB	4.26%

Fewer allocations 🎉 in #6179

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	263.3 KB	255.16 KB	-8.14 KB	-3.09%

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
#6179	`StringConcatBenchmark`	net6.0	61.7μs	629ns	6.26μs	43.44 KB
#6179	`StringConcatBenchmark`	netcoreapp3.1	54.3μs	230ns	889ns	42.64 KB
#6179	`StringConcatBenchmark`	net472	38.7μs	184ns	757ns	59.09 KB
#6179	`StringConcatAspectBenchmark`	net6.0	314μs	1.43μs	5.56μs	265.03 KB
#6179	`StringConcatAspectBenchmark`	netcoreapp3.1	327μs	3.01μs	28.5μs	255.16 KB
#6179	`StringConcatAspectBenchmark`	net472	280μs	4.72μs	45.7μs	278.53 KB

andrewlock · 2024-10-24T01:52:21Z

Benchmarks Report for tracer 🐌

Benchmarks for #6179 compared to master:

2 benchmarks are faster, with geometric mean 1.157
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.66μs	38.2ns	175ns	0.0119	0.00396	0	5.42 KB
master	`StartStopWithChild`	netcoreapp3.1	9.84μs	53.7ns	294ns	0.0147	0.00489	0	5.62 KB
master	`StartStopWithChild`	net472	16.3μs	49.8ns	186ns	1.03	0.318	0.0896	6.06 KB
#6179	`StartStopWithChild`	net6.0	7.57μs	43.2ns	340ns	0.0151	0.00753	0	5.43 KB
#6179	`StartStopWithChild`	netcoreapp3.1	9.72μs	54.3ns	356ns	0.0237	0.0142	0.00474	5.62 KB
#6179	`StartStopWithChild`	net472	16.5μs	59.8ns	232ns	1.04	0.329	0.0964	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	471μs	231ns	865ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	621μs	295ns	1.14μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	893μs	330ns	1.19μs	0.437	3.3 KB
#6179	`WriteAndFlushEnrichedTraces`	net6.0	466μs	273ns	1.06μs	0	2.7 KB
#6179	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	627μs	351ns	1.31μs	0	2.7 KB
#6179	`WriteAndFlushEnrichedTraces`	net472	842μs	337ns	1.26μs	0.419	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	196μs	1.1μs	8.59μs	0.212	18.45 KB
master	`SendRequest`	netcoreapp3.1	225μs	1.32μs	12.4μs	0.218	20.61 KB
master	`SendRequest`	net472	9.94E‑06ns	9.94E‑06ns	3.85E‑05ns	0	0 b
#6179	`SendRequest`	net6.0	200μs	1.25μs	12.4μs	0.185	18.45 KB
#6179	`SendRequest`	netcoreapp3.1	228μs	1.32μs	12.4μs	0.226	20.61 KB
#6179	`SendRequest`	net472	0ns	0ns	0ns	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	572μs	2.04μs	7.37μs	0.584	0	0	41.52 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	678μs	3.36μs	13.9μs	0.327	0	0	41.83 KB
master	`WriteAndFlushEnrichedTraces`	net472	885μs	3.04μs	11.4μs	8.74	2.62	0.437	53.37 KB
#6179	`WriteAndFlushEnrichedTraces`	net6.0	559μs	2.81μs	15.6μs	0.561	0	0	41.6 KB
#6179	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	692μs	3.79μs	23.4μs	0.338	0	0	41.74 KB
#6179	`WriteAndFlushEnrichedTraces`	net472	867μs	3.91μs	15.1μs	8.19	2.59	0.431	53.35 KB

Benchmarks.Trace.DbCommandBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6179

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0	1.156	1,413.89	1,223.12

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.41μs	1.37ns	5.29ns	0.0142	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.52ns	5.9ns	0.0141	1.02 KB
master	`ExecuteNonQuery`	net472	2.06μs	2.19ns	8.48ns	0.156	987 B
#6179	`ExecuteNonQuery`	net6.0	1.22μs	1.18ns	4.55ns	0.014	1.02 KB
#6179	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.29ns	5ns	0.0133	1.02 KB
#6179	`ExecuteNonQuery`	net472	2.13μs	2.13ns	7.95ns	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.25μs	0.447ns	1.67ns	0.0138	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.5μs	0.832ns	2.88ns	0.0132	976 B
master	`CallElasticsearch`	net472	2.51μs	1.42ns	5.3ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.36μs	0.515ns	1.93ns	0.0131	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.58μs	0.55ns	2.06ns	0.0135	1.02 KB
master	`CallElasticsearchAsync`	net472	2.54μs	1.6ns	5.99ns	0.166	1.05 KB
#6179	`CallElasticsearch`	net6.0	1.24μs	0.644ns	2.41ns	0.0137	976 B
#6179	`CallElasticsearch`	netcoreapp3.1	1.53μs	1.07ns	4ns	0.013	976 B
#6179	`CallElasticsearch`	net472	2.46μs	3.52ns	13.6ns	0.158	995 B
#6179	`CallElasticsearchAsync`	net6.0	1.23μs	0.482ns	1.8ns	0.0134	952 B
#6179	`CallElasticsearchAsync`	netcoreapp3.1	1.66μs	4.84ns	18.7ns	0.0139	1.02 KB
#6179	`CallElasticsearchAsync`	net472	2.57μs	2.61ns	10.1ns	0.166	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.2μs	0.484ns	1.68ns	0.0132	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.61μs	0.434ns	1.62ns	0.0129	952 B
master	`ExecuteAsync`	net472	1.8μs	0.733ns	2.64ns	0.145	915 B
#6179	`ExecuteAsync`	net6.0	1.31μs	0.791ns	3.06ns	0.0135	952 B
#6179	`ExecuteAsync`	netcoreapp3.1	1.66μs	1.06ns	4.12ns	0.0125	952 B
#6179	`ExecuteAsync`	net472	1.84μs	1.14ns	4.43ns	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.15μs	2.37ns	8.88ns	0.031	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.09μs	2.33ns	8.71ns	0.0381	2.76 KB
master	`SendAsync`	net472	7.76μs	1.9ns	7.35ns	0.498	3.15 KB
#6179	`SendAsync`	net6.0	4.14μs	1.92ns	7.45ns	0.031	2.22 KB
#6179	`SendAsync`	netcoreapp3.1	5.17μs	6.88ns	26.6ns	0.0368	2.76 KB
#6179	`SendAsync`	net472	7.85μs	3.92ns	15.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.55μs	0.725ns	2.62ns	0.0231	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.08μs	0.824ns	3.08ns	0.022	1.64 KB
master	`EnrichedLog`	net472	2.49μs	1.36ns	5.07ns	0.249	1.57 KB
#6179	`EnrichedLog`	net6.0	1.46μs	1.1ns	4.28ns	0.0233	1.64 KB
#6179	`EnrichedLog`	netcoreapp3.1	2.13μs	1.22ns	4.72ns	0.0223	1.64 KB
#6179	`EnrichedLog`	net472	2.58μs	0.779ns	2.92ns	0.25	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	116μs	153ns	573ns	0.0583	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	122μs	86.3ns	311ns	0.0606	0	4.28 KB
master	`EnrichedLog`	net472	153μs	148ns	572ns	0.69	0.23	4.46 KB
#6179	`EnrichedLog`	net6.0	117μs	200ns	776ns	0	0	4.28 KB
#6179	`EnrichedLog`	netcoreapp3.1	122μs	145ns	521ns	0.0599	0	4.28 KB
#6179	`EnrichedLog`	net472	150μs	189ns	733ns	0.68	0.227	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.06μs	0.767ns	2.87ns	0.0307	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.22μs	1.49ns	5.77ns	0.0294	2.2 KB
master	`EnrichedLog`	net472	4.89μs	2.67ns	10.4ns	0.32	2.02 KB
#6179	`EnrichedLog`	net6.0	2.98μs	0.817ns	3.16ns	0.0299	2.2 KB
#6179	`EnrichedLog`	netcoreapp3.1	4.25μs	2.3ns	8.92ns	0.0298	2.2 KB
#6179	`EnrichedLog`	net472	4.91μs	1.88ns	7.04ns	0.318	2.02 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`SendReceive`	net6.0	1.39μs	0.816ns	3.16ns	0.016	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.79μs	0.844ns	2.92ns	0.0156	0	1.14 KB
master	`SendReceive`	net472	2.18μs	1.04ns	4.05ns	0.183	0.00109	1.16 KB
#6179	`SendReceive`	net6.0	1.31μs	1.02ns	3.95ns	0.0157	0	1.14 KB
#6179	`SendReceive`	netcoreapp3.1	1.77μs	2.14ns	8.28ns	0.0151	0	1.14 KB
#6179	`SendReceive`	net472	2.11μs	1.27ns	4.76ns	0.183	0	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.76μs	0.913ns	3.42ns	0.022	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.89μs	2.25ns	8.44ns	0.0215	1.65 KB
master	`EnrichedLog`	net472	4.46μs	4.67ns	18.1ns	0.322	2.04 KB
#6179	`EnrichedLog`	net6.0	2.72μs	0.756ns	2.93ns	0.0217	1.6 KB
#6179	`EnrichedLog`	netcoreapp3.1	3.73μs	1.46ns	5.47ns	0.0224	1.65 KB
#6179	`EnrichedLog`	net472	4.47μs	4.67ns	18.1ns	0.322	2.04 KB

Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6179

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.158	490.44	423.65

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	490ns	0.31ns	1.2ns	0.00809	576 B
master	`StartFinishSpan`	netcoreapp3.1	603ns	0.916ns	3.3ns	0.00775	576 B
master	`StartFinishSpan`	net472	683ns	0.282ns	1.09ns	0.0916	578 B
master	`StartFinishScope`	net6.0	493ns	0.255ns	0.987ns	0.00968	696 B
master	`StartFinishScope`	netcoreapp3.1	754ns	4.12ns	22.5ns	0.0093	696 B
master	`StartFinishScope`	net472	902ns	0.969ns	3.75ns	0.104	658 B
#6179	`StartFinishSpan`	net6.0	424ns	0.259ns	1ns	0.00799	576 B
#6179	`StartFinishSpan`	netcoreapp3.1	564ns	0.315ns	1.22ns	0.0078	576 B
#6179	`StartFinishSpan`	net472	745ns	0.368ns	1.43ns	0.0917	578 B
#6179	`StartFinishScope`	net6.0	478ns	0.164ns	0.614ns	0.00984	696 B
#6179	`StartFinishScope`	netcoreapp3.1	758ns	2.29ns	8.86ns	0.00918	696 B
#6179	`StartFinishScope`	net472	878ns	1.01ns	3.92ns	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	647ns	0.516ns	2ns	0.00971	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	960ns	0.623ns	2.41ns	0.00958	696 B
master	`RunOnMethodBegin`	net472	1.09μs	0.976ns	3.78ns	0.104	658 B
#6179	`RunOnMethodBegin`	net6.0	708ns	0.374ns	1.45ns	0.00986	696 B
#6179	`RunOnMethodBegin`	netcoreapp3.1	976ns	0.529ns	1.91ns	0.00934	696 B
#6179	`RunOnMethodBegin`	net472	1.16μs	0.326ns	1.18ns	0.104	658 B

tracer/src/Datadog.Trace/AppSec/Security.cs

tracer/src/Datadog.Trace/AppSec/Waf/ConfigurationState.cs

NachoEchevarria

Nice job! Added some minor suggestions but LGTM!

…e, holding the current appsec state and decision maker on receiving rcm files

Any instead of All Nacho Suggestion Any instead of all Nacho suggestion Co-Authored-By: NachoEchevarria <53266532+NachoEchevarria@users.noreply.github.com>

test

andrewlock

LGTM from the tracer PoV (as it's only some renaming)

I don't know how big a deal it is, but there's a lot of LINQ and initializing of lots of empty dictionaries in the new ConfigurationState object. You might want to consider making some of the initialization lazy and unrolling some of the LINQ to avoid so many ToList() calls with unknown sizes etc. Not required though, and would definitely recommend doing in a separate PR if you do choose to do it 🙂

anna-git · 2024-10-28T14:20:06Z

LGTM from the tracer PoV (as it's only some renaming)

I don't know how big a deal it is, but there's a lot of LINQ and initializing of lots of empty dictionaries in the new ConfigurationState object. You might want to consider making some of the initialization lazy and unrolling some of the LINQ to avoid so many ToList() calls with unknown sizes etc. Not required though, and would definitely recommend doing in a separate PR if you do choose to do it 🙂

thanks! Yes this class has many dictionaries initialized by default. I was on the fence between that or always testing if they're null before 🤔 I'll add a backlog for it !

anna-git force-pushed the anna/asm/rcm-refactor branch from 3030e3c to dea3adb Compare October 23, 2024 16:26

anna-git force-pushed the anna/asm/rcm-refactor branch 4 times, most recently from f10670b to c73d41e Compare October 25, 2024 11:05

anna-git changed the title ~~Anna/asm/rcm refactor~~ [ASM] remote configuration refactoring and simplifying updates Oct 25, 2024

anna-git marked this pull request as ready for review October 25, 2024 12:55

anna-git requested review from a team as code owners October 25, 2024 12:55

NachoEchevarria reviewed Oct 25, 2024

View reviewed changes

tracer/src/Datadog.Trace/AppSec/Security.cs Outdated Show resolved Hide resolved

github-actions bot added the area:asm label Oct 25, 2024

NachoEchevarria reviewed Oct 25, 2024

View reviewed changes

tracer/src/Datadog.Trace/AppSec/Waf/ConfigurationState.cs Outdated Show resolved Hide resolved

NachoEchevarria reviewed Oct 25, 2024

View reviewed changes

tracer/src/Datadog.Trace/AppSec/Waf/ConfigurationState.cs Outdated Show resolved Hide resolved

NachoEchevarria approved these changes Oct 25, 2024

View reviewed changes

anna-git added 5 commits October 28, 2024 11:59

Just renaming in source code

ad8dc8c

Renaming and syntax simplifying

e1f8239

Refactoring of remote configuration state becoming configuration stat…

895f5c6

…e, holding the current appsec state and decision maker on receiving rcm files

Adapting tests to configuration state refactoring

4a60774

Keep coordinator without ifs and specific calls in Core / Framework

1edb59f

anna-git force-pushed the anna/asm/rcm-refactor branch from e6181d3 to 9a146ee Compare October 28, 2024 11:00

Any instead of All Nacho Suggestion

a2be7cd

Any instead of All Nacho Suggestion Any instead of all Nacho suggestion Co-Authored-By: NachoEchevarria <53266532+NachoEchevarria@users.noreply.github.com>

anna-git force-pushed the anna/asm/rcm-refactor branch from 9a146ee to a2be7cd Compare October 28, 2024 11:00

fix tests

2183bd7

test

anna-git force-pushed the anna/asm/rcm-refactor branch from 783a216 to 2183bd7 Compare October 28, 2024 12:25

andrewlock approved these changes Oct 28, 2024

View reviewed changes

anna-git merged commit ced96ee into master Oct 28, 2024
78 checks passed

anna-git deleted the anna/asm/rcm-refactor branch October 28, 2024 14:30

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ASM] remote configuration refactoring and simplifying updates #6179

[ASM] remote configuration refactoring and simplifying updates #6179

anna-git commented Oct 22, 2024 •

edited

Loading

datadog-ddstaging bot commented Oct 22, 2024 •

edited

Loading

andrewlock commented Oct 22, 2024 •

edited

Loading

andrewlock commented Oct 22, 2024 •

edited

Loading

andrewlock commented Oct 23, 2024 •

edited

Loading

Slower ⚠️ in #6179

Raw results

Raw results

Raw results

More allocations ⚠️ in #6179

Fewer allocations 🎉 in #6179

Raw results

andrewlock commented Oct 24, 2024 •

edited

Loading

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6179

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6179

Raw results

Raw results

NachoEchevarria left a comment

andrewlock left a comment

anna-git commented Oct 28, 2024 •

edited

Loading

[ASM] remote configuration refactoring and simplifying updates #6179

[ASM] remote configuration refactoring and simplifying updates #6179

Conversation

anna-git commented Oct 22, 2024 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

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

Datadog Report

andrewlock commented Oct 22, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Oct 22, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock commented Oct 23, 2024 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Slower ⚠️ in #6179

Raw results

Raw results

Raw results

More allocations ⚠️ in #6179

Fewer allocations 🎉 in #6179

Raw results

andrewlock commented Oct 24, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6179

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6179

Raw results

Raw results

NachoEchevarria left a comment

Choose a reason for hiding this comment

andrewlock left a comment

Choose a reason for hiding this comment

anna-git commented Oct 28, 2024 • edited Loading

anna-git commented Oct 22, 2024 •

edited

Loading

datadog-ddstaging bot commented Oct 22, 2024 •

edited

Loading

andrewlock commented Oct 22, 2024 •

edited

Loading

andrewlock commented Oct 22, 2024 •

edited

Loading

andrewlock commented Oct 23, 2024 •

edited

Loading

andrewlock commented Oct 24, 2024 •

edited

Loading

anna-git commented Oct 28, 2024 •

edited

Loading