feat(telemetry)!: Add Installation signature and AppProduct changes payloads#2213
Open
bwoebi wants to merge 1 commit into
Open
feat(telemetry)!: Add Installation signature and AppProduct changes payloads#2213bwoebi wants to merge 1 commit into
bwoebi wants to merge 1 commit into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 6f7344e | Docs | Datadog PR Page | Give us feedback! |
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
📚 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".
| #[cfg(feature = "telemetry")] | ||
| telemetry: Option<TelemetryConfig>, | ||
| #[cfg(feature = "telemetry")] | ||
| telemetry_handle: Option<TelemetryWorkerHandle>, |
There was a problem hiding this comment.
With the default telemetry feature enabled, this newly added field is never initialized by TraceExporterBuilder::new(), so constructing the builder fails to compile (cargo check -p libdd-data-pipeline reports E0063 at the Self { ... } initializer). Please add telemetry_handle: None under the same cfg gate in new() so the default data-pipeline build remains usable.
Useful? React with 👍 / 👎.
| Telemetry, | ||
| Apm, | ||
| Sidecar, | ||
| Civisibility, |
There was a problem hiding this comment.
Because this enum uses #[serde(rename_all = "snake_case")], the new Civisibility variant serializes as "civisibility" rather than the CI Visibility namespace name ("ci_visibility"). When callers register CI Visibility telemetry metrics with this variant, the payload will carry an unknown/misspelled namespace and those metrics can be rejected or misclassified; use a variant/serde rename that emits ci_visibility.
Useful? React with 👍 / 👎.
| dependencies: self.data.dependencies.unflushed().cloned().collect(), | ||
| integrations: self.data.integrations.unflushed().cloned().collect(), | ||
| install_signature: self.data.install_signature.clone(), | ||
| products: self.data.products.clone(), |
There was a problem hiding this comment.
When a product is registered before send_start, it is included in the initial app-started payload here, but app_started_sent_success() only clears dependencies/configurations/integrations and leaves the product name in products_pending. The next FlushData will therefore emit an app-product-change for the same initial state, double-reporting product enablement; clear the pending product entries after a successful AppStarted send as well.
Useful? React with 👍 / 👎.
Comment on lines
+15
to
+18
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub hash: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub metadata: Option<Vec<DependencyMetadata>>, |
There was a problem hiding this comment.
Adding hash and metadata as required struct fields breaks existing in-tree constructors that still use Dependency { name, version } (for example datadog-sidecar-ffi/src/lib.rs:515), so after the data-pipeline initializer is fixed those crates will fail with E0063. Update those constructors to include the new fields, or use ..Default::default(), so the workspace continues to compile.
Useful? React with 👍 / 👎.
Comment on lines
+531
to
+539
| if !self.data.products.is_empty() { | ||
| let products = self | ||
| .data | ||
| .products | ||
| .iter() | ||
| .map(|(name, state)| (name.clone(), state.clone())) | ||
| .collect(); | ||
| let product_change = | ||
| data::Payload::AppProductChange(data::AppProductChange { products }); |
There was a problem hiding this comment.
Once any product has been recorded, every ExtendedHeartbeat takes this branch because it checks the full products map rather than products_pending, so long-running processes will emit app-product-change for unchanged product states on every extended heartbeat even after the pending set was cleared. Build this payload only from pending products, or omit it here and let the normal change batch handle product changes.
Useful? React with 👍 / 👎.
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 👍 / 👎.
BenchmarksComparisonBenchmark execution time: 2026-07-15 15:02:45 Comparing candidate commit 6f7344e in PR branch Found 4 performance improvements and 10 performance regressions! Performance is the same for 115 metrics, 0 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.
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
1 participant
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.