[ASM] Changes to the collection of usr.id for authenticated clients #5738

robertpi · 2024-06-25T12:07:05Z

Summary of changes

Change to the automatic instrumentation of common authentication frameworks, to collect details of the authenticated user. This mechanism is only active when ASM is enabled. The full specification is in this internal RFC.

Reason for change

This will improve our detections of Account Take Overs (ATO).

Test coverage

Added two new unit test covering basic functionally such creating a hash to anonymize user ids.

Updated several existing integration tests that covered automatic collection of user ids, and add some integration tests to cover configure user id collection via remote config.

Other details

github-actions · 2024-06-25T12:19:44Z

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.

1 occurrences of :

+      appsec.events.users.login.failure.usr.id: NoSuchUser,

2 occurrences of :

-      _dd.appsec.events.users.login.failure.auto.mode: safe,
+      _dd.appsec.events.users.login.failure.auto.mode: identification,

1 occurrences of :

-      _dd.appsec.events.users.login.success.auto.mode: safe,
+      _dd.appsec.events.users.login.success.auto.mode: identification,

1 occurrences of :

-      appsec.events.users.login.failure.username: NoSuchUser,
[...]
+      appsec.events.users.login.failure.usr.id: NoSuchUser,

2 occurrences of :

-      _dd.appsec.events.users.login.failure.auto.mode: extended,
+      _dd.appsec.events.users.login.failure.auto.mode: identification,

1 occurrences of :

-      appsec.events.users.login.failure.email: test@test.com,
[...]
-      appsec.events.users.login.failure.username: TestUser,

1 occurrences of :

-      usr.email: test@test.com,
[...]
-      usr.name: TestUser,
-      _dd.appsec.events.users.login.success.auto.mode: extended,
+      _dd.appsec.events.users.login.success.auto.mode: identification,

datadog-ddstaging · 2024-06-25T12:25:41Z

Datadog Report

Branch report: robert/asm/user-id-collection-modes
Commit report: ffadd32
Test service: dd-trace-dotnet

✅ 0 Failed, 346392 Passed, 1759 Skipped, 15h 46m 11.62s Total Time

andrewlock · 2024-06-25T12:33: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 (5738) - mean (72ms)  : 64, 80
     .   : milestone, 72,
    master - mean (72ms)  : 64, 79
     .   : milestone, 72,

    section CallTarget+Inlining+NGEN
    This PR (5738) - mean (1,072ms)  : 1043, 1101
     .   : milestone, 1072,
    master - mean (1,072ms)  : 1045, 1098
     .   : milestone, 1072,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5738) - mean (108ms)  : 105, 111
     .   : milestone, 108,
    master - mean (109ms)  : 105, 113
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (5738) - mean (748ms)  : 728, 768
     .   : milestone, 748,
    master - mean (747ms)  : 729, 764
     .   : milestone, 747,

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

    section CallTarget+Inlining+NGEN
    This PR (5738) - mean (705ms)  : 684, 725
     .   : milestone, 705,
    master - mean (704ms)  : 677, 730
     .   : milestone, 704,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5738) - mean (192ms)  : 189, 195
     .   : milestone, 192,
    master - mean (192ms)  : 189, 194
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (5738) - mean (1,170ms)  : 1143, 1197
     .   : milestone, 1170,
    master - mean (1,169ms)  : 1144, 1195
     .   : milestone, 1169,

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

    section CallTarget+Inlining+NGEN
    This PR (5738) - mean (926ms)  : 902, 950
     .   : milestone, 926,
    master - mean (914ms)  : 896, 933
     .   : milestone, 914,

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

    section CallTarget+Inlining+NGEN
    This PR (5738) - mean (908ms)  : 882, 934
     .   : milestone, 908,
    master - mean (901ms)  : 879, 922
     .   : milestone, 901,

andrewlock · 2024-06-25T13:25:47Z

Benchmarks Report for appsec 🐌

Benchmarks for #5738 compared to master:

