[IAST] Fix version parsing in Dataflow #5263

kevingosse · 2024-03-04T17:57:11Z

Summary of changes

The code to parse the version in Dataflow was looking for the "V" prefix, but not "v". Because of that, it always reported a major version of 0 on .NET Framework (v4.0.30319).

Also, since we're already getting the version as numerical values, I changed the code to directly create the VersionInfo without parsing the string.

Reason for change

The logs weren't showing the right version:

Dataflow::Dataflow -> Detected runtime version : 0.0.30319.0

datadog-ddstaging · 2024-03-04T18:11:22Z

Datadog Report

Branch report: kevin/version_dataflow
Commit report: 28b636d
Test service: dd-trace-dotnet

✅ 0 Failed, 342020 Passed, 2074 Skipped, 42m 2.53s Wall Time
❄️ 1 New Flaky

New Flaky Tests (1)

InjectsLogsWhenEnabled - Datadog.Trace.ClrProfiler.IntegrationTests.VersionConflict.SerilogVersionConflict2xTests

andrewlock · 2024-03-04T18:25:38Z

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 (5263) - mean (75ms)  : 63, 86
     .   : milestone, 75,
    master - mean (74ms)  : 66, 82
     .   : milestone, 74,

    section CallTarget+Inlining+NGEN
    This PR (5263) - mean (993ms)  : 971, 1015
     .   : milestone, 993,
    master - mean (995ms)  : 967, 1024
     .   : milestone, 995,

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

    section CallTarget+Inlining+NGEN
    This PR (5263) - mean (714ms)  : 693, 735
     .   : milestone, 714,
    master - mean (716ms)  : 695, 736
     .   : milestone, 716,

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

    section CallTarget+Inlining+NGEN
    This PR (5263) - mean (673ms)  : 652, 694
     .   : milestone, 673,
    master - mean (675ms)  : 648, 701
     .   : milestone, 675,

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

    section CallTarget+Inlining+NGEN
    This PR (5263) - mean (1,066ms)  : 1038, 1094
     .   : milestone, 1066,
    master - mean (1,050ms)  : 1026, 1074
     .   : milestone, 1050,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5263) - mean (272ms)  : 268, 275
     .   : milestone, 272,
    master - mean (269ms)  : 262, 275
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (5263) - mean (870ms)  : 845, 894
     .   : milestone, 870,
    master - mean (860ms)  : 829, 891
     .   : milestone, 860,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5263) - mean (260ms)  : 256, 264
     .   : milestone, 260,
    master - mean (257ms)  : 252, 263
     .   : milestone, 257,

    section CallTarget+Inlining+NGEN
    This PR (5263) - mean (853ms)  : 824, 881
     .   : milestone, 853,
    master - mean (842ms)  : 817, 867
     .   : milestone, 842,

andrewlock · 2024-03-05T02:09:55Z

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 (5263) (10.829M)   : 0, 10829415
    master (10.925M)   : 0, 10925303
    benchmarks/2.9.0 (10.725M)   : 0, 10725277

    section Automatic
    This PR (5263) (7.485M)   : 0, 7485104
    master (7.496M)   : 0, 7496085
    benchmarks/2.9.0 (7.627M)   : 0, 7626893

    section Trace stats
    This PR (5263) (7.790M)   : 0, 7790056
    master (7.872M)   : 0, 7871947

    section Manual
    This PR (5263) (9.489M)   : 0, 9489357
    master (9.635M)   : 0, 9635035

    section Manual + Automatic
    This PR (5263) (7.052M)   : 0, 7052024
    master (7.114M)   : 0, 7113688

    section Version Conflict
    This PR (5263) (6.372M)   : 0, 6371626
    master (6.292M)   : 0, 6292435

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5263) (9.540M)   : 0, 9540454

    section Automatic
    This PR (5263) (6.658M)   : 0, 6657982

    section Trace stats
    This PR (5263) (6.869M)   : 0, 6869416

    section Manual
    This PR (5263) (8.178M)   : 0, 8177733

    section Manual + Automatic
    This PR (5263) (6.242M)   : 0, 6242361

    section Version Conflict
    This PR (5263) (5.749M)   : 0, 5748799

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5263) (10.367M)   : 0, 10366812
    master (9.856M)   : 0, 9856447
    benchmarks/2.9.0 (9.878M)   : 0, 9877925

    section Automatic
    This PR (5263) (7.142M)   : 0, 7142184
    master (7.076M)   : 0, 7076232
    benchmarks/2.9.0 (7.376M)   : 0, 7376498

    section Trace stats
    This PR (5263) (7.409M)   : 0, 7408547
    master (7.404M)   : 0, 7404111

    section Manual
    This PR (5263) (8.819M)   : 0, 8819274
    master (8.658M)   : 0, 8657799

    section Manual + Automatic
    This PR (5263) (6.923M)   : 0, 6922714
    master (6.832M)   : 0, 6831716

    section Version Conflict
    This PR (5263) (6.311M)   : 0, 6311451
    master (6.173M)   : 0, 6172929

gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5263) (7.393M)   : 0, 7393370
    master (7.559M)   : 0, 7559179
    benchmarks/2.9.0 (7.896M)   : 0, 7895750

    section No attack
    This PR (5263) (1.825M)   : 0, 1825296
    master (1.844M)   : 0, 1844300
    benchmarks/2.9.0 (3.239M)   : 0, 3239357

    section Attack
    This PR (5263) (1.445M)   : 0, 1444952
    master (1.451M)   : 0, 1451050
    benchmarks/2.9.0 (2.532M)   : 0, 2532305

    section Blocking
    This PR (5263) (3.206M)   : 0, 3206252
    master (3.102M)   : 0, 3101736

    section IAST default
    This PR (5263) (6.513M)   : 0, 6513084
    master (6.434M)   : 0, 6433840

    section IAST full
    This PR (5263) (5.685M)   : 0, 5684685
    master (5.626M)   : 0, 5625514

    section Base vuln
    This PR (5263) (0.960M)   : 0, 959756
    master (0.917M)   : 0, 917115

    section IAST vuln
    This PR (5263) (0.847M)   : 0, 846665
    master (0.881M)   : 0, 881291

lucaspimentel · 2024-03-05T21:55:36Z

tracer/src/Datadog.Tracer.Native/iast/iast_util.cpp

@@ -219,7 +219,7 @@ static thread_local std::unordered_map<void *, bool> locked;
    VersionInfo GetVersionInfo(const std::string& version)
    {
        auto v = version;
-        if (StartsWith(v, "V"))
+        if (StartsWith(v, "V") || StartsWith(v, "v"))


Should there be unit tests for this?

Probably? That's up to ASM though.

NachoEchevarria

Thank you!!!

Fix version parsing in Dataflow

28b636d

kevingosse requested a review from a team as a code owner March 4, 2024 17:57

lucaspimentel reviewed Mar 5, 2024

View reviewed changes

lucaspimentel approved these changes Mar 5, 2024

View reviewed changes

NachoEchevarria approved these changes Mar 6, 2024

View reviewed changes

kevingosse merged commit 7bef2de into master Mar 7, 2024
59 of 61 checks passed

kevingosse deleted the kevin/version_dataflow branch March 7, 2024 10:06

github-actions bot added this to the vNext milestone Mar 7, 2024

lucaspimentel added the area:asm-iast label Mar 7, 2024

lucaspimentel changed the title ~~Fix version parsing in Dataflow~~ [IAST] Fix version parsing in Dataflow Mar 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IAST] Fix version parsing in Dataflow #5263

[IAST] Fix version parsing in Dataflow #5263

kevingosse commented Mar 4, 2024

datadog-ddstaging bot commented Mar 4, 2024 •

edited

Loading

andrewlock commented Mar 4, 2024

andrewlock commented Mar 5, 2024

lucaspimentel Mar 5, 2024

kevingosse Mar 6, 2024

NachoEchevarria left a comment

[IAST] Fix version parsing in Dataflow #5263

[IAST] Fix version parsing in Dataflow #5263

Conversation

kevingosse commented Mar 4, 2024

Summary of changes

Reason for change

datadog-ddstaging bot commented Mar 4, 2024 • edited Loading

Datadog Report

New Flaky Tests (1)

andrewlock commented Mar 4, 2024

Execution-Time Benchmarks Report ⏱️

andrewlock commented Mar 5, 2024

Throughput/Crank Report:zap:

lucaspimentel Mar 5, 2024

Choose a reason for hiding this comment

kevingosse Mar 6, 2024

Choose a reason for hiding this comment

NachoEchevarria left a comment

Choose a reason for hiding this comment

datadog-ddstaging bot commented Mar 4, 2024 •

edited

Loading