Implement support for DD_TAGS in Client-Side stats#8823
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8823) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-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 highlighted 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). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (72ms) : 68, 76
master - mean (73ms) : 70, 77
section Bailout
This PR (8823) - mean (74ms) : 73, 75
master - mean (75ms) : 72, 79
section CallTarget+Inlining+NGEN
This PR (8823) - mean (1,085ms) : 1040, 1129
master - mean (1,080ms) : 1032, 1127
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (111ms) : 105, 118
master - mean (112ms) : 106, 117
section Bailout
This PR (8823) - mean (110ms) : 109, 112
master - mean (112ms) : 107, 117
section CallTarget+Inlining+NGEN
This PR (8823) - mean (778ms) : 751, 805
master - mean (776ms) : 749, 802
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (96ms) : 94, 99
master - mean (98ms) : 92, 104
section Bailout
This PR (8823) - mean (100ms) : 94, 106
master - mean (97ms) : 96, 99
section CallTarget+Inlining+NGEN
This PR (8823) - mean (938ms) : 899, 976
master - mean (934ms) : 898, 970
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (97ms) : 92, 102
master - mean (95ms) : 93, 98
section Bailout
This PR (8823) - mean (95ms) : 93, 97
master - mean (100ms) : 95, 104
section CallTarget+Inlining+NGEN
This PR (8823) - mean (811ms) : 772, 850
master - mean (811ms) : 770, 851
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (203ms) : 198, 207
master - mean (202ms) : 197, 206
section Bailout
This PR (8823) - mean (206ms) : 201, 211
master - mean (206ms) : 202, 209
section CallTarget+Inlining+NGEN
This PR (8823) - mean (1,209ms) : 1166, 1252
master - mean (1,210ms) : 1166, 1255
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (290ms) : 282, 297
master - mean (291ms) : 285, 298
section Bailout
This PR (8823) - mean (292ms) : 288, 297
master - mean (293ms) : 288, 298
section CallTarget+Inlining+NGEN
This PR (8823) - mean (973ms) : 949, 997
master - mean (977ms) : 957, 998
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (281ms) : 274, 287
master - mean (284ms) : 278, 291
section Bailout
This PR (8823) - mean (281ms) : 278, 285
master - mean (283ms) : 276, 290
section CallTarget+Inlining+NGEN
This PR (8823) - mean (1,168ms) : 1131, 1205
master - mean (1,176ms) : 1136, 1217
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8823) - mean (282ms) : 275, 289
master - mean (285ms) : 278, 291
section Bailout
This PR (8823) - mean (281ms) : 274, 288
master - mean (286ms) : 279, 293
section CallTarget+Inlining+NGEN
This PR (8823) - mean (1,053ms) : 1013, 1092
master - mean (1,046ms) : 1004, 1088
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmark execution time: 2026-07-02 09:48:24 Comparing candidate commit d673813 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 62 known flaky benchmarks, 64 flaky benchmarks without significant changes.
|
d0a473c to
c6898d1
Compare
c4004df to
8fe4238
Compare
c6898d1 to
f99f6dd
Compare
8fe4238 to
a146d8f
Compare
## Summary of changes - Mark testing only methods with `[TestingAndPrivateOnly]` - Pre-encode header tags for static UTF-8 messagepack keys - Avoid re-encoding the peer_tags, seeing as we already have that data ## Reason for change Minor tweaks (mostly perf) before adding the additional client-side-stats features ## Implementation details Mostly self-evident, written as distinct commits. The most interesting one is avoiding the additional interpolated string when encoding peer_tags. We also avoid re-encoding the tags seeing as we already do that work up front ## Test coverage Covered by existing tests, this is all refactoring ## Other details Part of a client-side stats stack of PRs. - #8822 👈 - #8766 - #8823 - #8824 https://datadoghq.atlassian.net/browse/APMLP-1446
f99f6dd to
889f756
Compare
a146d8f to
be245bd
Compare
…ide-Stats computation (#8766) ## Summary of changes Implements span-derived primary tags (also called "Additional metric tags") ## Reason for change This is a feature requested by customers, and defined in [the RFC](https://datadoghq.atlassian.net/wiki/spaces/APM/history/6482919540/PENDING+Span-Derived+Primary+Tags+-+V1) ## Implementation details - Add `DD_TRACE_STATS_ADDITIONAL_TAGS` for controlling which span-tags to include as metric dimensions in client-side-stats - This is an experimental feature, so is gated on `DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED` currently - Include the defined tags as additional dimensions in the metric key (similar to peer tags) - Apply cardinality limits to avoid memory leaks - NOTE: the cardinality limits implemented here are somewhat superseded by [a subsequent RFC](https://datadoghq.atlassian.net/wiki/spaces/APM/pages/6821151019/PENDING+Cardinality+Limits), and so are only partially implemented here. ## Test coverage - Added unit tests for all the behaviour - System tests TBC - once they're implemented, I'll enable them and confirm they're worked ## Other details Some aspects of the RFC are missing/modified based on other discussions - The telemetry metric in the span tags RFC is not defined, we will add the cardinality limit version instead. - The required logging will be added as part of the cardinality RFC implementation - Cardinality protection doesn't entirely match the RFC, as it follows the later RFC Part of a stack - #8822 - #8766 👈 - #8823 - #8824 https://datadoghq.atlassian.net/browse/APMLP-1446 --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
be245bd to
d673813
Compare
## Summary of changes Implements the Client-Side Stats [cardinality control RFC](https://datadoghq.atlassian.net/wiki/spaces/APM/pages/6821151019/PENDING+Cardinality+Limits) ## Reason for change Currently, we have a risk of unbounded memory growth caused by cardinality explosion. This has not been a big issue so far as CSS is off by default for .NET. However, resource name explosion (in particular), as well as span-derived primary tags and peer tags have the potential to cause OOM issues. This PR adds strict (configurable) caps to prevent issues from runaway memory growth. ## Implementation details Largely broken into separate distinct commits - Convert `StatsAggregationKey` to `readonly record struct`, seeing as that's basically what it was anyway - Make `IsTraceRoot` nullable (required for "overflow bucket") - Make resource length limit configurable (to support "big_resource" feature from agent) - Add per-field cardinality limitation to resource, httpendoint, peertags, additional tags - Per-field limits stop a single very high cardinality field rendering everything else useless - Only added to these 4 fields for now, but we can expand if necessary - Add whole-key cardinality limit - Essentially a cap on the maximum number of buckets we'll allocate - All additional stats flow into an "overflow" bucket - Add telemetry/heath-metrics/logging for tracking overflow occurences - Add some tests ## Test coverage Added a bunch of unit tests. Waiting on system tests to confirm the implementation. ## Other details Part of a stack - #8822 - #8766 - #8823 - #8824 👈 https://datadoghq.atlassian.net/browse/APMLP-1446
Summary of changes
Adds support for adding
DD_TAGSas span-derived primary tags (AKA Additional tags) in Client Side StatsReason for change
Implements the RFC for adding
DD_TAGSto the client-side stats payload.Implementation details
Relatively simple: we just thread any
DD_TAGSvalue through to the payload, and the backend/agent then ensures they're added as span-derived primary tags. There's no cardinality issues at play here, which simplifies things, and we can just include them in every payloadTest coverage
Added some unit tests, system tests to follow...
Other details
Part of a stack
https://datadoghq.atlassian.net/browse/APMLP-1446