1 benchmarks are faster, with geometric mean 1.144
2 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 - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5738

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑netcoreapp3.1	1.144	224.29	195.98

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	72μs	137ns	529ns	0.0717	0	6.01 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	63.2μs	89.1ns	345ns	0.0636	0	6.95 KB
master	`AllCycleSimpleBody`	net472	49.5μs	87.9ns	340ns	1.31	0	8.34 KB
master	`AllCycleMoreComplexBody`	net6.0	78.4μs	68.8ns	258ns	0.119	0	9.51 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	70.5μs	90.3ns	350ns	0.105	0	10.36 KB
master	`AllCycleMoreComplexBody`	net472	56μs	86.9ns	337ns	1.86	0.0281	11.85 KB
master	`ObjectExtractorSimpleBody`	net6.0	165ns	0.11ns	0.428ns	0.00396	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	224ns	0.166ns	0.621ns	0.00382	0	272 B
master	`ObjectExtractorSimpleBody`	net472	168ns	0.155ns	0.601ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	3.11μs	1.52ns	5.9ns	0.0528	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.93μs	2.83ns	11ns	0.0493	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	3.91μs	6.09ns	23.6ns	0.602	0.00586	3.8 KB
#5738	`AllCycleSimpleBody`	net6.0	73.5μs	75.4ns	292ns	0.0733	0	6 KB
#5738	`AllCycleSimpleBody`	netcoreapp3.1	62.6μs	114ns	441ns	0.0621	0	6.95 KB
#5738	`AllCycleSimpleBody`	net472	47.9μs	40.6ns	157ns	1.31	0	8.34 KB
#5738	`AllCycleMoreComplexBody`	net6.0	79.8μs	48.2ns	180ns	0.119	0	9.51 KB
#5738	`AllCycleMoreComplexBody`	netcoreapp3.1	70.7μs	150ns	563ns	0.141	0	10.37 KB
#5738	`AllCycleMoreComplexBody`	net472	56.8μs	59.3ns	230ns	1.87	0.0284	11.85 KB
#5738	`ObjectExtractorSimpleBody`	net6.0	186ns	0.915ns	3.77ns	0.00392	0	280 B
#5738	`ObjectExtractorSimpleBody`	netcoreapp3.1	196ns	0.143ns	0.556ns	0.00375	0	272 B
#5738	`ObjectExtractorSimpleBody`	net472	175ns	0.345ns	1.29ns	0.0446	0	281 B
#5738	`ObjectExtractorMoreComplexBody`	net6.0	3.05μs	2.72ns	10.2ns	0.0536	0	3.78 KB
#5738	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	4.07μs	2.61ns	9.4ns	0.0515	0	3.69 KB
#5738	`ObjectExtractorMoreComplexBody`	net472	3.88μs	4.2ns	16.3ns	0.603	0.00583	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	37.4μs	15.8ns	61.1ns	0.447	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	54.3μs	16.5ns	59.5ns	0.432	0	32.4 KB
master	`EncodeArgs`	net472	66.9μs	31.5ns	122ns	5.13	0.0667	32.5 KB
master	`EncodeLegacyArgs`	net6.0	74.8μs	391ns	1.92μs	0.0351	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	104μs	61.2ns	212ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	155μs	98.8ns	383ns	0.309	0	2.15 KB
#5738	`EncodeArgs`	net6.0	37.5μs	28ns	105ns	0.451	0	32.4 KB
#5738	`EncodeArgs`	netcoreapp3.1	54.6μs	19.9ns	74.3ns	0.439	0	32.4 KB
#5738	`EncodeArgs`	net472	67.4μs	37.9ns	147ns	5.16	0.067	32.5 KB
#5738	`EncodeLegacyArgs`	net6.0	78.1μs	44.2ns	171ns	0	0	2.14 KB
#5738	`EncodeLegacyArgs`	netcoreapp3.1	105μs	112ns	433ns	0	0	2.14 KB
#5738	`EncodeLegacyArgs`	net472	154μs	100ns	388ns	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	185μs	64.3ns	249ns	0	2.42 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	197μs	92.8ns	347ns	0	2.37 KB
master	`RunWafRealisticBenchmark`	net472	213μs	192ns	744ns	0.315	2.43 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	68.9ns	258ns	0	1.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	132μs	134ns	500ns	0	1.45 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	139μs	46.2ns	179ns	0.208	1.48 KB
#5738	`RunWafRealisticBenchmark`	net6.0	185μs	358ns	1.39μs	0	2.42 KB
#5738	`RunWafRealisticBenchmark`	netcoreapp3.1	195μs	234ns	908ns	0	2.37 KB
#5738	`RunWafRealisticBenchmark`	net472	210μs	171ns	664ns	0.314	2.43 KB
#5738	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	117ns	453ns	0	1.46 KB
#5738	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	130μs	86.6ns	324ns	0	1.45 KB
#5738	`RunWafRealisticBenchmarkWithAttack`	net472	140μs	79.4ns	286ns	0.211	1.48 KB

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

