[ASM][ATO] Adapt v3 new login tags and fix signup tags #6302

anna-git · 2024-11-18T16:56:44Z

Summary of changes

Fix signup tags that weren't correct.
Add login tags on failure and other internal tagss as per RFC

Reason for change

Implementation details

Test coverage

Other details

github-actions · 2024-11-18T16:57:30Z

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.

5 occurrences of :

+      appsec.events.users.login.success.usr.login: TestUser,

5 occurrences of :

+      _dd.appsec.usr.id: Guid_2,
+      _dd.appsec.usr.login: TestUser,

1 occurrences of :

+      appsec.events.users.login.failure.usr.login: anon_eb97d409396a3e5392936dad92b909da,

2 occurrences of :

+      _dd.appsec.usr.id: anon_7bcd1c9fc4f6e4c2460e0ad38d6ad0d9,
+      _dd.appsec.usr.login: anon_eb97d409396a3e5392936dad92b909da,

1 occurrences of :

-      appsec.events.users.login.failure.usr.id: anon_c34de12d00b78a9977da847b7e55202e,
+      appsec.events.users.login.failure.usr.login: anon_c34de12d00b78a9977da847b7e55202e,

1 occurrences of :

+      _dd.appsec.usr.login: anon_c34de12d00b78a9977da847b7e55202e,

1 occurrences of :

+      appsec.events.users.login.success.usr.login: anon_eb97d409396a3e5392936dad92b909da,

3 occurrences of :

+      appsec.events.users.login.failure.usr.login: TestUser,

3 occurrences of :

+      _dd.appsec.usr.id: Guid_1,
+      _dd.appsec.usr.login: TestUser,

3 occurrences of :

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

3 occurrences of :

+      _dd.appsec.usr.login: NoSuchUser,

andrewlock · 2024-11-18T17:31:02Z

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 (6302) - mean (68ms)  : 66, 71
     .   : milestone, 68,
    master - mean (69ms)  : 65, 72
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6302) - mean (980ms)  : 954, 1005
     .   : milestone, 980,
    master - mean (978ms)  : 952, 1004
     .   : milestone, 978,

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

    section CallTarget+Inlining+NGEN
    This PR (6302) - mean (675ms)  : 660, 691
     .   : milestone, 675,
    master - mean (680ms)  : 663, 697
     .   : milestone, 680,

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

    section CallTarget+Inlining+NGEN
    This PR (6302) - mean (635ms)  : 616, 654
     .   : milestone, 635,
    master - mean (639ms)  : 624, 653
     .   : milestone, 639,

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

    section CallTarget+Inlining+NGEN
    This PR (6302) - mean (1,106ms)  : 1068, 1144
     .   : milestone, 1106,
    master - mean (1,104ms)  : 1076, 1132
     .   : milestone, 1104,

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

    section CallTarget+Inlining+NGEN
    This PR (6302) - mean (871ms)  : 845, 897
     .   : milestone, 871,
    master - mean (874ms)  : 847, 901
     .   : milestone, 874,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6302) - mean (267ms)  : 262, 271
     .   : milestone, 267,
    master - mean (267ms)  : 263, 271
     .   : milestone, 267,

    section CallTarget+Inlining+NGEN
    This PR (6302) - mean (852ms)  : 816, 887
     .   : milestone, 852,
    master - mean (856ms)  : 823, 888
     .   : milestone, 856,

datadog-ddstaging · 2024-11-18T17:34:52Z

Datadog Report

Branch report: anna/asm/collect-login-and-adjustementstorfc
Commit report: 484d6f8
Test service: dd-trace-dotnet

✅ 0 Failed, 458777 Passed, 2833 Skipped, 19h 47m 9.09s Total Time

