dd-trace-dotnet [ASM] Improved Unsafe Encoder readability

Summary of changes

Externalized all embedded functions in Encoder

Reason for change

Encoder code was a bit difficult to read due to the long embedded functions it contained.

Implementation details

Created a EncoderContext struct passed by ref to all calls. Externalized all embedded functions passing previously captured data as arguments

Feb 12 '24 22:02 daniel-romano-DD

Datadog Report

Branch report: dani/asm/encoder_readability Commit report: 6257d33 Test service: dd-trace-dotnet

:x: 10 Failed (0 Known Flaky), 307636 Passed, 1575 Skipped, 46m 53.42s Wall Time :snowflake: 11 New Flaky

:x: Failed Tests (10)

This report shows up to 5 failed tests.

TestBlockedRequest - Datadog.Trace.Security.IntegrationTests.AspNetMvc5ClassicWithSecurity - Details

Expand for error

xpected collection to contain at least 5 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.

TestSecurity - Datadog.Trace.Security.IntegrationTests.AspNetMvc5ClassicWithSecurity - Details

Expand for error

xpected collection to contain at least 10 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.

TestSecurity - Datadog.Trace.Security.IntegrationTests.AspNetMvc5ClassicWithSecurity - Details

Expand for error

xpected collection to contain at least 10 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.

TestBlockedRequest - Datadog.Trace.Security.IntegrationTests.AspNetMvc5IntegratedWithSecurity - Details

Expand for error

xpected collection to contain at least 5 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.

TestSecurity - Datadog.Trace.Security.IntegrationTests.AspNetMvc5IntegratedWithSecurity - Details

Expand for error

xpected collection to contain at least 10 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.

New Flaky Tests (11)

EncodeLegacyArgs - Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Last Failure
TestBlockedRequest - Datadog.Trace.Security.IntegrationTests.AspNetMvc5ClassicWithSecurity
TestSecurity - Datadog.Trace.Security.IntegrationTests.AspNetMvc5ClassicWithSecurity
TestSecurity - Datadog.Trace.Security.IntegrationTests.AspNetMvc5ClassicWithSecurity
TestBlockedRequest - Datadog.Trace.Security.IntegrationTests.AspNetMvc5IntegratedWithSecurity

Feb 12 '24 22:02 datadog-ddstaging[bot]

Execution-Time Benchmarks Report :stopwatch:

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 (5180) - mean (74ms)  : 66, 82
     .   : milestone, 74,
    master - mean (76ms)  : 67, 85
     .   : milestone, 76,

    section CallTarget+Inlining+NGEN
    This PR (5180) - mean (978ms)  : 952, 1004
     .   : milestone, 978,
    master - mean (988ms)  : 965, 1011
     .   : milestone, 988,

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

    section CallTarget+Inlining+NGEN
    This PR (5180) - mean (707ms)  : 681, 733
     .   : milestone, 707,
    master - mean (714ms)  : 694, 734
     .   : milestone, 714,

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

    section CallTarget+Inlining+NGEN
    This PR (5180) - mean (655ms)  : 632, 677
     .   : milestone, 655,
    master - mean (663ms)  : 641, 686
     .   : milestone, 663,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5180) - mean (188ms)  : 184, 192
     .   : milestone, 188,
    master - mean (188ms)  : 184, 191
     .   : milestone, 188,

    section CallTarget+Inlining+NGEN
    This PR (5180) - mean (1,035ms)  : 1009, 1061
     .   : milestone, 1035,
    master - mean (1,053ms)  : 1027, 1080
     .   : milestone, 1053,

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

    section CallTarget+Inlining+NGEN
    This PR (5180) - mean (1,013ms)  : 986, 1039
     .   : milestone, 1013,
    master - mean (1,035ms)  : 1001, 1068
     .   : milestone, 1035,

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

    section CallTarget+Inlining+NGEN
    This PR (5180) - mean (985ms)  : 960, 1009
     .   : milestone, 985,
    master - mean (1,001ms)  : 978, 1024
     .   : milestone, 1001,