More allocations ⚠️ in #5738

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0	252.67 KB	266.77 KB	14.1 KB	5.58%

Fewer allocations 🎉 in #5738

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	59.22 KB	57.16 KB	-2.06 KB	-3.49%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	264.21 KB	253.93 KB	-10.28 KB	-3.89%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	62.2μs	774ns	7.7μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	61μs	755ns	7.44μs	42.64 KB
master	`StringConcatBenchmark`	net472	37.8μs	114ns	427ns	59.22 KB
master	`StringConcatAspectBenchmark`	net6.0	301μs	1.72μs	13.4μs	252.67 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	349μs	1.81μs	10.7μs	264.21 KB
master	`StringConcatAspectBenchmark`	net472	283μs	5.91μs	57.9μs	278.53 KB
#5738	`StringConcatBenchmark`	net6.0	63.1μs	809ns	8.09μs	43.44 KB
#5738	`StringConcatBenchmark`	netcoreapp3.1	54.4μs	303ns	1.82μs	42.64 KB
#5738	`StringConcatBenchmark`	net472	39.2μs	210ns	1.11μs	57.16 KB
#5738	`StringConcatAspectBenchmark`	net6.0	317μs	6.4μs	63.4μs	266.77 KB
#5738	`StringConcatAspectBenchmark`	netcoreapp3.1	336μs	1.88μs	12.2μs	253.93 KB
#5738	`StringConcatAspectBenchmark`	net472	277μs	5.3μs	51.7μs	278.53 KB

andrewlock · 2024-06-25T13:29:56Z

Benchmarks Report for tracer 🐌

Benchmarks for #5738 compared to master:

2 benchmarks are slower, with geometric mean 1.128
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.63μs	42.1ns	263ns	0.0153	0.00765	0	5.43 KB
master	`StartStopWithChild`	netcoreapp3.1	9.84μs	39.6ns	137ns	0.0145	0.00484	0	5.62 KB
master	`StartStopWithChild`	net472	16μs	62.2ns	241ns	1.02	0.3	0.0948	6.07 KB
#5738	`StartStopWithChild`	net6.0	7.87μs	43.4ns	345ns	0.0152	0.00759	0	5.42 KB
#5738	`StartStopWithChild`	netcoreapp3.1	9.92μs	54.5ns	345ns	0.0149	0.00496	0	5.61 KB
#5738	`StartStopWithChild`	net472	16μs	66.9ns	259ns	1.01	0.304	0.0879	6.08 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	456μs	448ns	1.74μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	630μs	435ns	1.69μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	831μs	740ns	2.77μs	0.411	3.3 KB
#5738	`WriteAndFlushEnrichedTraces`	net6.0	505μs	379ns	1.36μs	0	2.7 KB
#5738	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	637μs	356ns	1.38μs	0	2.7 KB
#5738	`WriteAndFlushEnrichedTraces`	net472	865μs	765ns	2.86μs	0.431	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	212μs	1.21μs	8.71μs	0.203	18.45 KB
master	`SendRequest`	netcoreapp3.1	224μs	1.19μs	6.53μs	0.242	20.61 KB
master	`SendRequest`	net472	0.00427ns	0.00176ns	0.00682ns	0	0 b
#5738	`SendRequest`	net6.0	199μs	1.18μs	11.8μs	0.185	18.45 KB
#5738	`SendRequest`	netcoreapp3.1	238μs	1.37μs	10.4μs	0.225	20.61 KB
#5738	`SendRequest`	net472	0.00283ns	0.000872ns	0.00338ns	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	581μs	3.18μs	19.3μs	0.566	0	0	41.62 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	703μs	3.96μs	37.6μs	0.34	0	0	41.64 KB
master	`WriteAndFlushEnrichedTraces`	net472	864μs	4.05μs	15.2μs	8.42	2.66	0.443	53.31 KB
#5738	`WriteAndFlushEnrichedTraces`	net6.0	591μs	3.14μs	16μs	0.558	0	0	41.57 KB
#5738	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	735μs	4.11μs	25.7μs	0.383	0	0	41.69 KB
#5738	`WriteAndFlushEnrichedTraces`	net472	892μs	3.3μs	12.8μs	8.04	2.23	0.446	53.31 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.19μs	1.18ns	4.41ns	0.0143	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.76μs	1.78ns	6.66ns	0.0132	1.02 KB
master	`ExecuteNonQuery`	net472	2.01μs	4.05ns	15.7ns	0.156	987 B
#5738	`ExecuteNonQuery`	net6.0	1.29μs	1.14ns	4.26ns	0.014	1.02 KB
#5738	`ExecuteNonQuery`	netcoreapp3.1	1.77μs	1.74ns	6.74ns	0.0133	1.02 KB
#5738	`ExecuteNonQuery`	net472	2μs	1.66ns	6.2ns	0.156	987 B

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

Slower ⚠️ in #5738

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net6.0	1.118	1,114.68	1,246.21

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.12μs	0.476ns	1.78ns	0.0135	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.5μs	0.553ns	2.14ns	0.0128	976 B
master	`CallElasticsearch`	net472	2.48μs	1.05ns	4.08ns	0.157	995 B
master	`CallElasticsearchAsync`	net6.0	1.32μs	0.756ns	2.83ns	0.0131	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	0.94ns	3.52ns	0.014	1.02 KB
master	`CallElasticsearchAsync`	net472	2.7μs	1.74ns	6.72ns	0.166	1.05 KB
#5738	`CallElasticsearch`	net6.0	1.25μs	0.534ns	2ns	0.0134	976 B
#5738	`CallElasticsearch`	netcoreapp3.1	1.47μs	0.823ns	3.19ns	0.0133	976 B
#5738	`CallElasticsearch`	net472	2.4μs	3.34ns	12.9ns	0.157	995 B
#5738	`CallElasticsearchAsync`	net6.0	1.3μs	1.21ns	4.67ns	0.013	952 B
#5738	`CallElasticsearchAsync`	netcoreapp3.1	1.61μs	1.35ns	5.24ns	0.0137	1.02 KB
#5738	`CallElasticsearchAsync`	net472	2.65μs	2.24ns	8.67ns	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.15μs	0.745ns	2.79ns	0.0132	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.57μs	0.965ns	3.74ns	0.0126	952 B
master	`ExecuteAsync`	net472	1.75μs	1.8ns	6.96ns	0.145	915 B
#5738	`ExecuteAsync`	net6.0	1.23μs	0.707ns	2.74ns	0.0136	952 B
#5738	`ExecuteAsync`	netcoreapp3.1	1.67μs	0.802ns	3.11ns	0.0125	952 B
#5738	`ExecuteAsync`	net472	1.77μs	0.463ns	1.79ns	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.3μs	1.61ns	6.04ns	0.0299	2.22 KB
master	`SendAsync`	netcoreapp3.1	5.09μs	2.03ns	7.6ns	0.0355	2.76 KB
master	`SendAsync`	net472	7.75μs	3.5ns	13.6ns	0.499	3.15 KB
#5738	`SendAsync`	net6.0	4.28μs	1.81ns	6.76ns	0.0321	2.22 KB
#5738	`SendAsync`	netcoreapp3.1	5.13μs	2.18ns	8.42ns	0.0358	2.76 KB
#5738	`SendAsync`	net472	7.69μs	5.64ns	21.1ns	0.499	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.56μs	1.15ns	4.47ns	0.0234	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.21μs	3.89ns	15ns	0.023	1.64 KB
master	`EnrichedLog`	net472	2.69μs	1.23ns	4.6ns	0.249	1.57 KB
#5738	`EnrichedLog`	net6.0	1.44μs	0.804ns	3.01ns	0.0231	1.64 KB
#5738	`EnrichedLog`	netcoreapp3.1	2.26μs	1.22ns	4.42ns	0.0223	1.64 KB
#5738	`EnrichedLog`	net472	2.52μs	1.26ns	4.87ns	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	167ns	646ns	0.059	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	120μs	205ns	794ns	0	0	4.28 KB
master	`EnrichedLog`	net472	150μs	175ns	678ns	0.672	0.224	4.46 KB
#5738	`EnrichedLog`	net6.0	115μs	234ns	907ns	0.0571	0	4.28 KB
#5738	`EnrichedLog`	netcoreapp3.1	120μs	154ns	596ns	0	0	4.28 KB
#5738	`EnrichedLog`	net472	147μs	178ns	688ns	0.663	0.221	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	2.83μs	1.18ns	4.56ns	0.0311	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.12μs	1.78ns	6.41ns	0.0288	2.2 KB
master	`EnrichedLog`	net472	4.81μs	3.49ns	12.1ns	0.318	2.02 KB
#5738	`EnrichedLog`	net6.0	3.06μs	0.558ns	2.16ns	0.0307	2.2 KB
#5738	`EnrichedLog`	netcoreapp3.1	4.1μs	2.09ns	8.11ns	0.0287	2.2 KB
#5738	`EnrichedLog`	net472	4.87μs	1.18ns	4.55ns	0.32	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.34μs	0.534ns	2ns	0.0161	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.74μs	0.866ns	3.24ns	0.015	0	1.14 KB
master	`SendReceive`	net472	2.13μs	0.731ns	2.73ns	0.183	0	1.16 KB
#5738	`SendReceive`	net6.0	1.37μs	1.29ns	4.99ns	0.0164	0	1.14 KB
#5738	`SendReceive`	netcoreapp3.1	1.73μs	0.744ns	2.88ns	0.0155	0	1.14 KB
#5738	`SendReceive`	net472	2.1μs	0.873ns	3.38ns	0.183	0.00105	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.61μs	0.818ns	2.95ns	0.0221	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.85μs	2.62ns	9.46ns	0.0213	1.65 KB
master	`EnrichedLog`	net472	4.41μs	2.77ns	10.4ns	0.323	2.04 KB
#5738	`EnrichedLog`	net6.0	2.7μs	1.16ns	4.5ns	0.0216	1.6 KB
#5738	`EnrichedLog`	netcoreapp3.1	3.95μs	1.14ns	4.11ns	0.0217	1.65 KB
#5738	`EnrichedLog`	net472	4.34μs	3.42ns	13.3ns	0.323	2.04 KB

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