andrewlock · 2024-11-18T18:19:23Z

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 (6302) (11.152M)   : 0, 11151575
    master (11.199M)   : 0, 11199251
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6302) (7.104M)   : 0, 7104324
    master (7.234M)   : 0, 7234132
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.559M)   : 0, 7558761

    section Manual
    master (11.028M)   : 0, 11028498

    section Manual + Automatic
    This PR (6302) (6.580M)   : 0, 6580439
    master (6.711M)   : 0, 6710751

    section DD_TRACE_ENABLED=0
    master (10.230M)   : 0, 10230285

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6302) (9.584M)   : 0, 9584071
    master (9.417M)   : 0, 9416704
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6302) (6.366M)   : 0, 6366278
    master (6.437M)   : 0, 6437027

    section Trace stats
    master (6.619M)   : 0, 6618620

    section Manual
    master (9.205M)   : 0, 9205053

    section Manual + Automatic
    This PR (6302) (6.007M)   : 0, 6007195
    master (5.852M)   : 0, 5852490

    section DD_TRACE_ENABLED=0
    master (8.872M)   : 0, 8871934

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6302) (9.780M)   : 0, 9780199
    master (9.905M)   : 0, 9904680
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6302) (6.291M)   : 0, 6290574
    master (6.337M)   : 0, 6337163
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (6.938M)   : 0, 6938182

    section Manual
    master (9.708M)   : 0, 9708036

    section Manual + Automatic
    This PR (6302) (5.956M)   : 0, 5956098
    master (5.665M)   : 0, 5664554

    section DD_TRACE_ENABLED=0
    master (9.111M)   : 0, 9111450

andrewlock · 2024-11-18T23:29:13Z

Benchmarks Report for tracer 🐌

Benchmarks for #6302 compared to master:

