Closed andrewlock closed 3 months ago
datadog-ddstaging[bot]
commented
3 months ago Branch report: andrew/config-refactor/1-non-null-config-result
Commit report: 6d71f7d
Test service: dd-trace-dotnet
:white_check_mark: 0 Failed, 341699 Passed, 1624 Skipped, 14h 18m 0.47s Total Time
andrewlock
commented
3 months ago 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:
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 (5713) - mean (73ms) : 64, 83
. : milestone, 73,
master - mean (74ms) : 64, 83
. : milestone, 74,
section CallTarget+Inlining+NGEN
This PR (5713) - mean (990ms) : 971, 1008
. : milestone, 990,
master - mean (990ms) : 969, 1011
. : milestone, 990,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5713) - mean (109ms) : 105, 114
. : milestone, 109,
master - mean (110ms) : 105, 114
. : milestone, 110,
section CallTarget+Inlining+NGEN
This PR (5713) - mean (704ms) : 682, 726
. : milestone, 704,
master - mean (702ms) : 679, 724
. : milestone, 702,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5713) - mean (94ms) : 90, 97
. : milestone, 94,
master - mean (93ms) : 90, 96
. : milestone, 93,
section CallTarget+Inlining+NGEN
This PR (5713) - mean (654ms) : 633, 676
. : milestone, 654,
master - mean (653ms) : 628, 678
. : milestone, 653,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5713) - mean (191ms) : 187, 195
. : milestone, 191,
master - mean (190ms) : 188, 193
. : milestone, 190,
section CallTarget+Inlining+NGEN
This PR (5713) - mean (1,089ms) : 1066, 1113
. : milestone, 1089,
master - mean (1,077ms) : 1056, 1097
. : milestone, 1077,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5713) - mean (277ms) : 274, 280
. : milestone, 277,
master - mean (277ms) : 270, 285
. : milestone, 277,
section CallTarget+Inlining+NGEN
This PR (5713) - mean (875ms) : 850, 900
. : milestone, 875,
master - mean (872ms) : 853, 890
. : milestone, 872,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5713) - mean (265ms) : 261, 269
. : milestone, 265,
master - mean (265ms) : 260, 269
. : milestone, 265,
section CallTarget+Inlining+NGEN
This PR (5713) - mean (858ms) : 831, 885
. : milestone, 858,
master - mean (853ms) : 830, 876
. : milestone, 853,
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 (5713) (11.648M) : 0, 11648483
master (11.593M) : 0, 11593369
benchmarks/2.9.0 (11.542M) : 0, 11542126
section Automatic
This PR (5713) (7.798M) : 0, 7798221
master (7.827M) : 0, 7827212
benchmarks/2.9.0 (8.263M) : 0, 8262905
section Trace stats
master (8.118M) : 0, 8118390
section Manual
This PR (5713) (9.932M) : 0, 9931503
master (9.920M) : 0, 9920204
section Manual + Automatic
This PR (5713) (7.368M) : 0, 7367887
master (7.358M) : 0, 7357979
section Version Conflict
master (6.635M) : 0, 6634804
gantt
title Throughput Linux arm64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5713) (9.572M) : 0, 9572321
master (9.689M) : 0, 9688586
benchmarks/2.9.0 (9.596M) : 0, 9596140
section Automatic
This PR (5713) (6.589M) : 0, 6588557
master (6.298M) : 0, 6298066
section Trace stats
master (6.862M) : 0, 6861645
section Manual
This PR (5713) (8.051M) : 0, 8050794
master (8.028M) : 0, 8028063
section Manual + Automatic
This PR (5713) (6.165M) : 0, 6165101
master (6.139M) : 0, 6139377
section Version Conflict
master (5.667M) : 0, 5667497
gantt
title Throughput Windows x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5713) (10.266M) : 0, 10266142
master (10.241M) : 0, 10241408
benchmarks/2.9.0 (10.213M) : 0, 10213239
section Automatic
This PR (5713) (7.286M) : 0, 7285540
master (7.186M) : 0, 7186302
benchmarks/2.9.0 (7.482M) : 0, 7482023
section Trace stats
master (7.568M) : 0, 7568098
section Manual
This PR (5713) (9.152M) : 0, 9151764
master (9.213M) : 0, 9213352
section Manual + Automatic
This PR (5713) (6.984M) : 0, 6983521
master (7.004M) : 0, 7004280
section Version Conflict
master (6.289M) : 0, 6288694
Benchmarks for #5713 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored.
Summary of changes
ITelemetredConfigurationSourcemethods to returnConfigurationResult<T>instead ofConfigurationResult<T>?ConfigurationResult<>to distinguish between "key not found" and "value not parsable"Reason for change
In https://github.com/DataDog/dd-trace-dotnet/pull/5661, we added support for mapping standard OTel configuration to DD equivalents. Unfortunately, doing this correctly required distinguishing between "key not found" and "value not parsable". As a workaround, an addition method
IsPresent(key)was added, but this essentially duplicates a lot of the work we're already doing when loading keys "normally".The changes in this PR are a first step to simplifying and unifying the support for Otel
Implementation details
ITelemeteredConfigurationSource.Get*methods to returnConfigurationResultinstead of a potentially null value. Update all sources as appropriate.ConfigurationResultbool IsPresent()LoadResultenum which can be one ofValid,NotFound,ParsingError, orValidationFailure. Previously we effectively had a bool of justValidorValidationFailure, and "is null" meantNotFoundorParsingErrorShouldFallBackConfigurationBuilderGetResultmethods. By extracting static lambdas we can reuse a lot of the logic for multiple typesITelemeteredConfigurationSource.IsPresentis still used in Otel paths currently - removed in a subsequent PRTest coverage
This is just a refactor, so all covered by existing tests
Other details
This is part of a big stack of config refactoring PRs: