Closed lucaspimentel closed 1 week ago
datadog-ddstaging[bot]
commented
1 week ago Branch report: lpimentel/remote-sampling-rules-precedence
Commit report: a5b7062
Test service: dd-trace-dotnet
:white_check_mark: 0 Failed, 336972 Passed, 1643 Skipped, 14h 10m 12.57s Total Time
andrewlock
commented
1 week 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 (5720) - mean (74ms) : 65, 82
. : milestone, 74,
master - mean (73ms) : 64, 82
. : milestone, 73,
section CallTarget+Inlining+NGEN
This PR (5720) - mean (987ms) : 962, 1012
. : milestone, 987,
master - mean (983ms) : 960, 1005
. : milestone, 983,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5720) - mean (109ms) : 106, 112
. : milestone, 109,
master - mean (109ms) : 106, 112
. : milestone, 109,
section CallTarget+Inlining+NGEN
This PR (5720) - mean (696ms) : 677, 716
. : milestone, 696,
master - mean (687ms) : 668, 706
. : milestone, 687,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5720) - mean (95ms) : 81, 108
. : milestone, 95,
master - mean (92ms) : 89, 95
. : milestone, 92,
section CallTarget+Inlining+NGEN
This PR (5720) - mean (649ms) : 632, 666
. : milestone, 649,
master - mean (650ms) : 629, 671
. : milestone, 650,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5720) - mean (197ms) : 193, 202
. : milestone, 197,
master - mean (191ms) : 188, 194
. : milestone, 191,
section CallTarget+Inlining+NGEN
This PR (5720) - mean (1,104ms) : 1073, 1136
. : milestone, 1104,
master - mean (1,075ms) : 1051, 1098
. : milestone, 1075,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5720) - mean (282ms) : 277, 286
. : milestone, 282,
master - mean (276ms) : 272, 280
. : milestone, 276,
section CallTarget+Inlining+NGEN
This PR (5720) - mean (885ms) : 849, 921
. : milestone, 885,
master - mean (869ms) : 848, 891
. : milestone, 869,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5720) - mean (274ms) : 267, 281
. : milestone, 274,
master - mean (265ms) : 261, 269
. : milestone, 265,
section CallTarget+Inlining+NGEN
This PR (5720) - mean (879ms) : 850, 908
. : milestone, 879,
master - mean (850ms) : 828, 872
. : milestone, 850,
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 (5720) (11.510M) : 0, 11509505
master (11.532M) : 0, 11532424
benchmarks/2.9.0 (11.656M) : 0, 11655894
section Automatic
This PR (5720) (7.852M) : 0, 7851584
master (7.718M) : 0, 7717683
benchmarks/2.9.0 (8.153M) : 0, 8153210
section Trace stats
master (8.061M) : 0, 8061054
section Manual
This PR (5720) (9.985M) : 0, 9984750
master (9.910M) : 0, 9910287
section Manual + Automatic
This PR (5720) (7.377M) : 0, 7376687
master (7.349M) : 0, 7348912
section Version Conflict
master (6.599M) : 0, 6598956
gantt
title Throughput Linux arm64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5720) (9.443M) : 0, 9442554
master (9.534M) : 0, 9533553
benchmarks/2.9.0 (9.505M) : 0, 9504870
section Automatic
This PR (5720) (6.582M) : 0, 6582108
master (6.513M) : 0, 6513135
section Trace stats
master (6.853M) : 0, 6852602
section Manual
This PR (5720) (8.246M) : 0, 8246380
master (8.286M) : 0, 8285867
section Manual + Automatic
This PR (5720) (6.153M) : 0, 6153309
master (6.156M) : 0, 6155750
section Version Conflict
master (5.788M) : 0, 5788056
gantt
title Throughput Windows x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5720) (9.993M) : 0, 9993199
master (10.234M) : 0, 10233736
benchmarks/2.9.0 (10.370M) : 0, 10369822
section Automatic
This PR (5720) (6.997M) : 0, 6997211
master (7.108M) : 0, 7107940
benchmarks/2.9.0 (7.364M) : 0, 7364262
section Trace stats
master (7.371M) : 0, 7370591
section Manual
This PR (5720) (9.019M) : 0, 9019463
master (8.754M) : 0, 8753769
section Manual + Automatic
This PR (5720) (6.833M) : 0, 6833419
master (6.808M) : 0, 6807752
section Version Conflict
master (6.155M) : 0, 6155424
Benchmarks for #5720 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored.
Summary of changes
When sampling rules are configured remotely, we should ignore all local sampling rules instead of merging remote and local rules into a single list.
Note that as long as there is a catch-all remote rule (effectively a remote global sampling rate using
DD_TRACE_SAMPLING_RULESinstead ofDD_TRACE_SAMPLE_RATE, as recommended) or if Adaptive Sampling is enabled, this will have no effect, since local rules will be ignored as long as a remote rule matches a trace. The edge case fixed in this PR only occurs if all of these are true:In the above case, we were applying the sampling rate from the matching local rule instead of ignoring all local rules.
Reason for change
Consistency across the language ~tracers~ ~libraries~ SDKs. Allows us to "remove" local configuration without having to specify a catch-all remote rule.
Implementation details
In the tracer settings, remove
RemoteSamplingRulesand keep onlyCustomSamplingRulesInternal. Its value will have either remote rules or local rules, with the remote value overriding any local value (like we do with other remote settings). Also add aboolpropertyCustomSamplingRulesIsRemoteto determine if the setting's source was local or remote so we can deserialize them correctly (they have different schemas).Test coverage