3 benchmarks are faster, with geometric mean 1.135
1 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.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartStopWithChild`	net6.0	8.16μs	45.8ns	290ns	0.0126	0.00422	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10.2μs	53.6ns	268ns	0.0148	0.00493	0	5.8 KB
master	`StartStopWithChild`	net472	16.4μs	42.1ns	158ns	1.05	0.312	0.104	6.22 KB
#6302	`StartStopWithChild`	net6.0	8.18μs	44ns	278ns	0.0157	0.00783	0	5.61 KB
#6302	`StartStopWithChild`	netcoreapp3.1	10.2μs	55ns	302ns	0.0259	0.0104	0	5.8 KB
#6302	`StartStopWithChild`	net472	16.4μs	62.6ns	243ns	1.04	0.321	0.0987	6.21 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	511μs	219ns	820ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	668μs	737ns	2.85μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	857μs	1.25μs	4.84μs	0.428	3.3 KB
#6302	`WriteAndFlushEnrichedTraces`	net6.0	505μs	472ns	2.5μs	0	2.7 KB
#6302	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	651μs	1.94μs	7.52μs	0	2.7 KB
#6302	`WriteAndFlushEnrichedTraces`	net472	843μs	419ns	1.51μ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	148μs	861ns	7.99μs	0.14	14.47 KB
master	`SendRequest`	netcoreapp3.1	171μs	1.19μs	11.8μs	0.165	17.27 KB
master	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#6302	`SendRequest`	net6.0	145μs	814ns	6.41μs	0.164	14.47 KB
#6302	`SendRequest`	netcoreapp3.1	167μs	962ns	7.51μs	0.173	17.27 KB
#6302	`SendRequest`	net472	8.36E‑05ns	5.83E‑05ns	0.000226ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #6302

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0	41.82 KB	41.44 KB	-376 B	-0.90%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	582μs	2.68μs	9.66μs	0.556	0	0	41.82 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	661μs	3.29μs	15.1μs	0.331	0	0	41.79 KB
master	`WriteAndFlushEnrichedTraces`	net472	860μs	4.24μs	18μs	8.19	2.59	0.431	53.27 KB
#6302	`WriteAndFlushEnrichedTraces`	net6.0	533μs	2.63μs	12.1μs	0.539	0	0	41.44 KB
#6302	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	648μs	3.52μs	19.6μs	0.336	0	0	41.77 KB
#6302	`WriteAndFlushEnrichedTraces`	net472	815μs	3.67μs	16μs	8.22	2.47	0.411	53.26 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.24μs	1.52ns	5.89ns	0.0142	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.68μs	0.676ns	2.53ns	0.0135	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.11μs	2.33ns	9.01ns	0.157	0.00106	987 B
#6302	`ExecuteNonQuery`	net6.0	1.23μs	1.2ns	4.64ns	0.014	0	1.02 KB
#6302	`ExecuteNonQuery`	netcoreapp3.1	1.73μs	0.573ns	2.14ns	0.0139	0	1.02 KB
#6302	`ExecuteNonQuery`	net472	2.08μs	1.15ns	4.3ns	0.156	0.00103	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.13μs	0.771ns	2.89ns	0.0136	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.56μs	2.7ns	10.1ns	0.0132	976 B
master	`CallElasticsearch`	net472	2.56μs	1.88ns	7.05ns	0.157	995 B
master	`CallElasticsearchAsync`	net6.0	1.27μs	0.535ns	2.07ns	0.0134	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.61μs	1.3ns	5.02ns	0.0137	1.02 KB
master	`CallElasticsearchAsync`	net472	2.65μs	1.85ns	6.91ns	0.166	1.05 KB
#6302	`CallElasticsearch`	net6.0	1.24μs	0.679ns	2.54ns	0.0137	976 B
#6302	`CallElasticsearch`	netcoreapp3.1	1.62μs	0.504ns	1.82ns	0.013	976 B
#6302	`CallElasticsearch`	net472	2.48μs	1.78ns	6.91ns	0.157	995 B
#6302	`CallElasticsearchAsync`	net6.0	1.29μs	0.411ns	1.59ns	0.013	952 B
#6302	`CallElasticsearchAsync`	netcoreapp3.1	1.67μs	0.635ns	2.38ns	0.0134	1.02 KB
#6302	`CallElasticsearchAsync`	net472	2.71μs	1.56ns	5.62ns	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.36μs	0.562ns	2.18ns	0.0129	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.65μs	0.696ns	2.51ns	0.0124	952 B
master	`ExecuteAsync`	net472	1.82μs	0.398ns	1.43ns	0.145	915 B
#6302	`ExecuteAsync`	net6.0	1.25μs	0.608ns	2.36ns	0.013	952 B
#6302	`ExecuteAsync`	netcoreapp3.1	1.57μs	1.17ns	4.52ns	0.0125	952 B
#6302	`ExecuteAsync`	net472	1.82μs	0.409ns	1.59ns	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.48μs	1.58ns	5.9ns	0.0315	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.33μs	2.31ns	8.63ns	0.0374	2.85 KB
master	`SendAsync`	net472	7.31μs	3.7ns	14.3ns	0.493	3.12 KB
#6302	`SendAsync`	net6.0	4.35μs	1.58ns	6.11ns	0.0327	2.31 KB
#6302	`SendAsync`	netcoreapp3.1	5.46μs	28.5ns	140ns	0.0373	2.85 KB
#6302	`SendAsync`	net472	7.34μs	2.78ns	10.8ns	0.496	3.12 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.52μs	2.22ns	8.6ns	0.0227	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.27μs	0.632ns	2.28ns	0.0216	1.64 KB
master	`EnrichedLog`	net472	2.73μs	1.34ns	5.19ns	0.249	1.57 KB
#6302	`EnrichedLog`	net6.0	1.61μs	1.87ns	6.75ns	0.0234	1.64 KB
#6302	`EnrichedLog`	netcoreapp3.1	2.21μs	1.19ns	4.45ns	0.0222	1.64 KB
#6302	`EnrichedLog`	net472	2.71μs	0.815ns	2.94ns	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	121μs	163ns	633ns	0	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	124μs	157ns	586ns	0.0621	0	4.28 KB
master	`EnrichedLog`	net472	152μs	107ns	387ns	0.675	0.225	4.46 KB
#6302	`EnrichedLog`	net6.0	121μs	187ns	724ns	0.0605	0	4.28 KB
#6302	`EnrichedLog`	netcoreapp3.1	124μs	216ns	838ns	0	0	4.28 KB
#6302	`EnrichedLog`	net472	152μs	89.1ns	321ns	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.07μs	1.18ns	4.56ns	0.0305	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.22μs	2.64ns	10.2ns	0.0296	2.2 KB
master	`EnrichedLog`	net472	5.01μs	1.2ns	4.64ns	0.319	2.02 KB
#6302	`EnrichedLog`	net6.0	3.05μs	1.13ns	4.39ns	0.0306	2.2 KB
#6302	`EnrichedLog`	netcoreapp3.1	4.15μs	5.28ns	19.7ns	0.0289	2.2 KB
#6302	`EnrichedLog`	net472	4.86μs	1.39ns	5.36ns	0.32	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.39μs	0.707ns	2.74ns	0.016	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.73μs	5.03ns	19.5ns	0.0154	1.14 KB
master	`SendReceive`	net472	2.16μs	2.04ns	7.88ns	0.183	1.16 KB
#6302	`SendReceive`	net6.0	1.3μs	0.352ns	1.27ns	0.0163	1.14 KB
#6302	`SendReceive`	netcoreapp3.1	1.73μs	0.745ns	2.88ns	0.0152	1.14 KB
#6302	`SendReceive`	net472	2.04μs	2.09ns	8.08ns	0.184	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.67μs	0.985ns	3.82ns	0.0227	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.93μs	2.47ns	9.55ns	0.0215	1.65 KB
master	`EnrichedLog`	net472	4.35μs	3.89ns	14ns	0.323	2.04 KB
#6302	`EnrichedLog`	net6.0	2.6μs	0.952ns	3.69ns	0.022	1.6 KB
#6302	`EnrichedLog`	netcoreapp3.1	3.87μs	2.51ns	9.04ns	0.0212	1.65 KB
#6302	`EnrichedLog`	net472	4.38μs	2.88ns	10.8ns	0.323	2.04 KB

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

Faster 🎉 in #6302

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net472	1.174	939.86	800.43
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.116	448.29	401.59
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1	1.115	775.76	695.81

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	448ns	0.279ns	1.08ns	0.00801	576 B
master	`StartFinishSpan`	netcoreapp3.1	560ns	0.304ns	1.1ns	0.00786	576 B
master	`StartFinishSpan`	net472	700ns	0.388ns	1.5ns	0.0915	578 B
master	`StartFinishScope`	net6.0	488ns	0.249ns	0.965ns	0.00979	696 B
master	`StartFinishScope`	netcoreapp3.1	776ns	0.426ns	1.65ns	0.00933	696 B
master	`StartFinishScope`	net472	940ns	0.499ns	1.93ns	0.104	658 B
#6302	`StartFinishSpan`	net6.0	402ns	0.299ns	1.16ns	0.00808	576 B
#6302	`StartFinishSpan`	netcoreapp3.1	582ns	0.612ns	2.37ns	0.00788	576 B
#6302	`StartFinishSpan`	net472	656ns	0.279ns	1.04ns	0.0918	578 B
#6302	`StartFinishScope`	net6.0	511ns	0.241ns	0.933ns	0.00983	696 B
#6302	`StartFinishScope`	netcoreapp3.1	696ns	0.484ns	1.81ns	0.00922	696 B
#6302	`StartFinishScope`	net472	800ns	0.399ns	1.44ns	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	690ns	1.36ns	5.29ns	0.00986	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	957ns	0.598ns	2.32ns	0.0092	696 B
master	`RunOnMethodBegin`	net472	1.14μs	0.616ns	2.39ns	0.104	658 B
#6302	`RunOnMethodBegin`	net6.0	664ns	0.864ns	3.35ns	0.00978	696 B
#6302	`RunOnMethodBegin`	netcoreapp3.1	923ns	0.644ns	2.41ns	0.00926	696 B
#6302	`RunOnMethodBegin`	net472	1.12μs	0.482ns	1.87ns	0.104	658 B

andrewlock · 2024-11-26T16:44:12Z

Benchmarks Report for appsec 🐌

Benchmarks for #6302 compared to master:

All benchmarks have the same speed
1 benchmarks have fewer allocations
2 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 - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	211μs	156ns	584ns	2.73	0	196.01 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	312μs	324ns	1.21μs	2.79	0	203.26 KB
master	`AllCycleSimpleBody`	net472	277μs	651ns	2.52μs	36.5	2.19	230.36 KB
master	`AllCycleMoreComplexBody`	net6.0	218μs	86ns	322ns	2.75	0	199.51 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	321μs	139ns	520ns	2.86	0	206.67 KB
master	`AllCycleMoreComplexBody`	net472	281μs	235ns	878ns	37.1	2.24	233.87 KB
master	`ObjectExtractorSimpleBody`	net6.0	142ns	0.195ns	0.73ns	0.00393	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	215ns	0.169ns	0.655ns	0.00374	0	272 B
master	`ObjectExtractorSimpleBody`	net472	169ns	0.104ns	0.36ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	2.88μs	2ns	7.22ns	0.0535	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.74μs	4.1ns	15.4ns	0.0486	0	3.69 KB
master	`ObjectExtractorMoreComplexBody`	net472	3.61μs	4.72ns	18.3ns	0.603	0.00542	3.8 KB
#6302	`AllCycleSimpleBody`	net6.0	213μs	222ns	859ns	2.81	0	196.51 KB
#6302	`AllCycleSimpleBody`	netcoreapp3.1	320μs	172ns	644ns	2.71	0	203.75 KB
#6302	`AllCycleSimpleBody`	net472	282μs	421ns	1.63μs	36.7	2.39	230.87 KB
#6302	`AllCycleMoreComplexBody`	net6.0	216μs	107ns	400ns	2.79	0	200.01 KB
#6302	`AllCycleMoreComplexBody`	netcoreapp3.1	317μs	129ns	481ns	2.86	0	207.17 KB
#6302	`AllCycleMoreComplexBody`	net472	286μs	520ns	2.01μs	37.2	2.43	234.39 KB
#6302	`ObjectExtractorSimpleBody`	net6.0	146ns	0.088ns	0.341ns	0.00394	0	280 B
#6302	`ObjectExtractorSimpleBody`	netcoreapp3.1	235ns	0.231ns	0.863ns	0.00376	0	272 B
#6302	`ObjectExtractorSimpleBody`	net472	164ns	0.113ns	0.407ns	0.0446	0	281 B
#6302	`ObjectExtractorMoreComplexBody`	net6.0	2.89μs	1.63ns	5.86ns	0.0535	0	3.78 KB
#6302	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.72μs	4.25ns	16.5ns	0.05	0	3.69 KB
#6302	`ObjectExtractorMoreComplexBody`	net472	3.62μs	2.02ns	7.57ns	0.602	0.00542	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.5μs	13.1ns	49ns	0.451	0	32.4 KB
master	`EncodeArgs`	netcoreapp3.1	54.7μs	22.4ns	83.9ns	0.44	0	32.4 KB
master	`EncodeArgs`	net472	66.8μs	44.3ns	171ns	5.15	0.0665	32.5 KB
master	`EncodeLegacyArgs`	net6.0	79.7μs	35.1ns	136ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	netcoreapp3.1	104μs	80.5ns	312ns	0	0	2.14 KB
master	`EncodeLegacyArgs`	net472	154μs	70.2ns	263ns	0.309	0	2.15 KB
#6302	`EncodeArgs`	net6.0	36.8μs	19.4ns	72.7ns	0.463	0	32.4 KB
#6302	`EncodeArgs`	netcoreapp3.1	54.1μs	33.9ns	127ns	0.43	0	32.4 KB
#6302	`EncodeArgs`	net472	65.7μs	82.5ns	320ns	5.16	0.0658	32.5 KB
#6302	`EncodeLegacyArgs`	net6.0	73.8μs	36.6ns	132ns	0	0	2.14 KB
#6302	`EncodeLegacyArgs`	netcoreapp3.1	107μs	408ns	1.58μs	0	0	2.15 KB
#6302	`EncodeLegacyArgs`	net472	154μs	145ns	563ns	0.308	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	182μs	233ns	902ns	0	2.44 KB
master	`RunWafRealisticBenchmark`	netcoreapp3.1	196μs	276ns	1.03μs	0	2.39 KB
master	`RunWafRealisticBenchmark`	net472	209μs	71.2ns	257ns	0.311	2.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net6.0	123μs	140ns	542ns	0	1.47 KB
master	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	130μs	196ns	732ns	0	1.46 KB
master	`RunWafRealisticBenchmarkWithAttack`	net472	139μs	68ns	263ns	0.209	1.49 KB
#6302	`RunWafRealisticBenchmark`	net6.0	184μs	96.8ns	349ns	0	2.44 KB
#6302	`RunWafRealisticBenchmark`	netcoreapp3.1	196μs	173ns	649ns	0	2.39 KB
#6302	`RunWafRealisticBenchmark`	net472	208μs	140ns	542ns	0.312	2.46 KB
#6302	`RunWafRealisticBenchmarkWithAttack`	net6.0	122μs	63.3ns	245ns	0	1.47 KB
#6302	`RunWafRealisticBenchmarkWithAttack`	netcoreapp3.1	133μs	91.3ns	354ns	0	1.46 KB
#6302	`RunWafRealisticBenchmarkWithAttack`	net472	138μs	56.8ns	205ns	0.207	1.48 KB

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

More allocations ⚠️ in #6302

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1	253.17 KB	264.21 KB	11.04 KB	4.36%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0	255.67 KB	266.14 KB	10.46 KB	4.09%

Fewer allocations 🎉 in #6302

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472	61.66 KB	59.07 KB	-2.59 KB	-4.20%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`StringConcatBenchmark`	net6.0	59.1μs	724ns	7.06μs	43.44 KB
master	`StringConcatBenchmark`	netcoreapp3.1	62.6μs	751ns	7.32μs	42.64 KB
master	`StringConcatBenchmark`	net472	36.9μs	78.3ns	293ns	61.66 KB
master	`StringConcatAspectBenchmark`	net6.0	310μs	4.87μs	45.9μs	255.67 KB
master	`StringConcatAspectBenchmark`	netcoreapp3.1	335μs	1.55μs	9.67μs	253.17 KB
master	`StringConcatAspectBenchmark`	net472	274μs	5.18μs	50.2μs	278.53 KB
#6302	`StringConcatBenchmark`	net6.0	58.5μs	675ns	6.68μs	43.44 KB
#6302	`StringConcatBenchmark`	netcoreapp3.1	61.5μs	797ns	7.89μs	42.64 KB
#6302	`StringConcatBenchmark`	net472	38μs	90.7ns	339ns	59.07 KB
#6302	`StringConcatAspectBenchmark`	net6.0	320μs	1.75μs	10.1μs	266.14 KB
#6302	`StringConcatAspectBenchmark`	netcoreapp3.1	348μs	1.49μs	6.84μs	264.21 KB
#6302	`StringConcatAspectBenchmark`	net472	272μs	4.83μs	46.1μs	278.53 KB

NachoEchevarria · 2024-11-29T17:09:30Z

...Profiler/AutoInstrumentation/AspNetCore/UserEvents/SignInManagerPasswordSignInIntegration.cs

-                }
-
+                var loginAnon = UserEventsCommon.Anonymize(login);
+                tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserLogin, loginAnon!);


is the ! in loginAnon! required? method Anonymize returns a non nullable string

yes, I m not sure why the compiler doesn't like it with the func tryAddTag

I suspect you can remove this now? 🤔 GetAnonId used to return a string?, but Anonymize returns string.

yes removed in 03da607

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

andrewlock

LGTM in general, just some small suggestions

andrewlock · 2024-12-10T13:27:32Z

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

+                }
+                else
+                {
+                    processPii = val => val;


nit: rather than allocating a lambda here, I would be inclined to make processPii nullable and execute it like this:

var userId = processPii?.Invoke(claim.Value) ?? claim.Value;

Changed in 03da607 in all files with this mechanism, thanks

andrewlock · 2024-12-10T13:32:07Z

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

+        var encodedBytes = Encoding.UTF8.GetBytes(id);
+        #if NET6_0_OR_GREATER
+        var destination = new Span<byte>(new byte[32]);
+        var successfullyHashed = SHA256.TryHashData(new ReadOnlySpan<byte>(encodedBytes), destination, out var bytesWritten);


Seeing as you're going to the trouble to differentiate for .NET 6, I think you should be able to make this pretty much non-allocating. e.g. you can potentially use this overload of GetBytes (though that might be more hassle than you want).

You can definitely stackalloc the destination bytes though for example:

Span<byte> destination = stackalloc byte[32];

I changed it here in commit 03da607 but I don't know if there's fewer allocations?

andrewlock · 2024-12-10T13:33:46Z

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

@@ -49,7 +59,47 @@ internal static class UserEventsCommon
            return new string(stringChars, 0, 37);
        }

-        return null;
+        Log.Warning<int>("Couldn't anonymize user information (login or id), byteArray length was {BytesWritten}", bytesWritten);


Is this method called at every login or for every request with identity information? If the latter, then this could be invoked a lot, so we might want to make this debug. If it's only on login, then it's probably ok, but given there's no action or anything for the user to do, I don't know if it should just be Debug anyway?

Yes, it should never happen but.. if it ever happened indeed it would be for every authenticated request (an incoming PR should collect user info for every authenticated request) so changed in 03da607

andrewlock · 2024-12-10T13:36:37Z

....Trace/ClrProfiler/AutoInstrumentation/AspNetCore/UserEvents/UserManagerCreateIntegration.cs

+                tryAddTag(Tags.AppSec.EventsUsers.SignUpEvent.UserId, userId!);
+                tryAddTag(Tags.AppSec.EventsUsers.InternalUserId, userId!);


The ! is because userId could be null here, right? I would suggest that either we should only add these tags if userId is non-null, or we should do a null coalesce?

Suggested change

tryAddTag(Tags.AppSec.EventsUsers.SignUpEvent.UserId, userId!);

tryAddTag(Tags.AppSec.EventsUsers.InternalUserId, userId!);

tryAddTag(Tags.AppSec.EventsUsers.SignUpEvent.UserId, userId ?? string.Empty);

tryAddTag(Tags.AppSec.EventsUsers.InternalUserId, userId ?? string.Empty);

Good catch thank you!! I actually removed those two lines, as they were in the if for foundUserId 03da607#diff-45833ea1519335c185aff6d47c0d0e1ce4f6188c5c8f55a4d541144abebe9963R31

tracer/test/Datadog.Trace.Tests/Telemetry/Metrics/common_metrics.json

andrewlock · 2024-12-10T14:13:30Z

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


+    internal static string? GetLogin(IIdentityUser? user) => user?.UserName ?? user?.Email;
+
+    internal static unsafe string Anonymize(string id)


Does this really need to be unsafe? 🤔

unfortunately stackalloc seems to need the unsafe keyword..

stackalloc doesn't need the unsafe keyword as long as you explicitly set the variable type to Span<T> and don't use var 🙂 https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/stackalloc

oh cool I didnt know that thanks
although it's gotta be compatible net461... so doesnt seem like I can really declare a Span<T> here

andrewlock · 2024-12-10T14:15:38Z

...Profiler/AutoInstrumentation/AspNetCore/UserEvents/SignInManagerPasswordSignInIntegration.cs

-                }
-
+                var loginAnon = UserEventsCommon.Anonymize(login);
+                tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserLogin, loginAnon!);


I suspect you can remove this now? 🤔 GetAnonId used to return a string?, but Anonymize returns string.

Adapt tests

## Summary of changes Fix signup tags that weren't correct. Add login tags on failure and other internal tagss as per [RFC](https://docs.google.com/document/d/1RT38U6dTTcB-8muiYV4-aVDCsT_XrliyakjtAPyjUpw/edit?tab=t.0#heading=h.dy0zssue7nq2) ## Reason for change ## Implementation details ## Test coverage ## Other details

github-actions bot added the area:asm label Nov 18, 2024

anna-git force-pushed the anna/asm/collect-login-and-adjustementstorfc branch from 655c1a5 to 14f86c6 Compare November 19, 2024 14:43

anna-git changed the title ~~[ASM] Fix user tags~~ [ASM] Adapt login tags and fix signup tags Nov 19, 2024

anna-git force-pushed the anna/asm/ato-headers-success branch 2 times, most recently from 8fa918f to 4f47cce Compare November 25, 2024 12:25

Base automatically changed from anna/asm/ato-headers-success to master November 25, 2024 16:43

anna-git force-pushed the anna/asm/collect-login-and-adjustementstorfc branch from 795342c to 56b3683 Compare November 26, 2024 15:47

anna-git mentioned this pull request Nov 28, 2024

[ASM][ATO] deactivate old login to be able to merge DataDog/system-tests#3585

Merged

7 tasks

anna-git marked this pull request as ready for review November 29, 2024 12:24

anna-git requested review from a team as code owners November 29, 2024 12:24

anna-git changed the title ~~[ASM] Adapt login tags and fix signup tags~~ [ASM] Adapt v3 new login tags and fix signup tags Nov 29, 2024

NachoEchevarria reviewed Nov 29, 2024

View reviewed changes

NachoEchevarria reviewed Dec 2, 2024

View reviewed changes

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

NachoEchevarria approved these changes Dec 3, 2024

View reviewed changes

anna-git force-pushed the anna/asm/collect-login-and-adjustementstorfc branch 2 times, most recently from 5d77cfc to 712e641 Compare December 9, 2024 15:32

andrewlock approved these changes Dec 10, 2024

View reviewed changes

anna-git added 4 commits December 11, 2024 17:29

add tags

22aae28

fix tags

214059d

Adapt user id and login and anonymization

defe7f3

Adapt tests

add telemetry for login

092c3cb

anna-git added 6 commits December 11, 2024 17:29

adapt integration tests

051e07c

nullability file

897be50

Answer to Andrew's comments

f520946

Fix getanony test

5807542

remove unsafe with span<t>

fce4f35

regenarate after rebase

484d6f8

anna-git force-pushed the anna/asm/collect-login-and-adjustementstorfc branch from 725cf8f to 484d6f8 Compare December 11, 2024 16:48

anna-git changed the title ~~[ASM] Adapt v3 new login tags and fix signup tags~~ [ASM][ATO] Adapt v3 new login tags and fix signup tags Dec 12, 2024

anna-git merged commit 22ef56a into master Dec 12, 2024
107 checks passed

anna-git deleted the anna/asm/collect-login-and-adjustementstorfc branch December 12, 2024 13:46

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

		tryAddTag(Tags.AppSec.EventsUsers.SignUpEvent.UserId, userId!);
		tryAddTag(Tags.AppSec.EventsUsers.InternalUserId, userId!);


		internal static string? GetLogin(IIdentityUser? user) => user?.UserName ?? user?.Email;

		internal static unsafe string Anonymize(string id)

[ASM][ATO] Adapt v3 new login tags and fix signup tags #6302

[ASM][ATO] Adapt v3 new login tags and fix signup tags #6302

Conversation

anna-git commented Nov 18, 2024 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

github-actions bot commented Nov 18, 2024 • edited Loading

Snapshots difference summary

andrewlock commented Nov 18, 2024 • edited Loading

Execution-Time Benchmarks Report ⏱️

datadog-ddstaging bot commented Nov 18, 2024 • edited Loading

Datadog Report

andrewlock commented Nov 18, 2024 • edited Loading

Throughput/Crank Report ⚡

andrewlock commented Nov 18, 2024 • edited Loading

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #6302

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #6302

Raw results

Raw results

andrewlock commented Nov 26, 2024 • edited Loading

Benchmarks Report for appsec 🐌

Benchmark details

Raw results

Raw results

Raw results

More allocations ⚠️ in #6302

Fewer allocations 🎉 in #6302

Raw results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewlock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anna-git commented Nov 18, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024 •

edited

Loading

andrewlock commented Nov 18, 2024 •

edited

Loading

datadog-ddstaging bot commented Nov 18, 2024 •

edited

Loading

andrewlock commented Nov 18, 2024 •

edited

Loading

andrewlock commented Nov 18, 2024 •

edited

Loading

andrewlock commented Nov 26, 2024 •

edited

Loading