feat(telemetry)!: Add Installation signature and AppProduct changes payloads#2213
Open
bwoebi wants to merge 2 commits into
Open
feat(telemetry)!: Add Installation signature and AppProduct changes payloads#2213bwoebi wants to merge 2 commits into
bwoebi wants to merge 2 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
bwoebi
commented
Jul 8, 2026
Comment on lines
+21
to
+22
| pub use libdd_common::tag::{parse_tags, Tag}; | ||
| pub use libdd_common::{parse_uri, Endpoint}; |
Contributor
Author
There was a problem hiding this comment.
This is here. But we expose Tag currently publicly in data/metrics.rs. So we need that right now...
Contributor
There was a problem hiding this comment.
Hasn't @hoolioh removed the need for Endpoint recently in Telemetry? But I guess if you need Tag anyway...
Contributor
📚 Documentation Check Results📦
|
bwoebi
changed the title
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: 6f102bdf44
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+17
to
+18
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub metadata: Option<Vec<DependencyMetadata>>, |
There was a problem hiding this comment.
Because Store<Dependency> deduplicates by the derived Hash/Eq, including the new metadata field in that identity makes the same package/version a second stored dependency when SCA metadata is attached after the initial dependency report. On the next extended heartbeat, unflush_stored() will re-send both the stale entry and the updated metadata entry for the same dependency; track identity by the stable dependency key and update metadata in place instead.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Might be worth looking into as well.
Contributor
Author
There was a problem hiding this comment.
True, addressed this.
BenchmarksComparisonBenchmark execution time: 2026-07-20 21:11:58 Comparing candidate commit 87f6745 in PR branch Found 2 performance improvements and 7 performance regressions! Performance is the same for 140 metrics, 10 unstable metrics.
|
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
|
And some smaller stuff like more metric namespaces & Rate MetricType.
yannham
reviewed
Jul 20, 2026
| pub enum MetricType { | ||
| Gauge, | ||
| Count, | ||
| Rate, |
Contributor
There was a problem hiding this comment.
Is it ok to add a field in the middle of repr(C) enum? Won't that change the integer representation of Distribution?
Contributor
Author
There was a problem hiding this comment.
It will, but stuff is compiled together / cbdingen hopefully rerun? But let me move it down just to be sure.
Contributor
There was a problem hiding this comment.
I'm not too worried about the Rust part, but a potential FFI consumer. I have no idea who consumes this, so it's purely defensive...
Comment on lines
+17
to
+18
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub metadata: Option<Vec<DependencyMetadata>>, |
Contributor
There was a problem hiding this comment.
Might be worth looking into as well.
| #[strum(serialize_all = "snake_case")] | ||
| pub enum ConfigurationOrigin { | ||
| EnvVar, | ||
| OtelEnvVar, |
Contributor
There was a problem hiding this comment.
Same question about changing a repr(C) enum
Contributor
Author
There was a problem hiding this comment.
Note that several times stuff has been added before Unknown.
Comment on lines
+21
to
+22
| pub use libdd_common::tag::{parse_tags, Tag}; | ||
| pub use libdd_common::{parse_uri, Endpoint}; |
Contributor
There was a problem hiding this comment.
Hasn't @hoolioh removed the need for Endpoint recently in Telemetry? But I guess if you need Tag anyway...
Contributor
|
Nothing blocking on my side, but I think some of the codex comments might be relevant to check |
Comment on lines
+653
to
+655
| // Consolidated path: report health metrics through the externally-owned worker. | ||
| // We do not own it (no spawn, no start, no shutdown handle). | ||
| (Some(TelemetryClient::with_handle(shared_handle)), None) |
Contributor
There was a problem hiding this comment.
Just a heads up: I don't know how you want to create it externally, but you should make sure to recreate the client on forks and recreate the writer with it (might be a bit tedious to make sure the fork hooks run in the right order). Currently the writer is recreated by the python NativeWriter which also recreates the telemetry client.
Using Keyed to avoid messing with Hash and Eq. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
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
3 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.
And some smaller stuff like more metric namespaces.
This bridges the gap to the current capabilities exposed by dd-trace-py.