Wire per-component cardinality limits from Config#11732
Conversation
…its RFC Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dougqh
requested review from
AlexeyKuznetsov-DD and
ygree
and removed request for
a team
dd-octo-sts
Bot
added
the
tag: ai generated
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75f72165fd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return configProvider.getInteger( | ||
| "trace.stats." + tagName + ".cardinality.limit", defaultLimit); |
There was a problem hiding this comment.
When any of the new DD_TRACE_STATS_*_CARDINALITY_LIMIT values is set to 0, a negative number, or a value above 2^29, this method returns it unchanged; the callers added in AggregateEntry and PeerTagSchema pass it directly into PropertyCardinalityHandler/TagCardinalityHandler, whose constructors throw for those ranges. In that misconfigured environment the stats classes fail initialization instead of falling back to the default/logging, so the accessor should reject invalid limits before handing them to the handlers.
Useful? React with 👍 / 👎.
| "DD_TRACE_STATS_RESOURCE_CARDINALITY_LIMIT": [ | ||
| { | ||
| "version": "A", | ||
| "type": "integer", |
There was a problem hiding this comment.
int for the new config metadata type
The supported-configurations metadata uses type: "int" for the existing integral settings (including the tracer metrics limits), while these ten new cardinality entries are the only ones using "integer". The local supported-configurations validation/publishing jobs consume this file, so the new configs can be rejected or omitted by downstream metadata consumers even though the agent code reads integers; please use int consistently for these entries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yup, int is used instead of integer in the file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
2 participants
What Does This Do
Adds the ability to configure cardinality limits per component / tag for client-side stats
Motivation
Avoid problems with high cardinality leading to excessive resource consumption while informing the customer (via sentinel value) when a limit was reached
Additional Notes
Unlike #11387 which introduced the limiting mechanism, this PR enables the limits by default.
Config.getTraceStatsCardinalityLimit(tagName, defaultLimit)method following the RFC naming patternDD_TRACE_STATS_{tag_name}_CARDINALITY_LIMITPropertyCardinalityHandlerinstances and theTagCardinalityHandlerinPeerTagSchemato read their limits from Config at class-load time, with defaults fromMetricCardinalityLimitsuseBlockedSentinel = true) now that limits are configurable; previously hardcoded tofalsepending this wiringsupported-configurations.jsonStacked on #11387.
Test plan
CardinalityHandlerTestandAggregateEntryTestcover sentinel substitution behaviorMetricCardinalityLimitsconstantsDD_TRACE_STATS_RESOURCE_CARDINALITY_LIMITetc. are picked up at startup via Configtag: no release notetag: ai generated🤖 Generated with Claude Code