feat(stats)!: send telemetry for cardinality limits#2159
Open
VianneyRuhlmann wants to merge 1 commit into
Open
Conversation
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jun 24, 2026
|
Contributor
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
Contributor
📚 Documentation Check Results📦
|
Contributor
🔒 Cargo Deny Results📦
|
![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: f971f0c30c
ℹ️ 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".
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_send_telemetry_for_cardinality_limits
branch
2 times, most recently
from
98f8c1d to
fffe30d
Compare
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_add_whole-key_cardinality_limit
branch
2 times, most recently
from
2655975 to
bf5e284
Compare
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_send_telemetry_for_cardinality_limits
branch
from
fffe30d to
e5c0f96
Compare
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_add_whole-key_cardinality_limit
branch
from
bf5e284 to
ce821ea
Compare
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_send_telemetry_for_cardinality_limits
branch
from
e5c0f96 to
d97928e
Compare
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_add_whole-key_cardinality_limit
branch
from
ce821ea to
ebd7e4e
Compare
VianneyRuhlmann
force-pushed
the
06-24-feat_stats_send_telemetry_for_cardinality_limits
branch
from
d97928e to
c84447e
Compare
ajgajg1134
reviewed
Jun 24, 2026
| /// Metric name for the number of spans collapsed into the overflow sentinel bucket due to | ||
| /// per-bucket cardinality limiting. Emitted on both the DogStatsD health-metrics channel | ||
| /// and the instrumentation-telemetry channel. | ||
| pub const COLLAPSED_SPANS_METRIC: &str = "datadog.tracer.stats.collapsed_spans"; |
Contributor
There was a problem hiding this comment.
Just a headsup that this name might change to datadog.tracer.stats_collapsed_spans as it seems the ".stats." namespace doesn't really exist for most tracers?
| } | ||
|
|
||
| /// Build a concentrator with `max_entries_per_bucket = 1` pre-seeded with four distinct spans | ||
| /// so that three spans is collapsed into the overflow bucket. |
Contributor
There was a problem hiding this comment.
Suggested change
| /// so that three spans is collapsed into the overflow bucket. | |
| /// so that three spans are collapsed into the overflow bucket. |
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
bantonsson
reviewed
Jun 25, 2026
| telemetry = ["libdd-telemetry"] | ||
| dogstatsd = ["libdd-dogstatsd-client"] | ||
| https = ["libdd-common/https", "libdd-capabilities-impl/https", "libdd-shared-runtime/https","libdd-telemetry?/https","libdd-dogstatsd-client?/https" ] | ||
| fips = ["libdd-common/fips", "libdd-capabilities-impl/fips", "libdd-shared-runtime/fips"] |
Contributor
There was a problem hiding this comment.
Is fips support missing for the added crates?
bwoebi
reviewed
Jun 25, 2026
Comment on lines
+122
to
+123
| None, | ||
| None, |
Contributor
There was a problem hiding this comment.
Why would we not want to benefit from that telemetry in PHP?
Contributor
Author
There was a problem hiding this comment.
You probably do I just wanted to limit the number of crates impacted by this PR (also why I enabled it in a separate PR for data-pipeline) but I can enable it for php too
Contributor
There was a problem hiding this comment.
Please do so :-)
I definitely won't have this telemetry on my radar myself afterwards
VianneyRuhlmann
changed the title
Eldolfin
approved these changes
Jun 26, 2026
| /// agent | ||
| /// - `meta` metadata used in ClientStatsPayload and as headers to send stats to the agent | ||
| /// - `endpoint` the Endpoint used to send stats to the agent | ||
| #[allow(clippy::too_many_arguments)] |
Contributor
There was a problem hiding this comment.
Suggested change
| #[allow(clippy::too_many_arguments)] | |
| #[allow(clippy::too_many_arguments, reason = "TODO: refactor")] |
Contributor
Author
There was a problem hiding this comment.
I don't think it's an issue for the StatsExporter to have a many arguments for new , especially since all of them have different types. (maybe the two obfuscation ones could be wrapped in a struct)
Contributor
There was a problem hiding this comment.
I was thinking about eventually using the builder pattern here maybe ?
It's just for readability tho, not blocking ofc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Add telemetry for the collapsed stats group based on this RFC
Motivation
Follow-up of #2158
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.