Slower ⚠️ in #5738

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1	1.139	550.70	627.17

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	396ns	0.239ns	0.927ns	0.00818	576 B
master	`StartFinishSpan`	netcoreapp3.1	551ns	0.495ns	1.79ns	0.0077	576 B
master	`StartFinishSpan`	net472	636ns	0.44ns	1.7ns	0.0917	578 B
master	`StartFinishScope`	net6.0	484ns	0.263ns	1.02ns	0.00986	696 B
master	`StartFinishScope`	netcoreapp3.1	715ns	0.365ns	1.41ns	0.00932	696 B
master	`StartFinishScope`	net472	814ns	0.662ns	2.56ns	0.104	658 B
#5738	`StartFinishSpan`	net6.0	397ns	0.482ns	1.87ns	0.00814	576 B
#5738	`StartFinishSpan`	netcoreapp3.1	629ns	1.98ns	7.66ns	0.00744	576 B
#5738	`StartFinishSpan`	net472	691ns	0.693ns	2.68ns	0.0918	578 B
#5738	`StartFinishScope`	net6.0	475ns	0.186ns	0.671ns	0.00977	696 B
#5738	`StartFinishScope`	netcoreapp3.1	770ns	0.506ns	1.89ns	0.00934	696 B
#5738	`StartFinishScope`	net472	851ns	0.471ns	1.83ns	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	674ns	0.387ns	1.5ns	0.00986	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	921ns	0.729ns	2.82ns	0.00927	696 B
master	`RunOnMethodBegin`	net472	1.08μs	0.482ns	1.87ns	0.104	658 B
#5738	`RunOnMethodBegin`	net6.0	662ns	0.157ns	0.609ns	0.00959	696 B
#5738	`RunOnMethodBegin`	netcoreapp3.1	933ns	0.225ns	0.841ns	0.00938	696 B
#5738	`RunOnMethodBegin`	net472	1.09μs	0.27ns	1.05ns	0.104	658 B

andrewlock · 2024-06-25T15:56:36Z