Feb 12 '24 22:02 andrewlock

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 (5180) (11.135M)   : 0, 11134619
    master (10.904M)   : 0, 10903785
    benchmarks/2.9.0 (11.116M)   : 0, 11116034

    section Automatic
    This PR (5180) (7.702M)   : 0, 7702306
    master (7.548M)   : 0, 7547783
    benchmarks/2.9.0 (7.891M)   : 0, 7891342

    section Trace stats
    This PR (5180) (8.009M)   : 0, 8008631
    master (7.902M)   : 0, 7902379

    section Manual
    This PR (5180) (9.873M)   : 0, 9872707
    master (9.632M)   : 0, 9631577

    section Manual + Automatic
    This PR (5180) (7.221M)   : 0, 7220647
    master (7.263M)   : 0, 7262759

    section Version Conflict
    This PR (5180) (6.482M)   : 0, 6481633
    master (6.604M)   : 0, 6603647

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5180) (9.584M)   : 0, 9583656
    master (9.355M)   : 0, 9354781
    benchmarks/2.9.0 (9.562M)   : 0, 9562342

    section Automatic
    This PR (5180) (6.564M)   : 0, 6563941
    master (6.646M)   : 0, 6646272

    section Trace stats
    This PR (5180) (6.857M)   : 0, 6857436
    master (6.782M)   : 0, 6782074

    section Manual
    This PR (5180) (8.135M)   : 0, 8135346
    master (8.426M)   : 0, 8425694

    section Manual + Automatic
    This PR (5180) (6.256M)   : 0, 6256358
    master (6.313M)   : 0, 6313053

    section Version Conflict
    This PR (5180) (5.775M)   : 0, 5774596
    master (5.623M)   : 0, 5622957

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5180) (10.196M)   : 0, 10195933
    master (10.039M)   : 0, 10039221
    benchmarks/2.9.0 (10.060M)   : 0, 10060320

    section Automatic
    This PR (5180) (7.195M)   : 0, 7195195
    master (7.158M)   : 0, 7157695
    benchmarks/2.9.0 (7.535M)   : 0, 7534759

    section Trace stats
    This PR (5180) (7.491M)   : 0, 7491146
    master (7.442M)   : 0, 7442467

    section Manual
    This PR (5180) (9.162M)   : 0, 9162441
    master (8.814M)   : 0, 8814383

    section Manual + Automatic
    This PR (5180) (6.989M)   : 0, 6988712
    master (6.903M)   : 0, 6903478

    section Version Conflict
    This PR (5180) (6.180M)   : 0, 6179941
    master (6.144M)   : 0, 6143514

gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5180) (7.377M)   : 0, 7376858
    master (7.398M)   : 0, 7398268
    benchmarks/2.9.0 (7.618M)   : 0, 7617642

    section No attack
    This PR (5180) (1.913M)   : 0, 1912713
    master (1.836M)   : 0, 1835561
    benchmarks/2.9.0 (3.121M)   : 0, 3120509

    section Attack
    This PR (5180) (1.522M)   : 0, 1521668
    master (1.457M)   : 0, 1457427
    benchmarks/2.9.0 (2.488M)   : 0, 2488303

    section Blocking
    This PR (5180) (3.297M)   : 0, 3297062
    master (3.175M)   : 0, 3174901

    section IAST default
    This PR (5180) (6.380M)   : 0, 6379685
    master (6.462M)   : 0, 6462305

    section IAST full
    This PR (5180) (5.587M)   : 0, 5587137
    master (5.843M)   : 0, 5843231

    section Base vuln
    This PR (5180) (0.974M)   : 0, 974210
    master (0.991M)   : 0, 991340

    section IAST vuln
    This PR (5180) (0.884M)   : 0, 884443
    master (0.892M)   : 0, 892185

Feb 13 '24 20:02 andrewlock

Superseeded by https://github.com/DataDog/dd-trace-dotnet/pull/5587

May 20 '24 16:05 daniel-romano-DD