chore(deps): update libdatadog to 72fa8685 and serverless-components to 9daae40 - #1332
chore(deps): update libdatadog to 72fa8685 and serverless-components to 9daae40#1332lucaspimentel wants to merge 6 commits into
libdatadog to 72fa8685 and serverless-components to 9daae40#1332Conversation
|
🔗 Commit SHA: 52491e1 | Docs | View more details | Give us feedback! |
There was a problem hiding this comment.
Pull request overview
Updates Bottlecap’s pinned libdatadog and serverless-components revisions in lockstep to unblock upcoming trace features, and adjusts internal code to match upstream API changes (notably TracerHeaderTags restructuring and SpanConcentrator::new signature/cardinality behavior).
Changes:
- Bump
libdatadogrev to72fa8685andserverless-componentsrev to9daae40, with correspondingCargo.lockresolution changes (including intentionallibdd-*duplication). - Update trace header tag plumbing to use
TracerGenericTagsand carry header tags across async boundaries viaOwnedTracerHeaderTags. - Update stats concentrator initialization to explicitly preserve “unbounded” cardinality behavior via
CardinalityLimitConfig, and adapt tests accordingly.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bottlecap/tests/apm_integration_test.rs | Adjusts test header tag construction to the new TracerHeaderTags { generic: ... } shape. |
| bottlecap/src/traces/trace_processor.rs | Updates header-tag reads and uses OwnedTracerHeaderTags when holding traces for AppSec. |
| bottlecap/src/traces/trace_aggregator.rs | Refactors OwnedTracerHeaderTags to store the new generic tag struct and reconstruct TracerHeaderTags. |
| bottlecap/src/traces/trace_aggregator_service.rs | Updates tests to construct header tags with nested generic fields. |
| bottlecap/src/traces/trace_agent.rs | Switches computations to read stats/top-level flags from tags.generic.*. |
| bottlecap/src/traces/stats_concentrator_service.rs | Adapts SpanConcentrator::new call to new cardinality config + additional tag keys, and updates the no-cardinality-limit test. |
| bottlecap/src/otlp/agent.rs | Reads client_computed_stats from tracer_header_tags.generic. |
| bottlecap/src/lifecycle/invocation/processor.rs | Updates test tag construction to the new generic nesting. |
| bottlecap/src/appsec/processor/context.rs | Stores held trace header tags as OwnedTracerHeaderTags and converts back via to_tracer_header_tags(). |
| bottlecap/Cargo.toml | Bumps libdatadog + serverless-components git rev pins and removes [patch.crates-io]. |
| bottlecap/Cargo.lock | Reflects new dependency graph and the intended registry+git libdd-* duplication. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
libdatadog to 72fa8685 and serverless-components to 9daae40
a3a610b to
5110d8d
Compare
Wire DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED, DD_TRACE_STATS_ADDITIONAL_TAGS, and DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT into StatsConcentratorService, matching the Serverless Compatibility Layer (datadog-trace-agent). Lets users configure span meta keys as additional stats aggregation dimensions (ClientGroupedStats.additional_metric_tags), gated behind the experimental features flag. libdd-trace-stats (pinned via #1332) already implements additional_metric_tag_keys end-to-end; this only adds the bottlecap-side config plumbing. The deprecated span_derived_primary_tags proto field (superseded by additional_metric_tags) intentionally stays empty. Depends on #1332 (libdatadog/serverless-components rev bump); base this branch on lpimentel/bump-libdatadog-72fa8685 until that merges.
5110d8d to
7996ec2
Compare
Wire DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED, DD_TRACE_STATS_ADDITIONAL_TAGS, and DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT into StatsConcentratorService, matching the Serverless Compatibility Layer (datadog-trace-agent). Lets users configure span meta keys as additional stats aggregation dimensions (ClientGroupedStats.additional_metric_tags), gated behind the experimental features flag. libdd-trace-stats (pinned via #1332) already implements additional_metric_tag_keys end-to-end; this only adds the bottlecap-side config plumbing. The deprecated span_derived_primary_tags proto field (superseded by additional_metric_tags) intentionally stays empty. Depends on #1332 (libdatadog/serverless-components rev bump); base this branch on lpimentel/bump-libdatadog-72fa8685 until that merges.
The excess-key warning predicted libdatadog's normalization instead of reading it: a hand-copied `MAX_ADDITIONAL_METRIC_TAG_KEYS = 4` mirroring a private upstream constant, plus a local re-implementation of its sort/dedup/truncate. Both could drift silently, and the copy would then name the wrong keys as dropped. libdatadog already exposes the survivors via `SpanConcentrator::additional_metric_tag_keys()`, so ask for them instead: diff the requested list against the kept list, move the warning to after `SpanConcentrator::new`, and delete the constant and the mirror. The effective cap is now `kept.len()` rather than a number we assert on faith. The reworked test builds a real concentrator and checks which keys survive, so it exercises upstream's actual rule -- and confirms the cap is in fact 4, which nothing previously verified. Also drop the #1332 reference from the `resolve_cardinality_limits` doc comment; it is stale once that PR merges, and the rationale reads better stated directly. 🤖
… hold path as one struct HoldArguments flattened all 10 TracerHeaderTags fields with a tracer_header_tags_ prefix, purely to own the Strings across the hold boundary. OwnedTracerHeaderTags already does that job, so collapse the 10 fields into a single header_tags: OwnedTracerHeaderTags. No functional change.
…to 9daae40 No functional change intended. Bumps bottlecap's 7 direct libdatadog dependencies from 85ce322a to 72fa8685, and its 3 serverless-components dependencies (dogstatsd, datadog-fips, datadog-agent-config) from d0c7f44 to 9daae40, which pins the same libdatadog rev. Both bumps land together because SCL's Cargo.lock at d0c7f44 pinned libdatadog to 85ce322a; bumping only one side leaves the two `ReplaceRule` types structurally identical but resolved as different crate instances, which rustc rejects. Header-tag regrouping: libdatadog #2279 split `TracerHeaderTags`'s four plain-value fields (client_computed_stats, client_computed_top_level, dropped_p0_traces, dropped_p0_spans) into a nested TracerGenericTags. Mechanical follow-through at every construction site and the three field reads that touched them directly. SpanConcentrator::new signature: 5th param changed from `override_max_entries_per_bucket: Option<usize>` to `override_cardinality_limits: Option<CardinalityLimitConfig>`, plus a new 6th `additional_metric_tag_keys: Vec<String>` param. Stats cardinality limits are explicitly pinned (whole_key_limit: usize::MAX, all four per-field limits: usize::MAX - 1) to preserve bottlecap's pre-existing unbounded aggregation; passing None would silently opt into the new default caps (7000 whole-key, 1024 resource, etc.) and collapse high-cardinality Lambda stats into the tracer_blocked_value overflow bucket. `[patch.crates-io]` removed: rev 72fa8685 carries an unreleased breaking libdd-telemetry change under an unchanged crate version, which makes dd-trace-rs's datadog-opentelemetry v0.5.0 fail to build against a patched libdd-telemetry. Letting the tracer resolve libdd-* from crates.io (the arrangement serverless-components already uses) avoids that, at the cost of 11 duplicated libdd-* crates in the dependency graph. bottlecap's own libdd-trace-stats no longer enables stats-obfuscation as a side effect (that feature arrived only via the now-deleted patch's crate-id unification), which is why SpanConcentrator::new above drops its obfuscation argument entirely rather than gaining a 7th parameter.
Build the held-trace sender by spreading the existing processor instead of copying each field, so it does not need updating when a field is added. Also document why the libdd-* crates are no longer deduplicated via `[patch.crates-io]`, and how to re-check when bumping libdatadog or dd-trace-rs. 🤖
7996ec2 to
b7ea62c
Compare
Wire DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED, DD_TRACE_STATS_ADDITIONAL_TAGS, and DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT into StatsConcentratorService, matching the Serverless Compatibility Layer (datadog-trace-agent). Lets users configure span meta keys as additional stats aggregation dimensions (ClientGroupedStats.additional_metric_tags), gated behind the experimental features flag. libdd-trace-stats (pinned via #1332) already implements additional_metric_tag_keys end-to-end; this only adds the bottlecap-side config plumbing. The deprecated span_derived_primary_tags proto field (superseded by additional_metric_tags) intentionally stays empty. Depends on #1332 (libdatadog/serverless-components rev bump); base this branch on lpimentel/bump-libdatadog-72fa8685 until that merges.
The excess-key warning predicted libdatadog's normalization instead of reading it: a hand-copied `MAX_ADDITIONAL_METRIC_TAG_KEYS = 4` mirroring a private upstream constant, plus a local re-implementation of its sort/dedup/truncate. Both could drift silently, and the copy would then name the wrong keys as dropped. libdatadog already exposes the survivors via `SpanConcentrator::additional_metric_tag_keys()`, so ask for them instead: diff the requested list against the kept list, move the warning to after `SpanConcentrator::new`, and delete the constant and the mirror. The effective cap is now `kept.len()` rather than a number we assert on faith. The reworked test builds a real concentrator and checks which keys survive, so it exercises upstream's actual rule -- and confirms the cap is in fact 4, which nothing previously verified. Also drop the #1332 reference from the `resolve_cardinality_limits` doc comment; it is stale once that PR merges, and the rationale reads better stated directly. 🤖
litianningdatadog
left a comment
There was a problem hiding this comment.
LGTM. Will we release only after the whole stack is merged? By the time do we plan to restore patch.crates-io section to avoid dup libs in binary?
Not the whole stack necessarily. First I needed to do some version bumps and dependency management (#1332, this PR). Bumping those version pulled in some upstream changes that needed to be handled as well, so I did that in #1338. I kept them in separate PRs so it was (hopefully!) easier to review. I will merge them into The other two PRs are the ones that add new features. These are more independent and don't necessarily have to be in the same release:
I don't have a timeline for this. Short term, restoring the Long-term, we need to do some refactoring to avoid dependency issues like these. For example, |
|
Also, for reference, this whole version bump (which includes the duplication of the |
Wire DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED, DD_TRACE_STATS_ADDITIONAL_TAGS, and DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT into StatsConcentratorService, matching the Serverless Compatibility Layer (datadog-trace-agent). Lets users configure span meta keys as additional stats aggregation dimensions (ClientGroupedStats.additional_metric_tags), gated behind the experimental features flag. libdd-trace-stats (pinned via #1332) already implements additional_metric_tag_keys end-to-end; this only adds the bottlecap-side config plumbing. The deprecated span_derived_primary_tags proto field (superseded by additional_metric_tags) intentionally stays empty. Depends on #1332 (libdatadog/serverless-components rev bump); base this branch on lpimentel/bump-libdatadog-72fa8685 until that merges.
The excess-key warning predicted libdatadog's normalization instead of reading it: a hand-copied `MAX_ADDITIONAL_METRIC_TAG_KEYS = 4` mirroring a private upstream constant, plus a local re-implementation of its sort/dedup/truncate. Both could drift silently, and the copy would then name the wrong keys as dropped. libdatadog already exposes the survivors via `SpanConcentrator::additional_metric_tag_keys()`, so ask for them instead: diff the requested list against the kept list, move the warning to after `SpanConcentrator::new`, and delete the constant and the mirror. The effective cap is now `kept.len()` rather than a number we assert on faith. The reworked test builds a real concentrator and checks which keys survive, so it exercises upstream's actual rule -- and confirms the cap is in fact 4, which nothing previously verified. Also drop the #1332 reference from the `resolve_cardinality_limits` doc comment; it is stale once that PR merges, and the rationale reads better stated directly. 🤖
Wire DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED, DD_TRACE_STATS_ADDITIONAL_TAGS, and DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT into StatsConcentratorService, matching the Serverless Compatibility Layer (datadog-trace-agent). Lets users configure span meta keys as additional stats aggregation dimensions (ClientGroupedStats.additional_metric_tags), gated behind the experimental features flag. libdd-trace-stats (pinned via #1332) already implements additional_metric_tag_keys end-to-end; this only adds the bottlecap-side config plumbing. The deprecated span_derived_primary_tags proto field (superseded by additional_metric_tags) intentionally stays empty. Depends on #1332 (libdatadog/serverless-components rev bump); base this branch on lpimentel/bump-libdatadog-72fa8685 until that merges.
The excess-key warning predicted libdatadog's normalization instead of reading it: a hand-copied `MAX_ADDITIONAL_METRIC_TAG_KEYS = 4` mirroring a private upstream constant, plus a local re-implementation of its sort/dedup/truncate. Both could drift silently, and the copy would then name the wrong keys as dropped. libdatadog already exposes the survivors via `SpanConcentrator::additional_metric_tag_keys()`, so ask for them instead: diff the requested list against the kept list, move the warning to after `SpanConcentrator::new`, and delete the constant and the mirror. The effective cap is now `kept.len()` rather than a number we assert on faith. The reworked test builds a real concentrator and checks which keys survive, so it exercises upstream's actual rule -- and confirms the cap is in fact 4, which nothing previously verified. Also drop the #1332 reference from the `resolve_cardinality_limits` doc comment; it is stale once that PR merges, and the rationale reads better stated directly. 🤖
Stacked PRs:
datadog-agent-trace-samplercrate serverless-components#141libdatadogto72fa8685andserverless-componentsto9daae40#1332 👈🏽 This PROverview
Tactical dependency bump, behavior-preserving: every changed call site is adapted to the
new APIs without altering runtime behavior. In particular the new
override_cardinality_limitsparameter is pinned to reproduce bottlecap's pre-bumpaggregation exactly (see commit 2); adopting libdatadog's default cardinality limits is a
customer-visible change and is deferred to the follow-up PR. Two queued features are blocked
behind the same dependency move:
54e570aeof the shared
datadog-agent-trace-samplercrate, which happens to sit on the samelibdatadog rev bottlecap uses today. Repinning to merged
main(9daae40) pulls inlibdatadog
72fa8685viadatadog-agent-configand hard-fails to compile againstbottlecap's current libdatadog
85ce322a.72fa8685foradditional_metric_tag_keys+CardinalityLimitConfig.This PR does only the bump; both features become pure wiring afterwards.
85ce322a72fa8685d0c7f449daae40dd-trace-rs50bfea87[patch.crates-io](16 entries)Both rev bumps must land together: SCL
d0c7f44'sdatadog-agent-configpinned libdatadog85ce322a, so bumping only one side leaves twoReplaceRuletypes structurally identicalbut resolved as distinct crate instances, which rustc rejects with
E0308.[patch.crates-io]is deleted rather than repointed: rev72fa8685carries an unreleasedbreaking
libdd-telemetrychange under an unchanged crate version (libdatadog #2172), sopatching
datadog-opentelemetryv0.5.0's transitivelibdd-telemetryagainst it failsE0107/E0433. Letting the tracer resolvelibdd-*fromcrates.io— the arrangementserverless-componentsalready uses — avoids that, at the cost of 11 duplicatedlibdd-*crates in the dependency graph. As a side effect, bottlecap's own
libdd-trace-statsnolonger enables
stats-obfuscation(that feature arrived only via the now-deleted patch'scrate-id unification), so
SpanConcentrator::newdrops its obfuscation argument entirely(arity 6, not 7) rather than gaining a new parameter.
dd-trace-rsis intentionally not bumped: v0.5.1 raiseslibdd-samplingto 6.0.0, which72fa8685does not carry. That belongs to a futurecrates.iomigration.Commits
struct — pure no-op cleanup on the old rev.
HoldArgumentsflattened all 10TracerHeaderTagsfields with atracer_header_tags_prefix; collapses them into theOwnedTracerHeaderTagsstruct that already exists for this purpose. Makes the bumpcommit's appsec diff zero.
bump itself:
TracerHeaderTagssplit (libdatadog #2279):client_computed_stats,client_computed_top_level,dropped_p0_traces,dropped_p0_spansmoved into a nestedTracerGenericTags. Mechanical follow-through at every construction site and the fieldreads that touched them directly.
SpanConcentrator::newsignature: 5th param changed fromoverride_max_entries_per_bucket: Option<usize>tooverride_cardinality_limits: Option<CardinalityLimitConfig>, plus a new 6thadditional_metric_tag_keys: Vec<String>param. All five limits are pinned toeffectively-unbounded values so bottlecap's pre-existing unbounded aggregation is
preserved bit-for-bit; passing
Nonewould silently opt intoCardinalityLimitConfig::default(). Per-field limits areusize::MAX - 1rather thanusize::MAXonly because the constructor warns whenwhole_key_limitis not strictlygreater than every per-field limit; both values are unreachable.
additional_metric_tag_keysisVec::new()— no change to the aggregation dimensions.[patch.crates-io]removed (see above).held-trace sender by spreading the existing processor rather than copying each field, so
it does not silently need updating when
SendingTraceProcessorgains one. Also documentsin
Cargo.tomlwhy thelibdd-*crates are no longer deduplicated, and how to re-check(
cargo tree --duplicates | grep ^libdd-) when bumping either side. Repointing the patchat only the non-
libdd-telemetrycrates was tested and does not dedupe: this rev'slibdd-commonis 5.1.0 while publishedlibdd-data-pipeline7.0.0 requires^5.2.0, sothe patch simply goes unused for the registry consumers.
Size impact (amd64)
Built via
ARCHITECTURE=amd64 FIPS=false ./scripts/build_bottlecap_layer.shonmain(
fbf128dc) and on this branch.85ce322a)72fa8685)libdd-*duplicates (cargo tree --duplicates)Both figures land far under the GitLab CI caps for amd64 (27 MB compressed / 54 MB
uncompressed). The release profile (
opt-level = "z", LTO,codegen-units = 1,strip = true) absorbs nearly all of the size cost of the 11 newly-duplicatedlibdd-*crates.
Downstream handoff (not part of this PR)
lpimentel/add-trace-error-sampler([APMSVLS-469] feat(traces): rescue errored traces via agent-side error sampler #1320): rebase ontomain, repin its fourserverless-components deps
54e570ae→9daae40. Conflicts will concentrate inCargo.toml/Cargo.lockplus anyTracerHeaderTagsliterals its commits added.cardinality limits — 7000 whole-key, 1024 resource, 512 http endpoint, 512 peer tags, 100
additional tags — plus reporting when keys collapse. That is where the customer-visible
change lives, so it is reviewed on its own rather than buried in a
chore(deps)PR.lpimentel/span-derived-primary-tags([APMSVLS-485] feat(traces): span-derived primary tags #1336): rebases onto that follow-up PR.Testing
cargo check --workspace --all-targets— clean, no warningscargo fmt --all -- --check— cleancargo clippy --workspace --all-targets --features default -- -D warnings— cleancargo clippy --workspace --all-targets --no-default-features --features fips -- -D warnings— clean (highest-risk check: patch-block deletion changes TLS feature flow)
cargo nextest run --workspace— cleancargo nextest run --workspace -E 'test(cardinality)'—test_no_cardinality_limit_appliedfeeds 7,001 distinct resources (exceeding both thedefault
whole_key_limitof 7,000 andresource_limitof 1,024) and asserts that nokey collapses into
tracer_blocked_value, so a regression back toNonefails it twiceover; passes
cargo audit— 1 pre-existing vulnerability (h20.4.13, RUSTSEC-2026-0258), unchanged bythis PR and present on
mainbefore this branch; 9 unmaintained/unsound warnings, allnon-blocking in CI's
rustsec/audit-checkjobdd-rust-license-tool check— already up to date, noLICENSE-3rdparty.csvchanges needed(crate name-keyed, so the git/crates.io copies of each
libdd-*crate collapse to one row)./scripts/verify_tls_root_features.sh— OK, reqwest root sources correct for both defaultand FIPS builds