Throughput/Crank Report:zap:

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 (5738) (10.839M)   : 0, 10839499
    master (11.705M)   : 0, 11705096
    benchmarks/2.9.0 (12.028M)   : 0, 12027804

    section Automatic
    This PR (5738) (7.464M)   : 0, 7464442
    master (7.833M)   : 0, 7832844
    benchmarks/2.9.0 (8.471M)   : 0, 8471115

    section Trace stats
    master (8.082M)   : 0, 8081964

    section Manual
    This PR (5738) (9.710M)   : 0, 9710455
    master (9.998M)   : 0, 9998100

    section Manual + Automatic
    This PR (5738) (7.135M)   : 0, 7135008
    master (7.346M)   : 0, 7345601

    section Version Conflict
    master (6.643M)   : 0, 6642934

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5738) (9.689M)   : 0, 9688856
    master (9.535M)   : 0, 9534826
    benchmarks/2.9.0 (9.556M)   : 0, 9555936

    section Automatic
    This PR (5738) (6.701M)   : 0, 6701428
    master (6.547M)   : 0, 6546581

    section Trace stats
    master (6.947M)   : 0, 6947401

    section Manual
    This PR (5738) (8.318M)   : 0, 8317505
    master (8.293M)   : 0, 8292792

    section Manual + Automatic
    This PR (5738) (6.207M)   : 0, 6207003
    master (6.185M)   : 0, 6184603

    section Version Conflict
    master (5.716M)   : 0, 5716316

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5738) (10.101M)   : 0, 10100755
    master (10.415M)   : 0, 10414832
    benchmarks/2.9.0 (9.885M)   : 0, 9885158

    section Automatic
    This PR (5738) (7.178M)   : 0, 7177794
    master (7.218M)   : 0, 7218121
    benchmarks/2.9.0 (6.977M)   : 0, 6977395

    section Trace stats
    master (7.576M)   : 0, 7576347

    section Manual
    This PR (5738) (8.951M)   : 0, 8950838
    master (9.045M)   : 0, 9045472

    section Manual + Automatic
    This PR (5738) (6.952M)   : 0, 6951664
    master (6.817M)   : 0, 6816758

    section Version Conflict
    master (6.066M)   : 0, 6066034

robertpi · 2024-06-27T12:40:23Z

tracer/build/_build/Build.Utilities.cs

-                        excludeFile: file => !Path.GetFileNameWithoutExtension(file.FullName).EndsWith(".received"));
-
-                    DeleteDirectory(extractLocation);
+                    try


Because this nuke task kept failing for me, it appears that in some cases we download artifacts that aren't zips, but I couldn't figure out how to filter them.

daniel-romano-DD · 2024-07-04T14:47:49Z

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

@@ -55,7 +60,7 @@ public SecuritySettings(IConfigurationSource? source, IConfigurationTelemetry te
            WafTimeoutMicroSeconds = (ulong)config
                                           .WithKeys(ConfigurationKeys.AppSec.WafTimeout)
                                           .GetAs<int>(
-                                                getDefaultValue: () => 100_000, // Default timeout of 100 ms, only extreme conditions should cause timeout
+                                                getDefaultValue: _ => 100_000, // Default timeout of 100 ms, only extreme conditions should cause timeout


Why this change? Out of curiosity

daniel-romano-DD · 2024-07-04T14:57:55Z

tracer/test/Datadog.Trace.Security.Unit.Tests/SecurityUserTrackingModeTests.cs

+    // This test suite demonstrates that these two booleans are correctly calculated from the above table.
+    // Local values can't be null, as the default is calculated within the framework for local config
+    // telemetry.
+    // Remote config can be null, and this tell us remote config is not active.


This is super helpful 👍

daniel-romano-DD · 2024-07-04T14:59:14Z

tracer/test/Datadog.Trace.Tests/Telemetry/config_norm_rules.json

@@ -77,7 +77,7 @@
  "tracer_instance_count": "trace_instance_count",
  "trace.db.client.split-by-instance": "trace_db_client_split_by_instance",
  "trace.db.client.split-by-instance.type.suffix": "trace_db_client_split_by_instance_type_suffix",
-  "trace.http.client.split-by-domain": "trace_http_client_split_by_domain",
+  "trace.http.client.split-by-domain" : "trace_http_client_split_by_domain",


I think the extra space slipped in

daniel-romano-DD

Didn't delve to the bottom of the logic, but coding looks good to me.

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

.../AutoInstrumentation/AspNetCore/UserEvents/AuthenticationHttpContextExtensionsIntegration.cs

NachoEchevarria · 2024-07-04T16:37:42Z

.../src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNetCore/UserEvents/UserEventsCommon.cs

+            // we want to end up with a string of the form anon_0c76692372ebf01a7da6e9570fb7d0a1
+            // 37 is 5 prefix character plus 32 hexadecimal digits (presenting 16 bytes)
+            // stackalloc to avoid intermediate heap allocation
+            var stringChars = stackalloc char[37];


...tName=login.auto.success_bodyString=Input.UserName=TestUser&Input.Password=test.verified.txt

NachoEchevarria

LGTM!

datadog-ddstaging · 2024-07-05T12:47:00Z

Datadog Report

Branch report: robert/asm/user-id-collection-modes
Commit report: be2cd28
Test service: dd-trace-dotnet

❌ 8 Failed (0 Known Flaky), 346276 Passed, 1696 Skipped, 14h 27m 12.63s Total Time

❌ Failed Tests (8)

This report shows up to 5 failed tests.

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

datadog-ddstaging · 2024-07-05T12:47:12Z

Datadog Report

Branch report: robert/asm/user-id-collection-modes
Commit report: be2cd28
Test service: dd-trace-dotnet

❌ 8 Failed (0 Known Flaky), 346276 Passed, 1696 Skipped, 14h 27m 12.63s Total Time

❌ Failed Tests (8)

This report shows up to 5 failed tests.

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

datadog-ddstaging · 2024-07-05T12:47:22Z

Datadog Report

Branch report: robert/asm/user-id-collection-modes
Commit report: be2cd28
Test service: dd-trace-dotnet

❌ 8 Failed (0 Known Flaky), 346276 Passed, 1696 Skipped, 14h 27m 12.63s Total Time

❌ Failed Tests (8)

This report shows up to 5 failed tests.

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

datadog-ddstaging · 2024-07-05T12:47:46Z

Datadog Report

Branch report: robert/asm/user-id-collection-modes
Commit report: be2cd28
Test service: dd-trace-dotnet

❌ 8 Failed (0 Known Flaky), 346276 Passed, 1696 Skipped, 14h 27m 12.63s Total Time

❌ Failed Tests (8)

This report shows up to 5 failed tests.

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

TestChangeUserIdCollection - Datadog.Trace.Security.IntegrationTests.Rcm.AspNetCore5AsmFeatureUserIdSecurityEnabled - Details

Expand for error

 Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.received.txt
 Verified: Security.AspNetCore5AsmFeatureUserIdSecurityEnabled._.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

andrewlock · 2024-08-06T16:36:46Z

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

+                                               .AsString(
+                                                    isPresent => isPresent ? UserTrackingDisabled : GetLegacyUserTracking(),


This is a change to one of the core design parts of the ConfigurationBuilder and I'm not sure if we need it 😬 Also, it's kinda hard to follow, what behaviour are we trying to get here? I'm 90% sure we can do it without changing the builder (but I could be wrong)...

The config setting name was changed, if the new value is present we should use it, otherwise we should look for the old one.

The change allows us to do this all inside the configuration combinators, we could implement this logic without the changer but it would mean moving some of the defaults outside of the combinators so the changes would be visible to the telemetry.

The config setting name was changed, if the new value is present we should use it, otherwise we should look for the old one.

We have that capability already, by passing the key values to the WithKeys() and it's handled correctly by telemetry in that case 🤔

e.g. see here, where we have two "deprecated" values that we fallback to if the newer one isn't present:

dd-trace-dotnet/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs

Line 394 in 0f912fe

.WithKeys(ConfigurationKeys.PropagationStyleInject, "DD_PROPAGATION_STYLE_INJECT", ConfigurationKeys.PropagationStyle)

As discussed on slack used .AsStringResult and rolled back the changes.

update config keys Add missing usr.id telementry and correct some test cases unit tests working, integration failing puzzling stuff working as expected, finally more tests add back missing code test almost working, just need iron stuff and remove logs include correct snapshot last changes before squashing really final tweaks this time more fixes correct error caused by change in RCM subscriptions make tests work for new modes corrections from system tests

bouwkast

Focused mainly on the IDM stuff but looked good to me, thanks!

robertpi changed the title ~~Robert/asm/user id collection modes~~ [ASM] Changes to the collection of usr.id for authenticated clients Jun 27, 2024

robertpi added the area:asm label Jun 27, 2024

robertpi marked this pull request as ready for review June 27, 2024 12:34

robertpi requested review from a team as code owners June 27, 2024 12:34

robertpi commented Jun 27, 2024

View reviewed changes

robertpi marked this pull request as draft June 27, 2024 13:10

robertpi marked this pull request as ready for review June 27, 2024 15:31

robertpi force-pushed the robert/asm/user-id-collection-modes branch 2 times, most recently from 802a5ea to 474ee8e Compare July 4, 2024 14:31

daniel-romano-DD reviewed Jul 4, 2024

View reviewed changes

daniel-romano-DD approved these changes Jul 4, 2024

View reviewed changes

NachoEchevarria reviewed Jul 4, 2024

View reviewed changes

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

NachoEchevarria reviewed Jul 4, 2024

View reviewed changes

.../AutoInstrumentation/AspNetCore/UserEvents/AuthenticationHttpContextExtensionsIntegration.cs Outdated Show resolved Hide resolved

NachoEchevarria reviewed Jul 4, 2024

View reviewed changes

.../AutoInstrumentation/AspNetCore/UserEvents/AuthenticationHttpContextExtensionsIntegration.cs Show resolved Hide resolved

NachoEchevarria reviewed Jul 4, 2024

View reviewed changes

NachoEchevarria reviewed Jul 5, 2024

View reviewed changes

...tName=login.auto.success_bodyString=Input.UserName=TestUser&Input.Password=test.verified.txt Outdated Show resolved Hide resolved

NachoEchevarria approved these changes Jul 5, 2024

View reviewed changes

robertpi force-pushed the robert/asm/user-id-collection-modes branch from 2003580 to e9f92af Compare July 10, 2024 13:29

robertpi force-pushed the robert/asm/user-id-collection-modes branch from e9f92af to 387c676 Compare August 6, 2024 14:13

andrewlock reviewed Aug 6, 2024

View reviewed changes

robertpi added 10 commits August 9, 2024 16:00

make RC stuff conform to snake case standard

af9a0ca

fix up integration test results

0675014

fix unit tests build

eb1d602

response to comments

069e09a

fix god damn build errors

1162226

fix snapshots again

f43b6b5

fix snapshots again

dbad36a

finally the right combination?

465f65e

Adapt config system to Andrews comments

f2d8c04

robertpi force-pushed the robert/asm/user-id-collection-modes branch from c0f8b8a to f2d8c04 Compare August 9, 2024 14:28

fix unit tests build

ffadd32

bouwkast approved these changes Aug 12, 2024

View reviewed changes

robertpi merged commit 8bf5368 into master Aug 12, 2024
64 of 69 checks passed

robertpi deleted the robert/asm/user-id-collection-modes branch August 12, 2024 12:36

github-actions bot added this to the vNext-v3 milestone Aug 12, 2024

		.AsString(
		isPresent => isPresent ? UserTrackingDisabled : GetLegacyUserTracking(),

[ASM] Changes to the collection of usr.id for authenticated clients #5738

[ASM] Changes to the collection of usr.id for authenticated clients #5738

Conversation

robertpi commented Jun 25, 2024 • edited Loading

Summary of changes

Reason for change

Test coverage

Other details

github-actions bot commented Jun 25, 2024 • edited Loading

Snapshots difference summary

datadog-ddstaging bot commented Jun 25, 2024 • edited Loading

Datadog Report

andrewlock commented Jun 25, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Jun 25, 2024 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Faster 🎉 in #5738

Raw results

Raw results

Raw results

More allocations ⚠️ in #5738

Fewer allocations 🎉 in #5738

Raw results

andrewlock commented Jun 25, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #5738

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #5738

Raw results

Raw results

andrewlock commented Jun 25, 2024 • edited Loading

Throughput/Crank Report:zap:

robertpi Jun 27, 2024

Choose a reason for hiding this comment

daniel-romano-DD Jul 4, 2024

Choose a reason for hiding this comment

daniel-romano-DD Jul 4, 2024

Choose a reason for hiding this comment

daniel-romano-DD Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

daniel-romano-DD left a comment

Choose a reason for hiding this comment

NachoEchevarria Jul 4, 2024

Choose a reason for hiding this comment

NachoEchevarria left a comment

Choose a reason for hiding this comment

datadog-ddstaging bot commented Jul 5, 2024

Datadog Report

❌ Failed Tests (8)

datadog-ddstaging bot commented Jul 5, 2024

Datadog Report

❌ Failed Tests (8)

datadog-ddstaging bot commented Jul 5, 2024

Datadog Report

❌ Failed Tests (8)

datadog-ddstaging bot commented Jul 5, 2024

Datadog Report

❌ Failed Tests (8)

andrewlock Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

robertpi Aug 7, 2024

Choose a reason for hiding this comment

andrewlock Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

robertpi Aug 9, 2024

Choose a reason for hiding this comment

robertpi commented Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 25, 2024 •

edited

Loading

datadog-ddstaging bot commented Jun 25, 2024 •

edited

Loading

andrewlock commented Jun 25, 2024 •

edited

Loading

andrewlock commented Jun 25, 2024 •

edited

Loading

andrewlock commented Jun 25, 2024 •

edited

Loading

andrewlock commented Jun 25, 2024 •

edited

Loading

daniel-romano-DD Jul 4, 2024 •

edited

Loading

andrewlock Aug 6, 2024 •

edited

Loading

andrewlock Aug 7, 2024 •

edited

Loading