From c6620574078b18dc0826449e5e523794dae2a906 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 21 Aug 2026 14:00:26 -0400 Subject: [PATCH 1/7] [APMSVLS-485] feat(traces): span-derived primary tags for stats 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. --- bottlecap/src/config/mod.rs | 111 ++++++++++++++++++ .../src/traces/stats_concentrator_service.rs | 91 ++++++++++++-- 2 files changed, 191 insertions(+), 11 deletions(-) diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index db0a1f192..ee294f23d 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -80,6 +80,19 @@ pub struct LambdaConfig { /// without durable execution context enrichment. Defaults to 0 until the tracer-side /// durable execution support is released; set to 50 to re-enable enrichment. pub lambda_durable_function_log_buffer_size: usize, + + /// `DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED` — gates `additional_metric_tags` and + /// `additional_metric_tags_cardinality_limit` below, matching the Serverless + /// Compatibility Layer (`datadog-trace-agent`). + pub trace_experimental_features_enabled: bool, + /// `DD_TRACE_STATS_ADDITIONAL_TAGS` — comma-separated span `meta` keys included as + /// additional dimensions on trace stats aggregation (`ClientGroupedStats.additional_metric_tags`). + /// Only honored when `trace_experimental_features_enabled` is true. + pub additional_metric_tags: Vec, + /// `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT` — per-bucket cap on distinct + /// `additional_metric_tags` value combinations; `None` uses libdatadog's default (100). + /// Only honored when `trace_experimental_features_enabled` is true. + pub additional_metric_tags_cardinality_limit: Option, } impl Default for LambdaConfig { @@ -104,6 +117,9 @@ impl Default for LambdaConfig { api_security_sample_delay: Duration::from_secs(30), custom_metrics_exclude_tags: Vec::new(), lambda_durable_function_log_buffer_size: 0, + trace_experimental_features_enabled: false, + additional_metric_tags: Vec::new(), + additional_metric_tags_cardinality_limit: None, } } } @@ -180,6 +196,23 @@ pub struct LambdaConfigSource { /// 0 (hold mechanism disabled). #[serde(deserialize_with = "deser_opt_lossless")] pub lambda_durable_function_log_buffer_size: Option, + + /// `DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED` — see `LambdaConfig::trace_experimental_features_enabled`. + #[serde(deserialize_with = "deser_opt_bool")] + pub trace_experimental_features_enabled: Option, + /// `DD_TRACE_STATS_ADDITIONAL_TAGS` — see `LambdaConfig::additional_metric_tags`. + /// Gated on `trace_experimental_features_enabled` in `merge_from`, not here. Field is + /// named `trace_stats_additional_tags` (rather than `additional_metric_tags`) so it maps + /// to the `DD_TRACE_STATS_ADDITIONAL_TAGS` env var via the field-name-to-env-var convention. + #[serde(deserialize_with = "deser_csv")] + pub trace_stats_additional_tags: Vec, + /// `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT` — see + /// `LambdaConfig::additional_metric_tags_cardinality_limit`. Gated on + /// `trace_experimental_features_enabled` in `merge_from`, not here. See + /// `trace_stats_additional_tags` above for why the field name differs from the + /// `LambdaConfig` field it merges into. + #[serde(deserialize_with = "deser_opt_lossless")] + pub trace_stats_additional_tags_cardinality_limit: Option, } impl DatadogConfigExtension for LambdaConfig { @@ -204,6 +237,7 @@ impl DatadogConfigExtension for LambdaConfig { api_security_enabled, api_security_sample_delay, lambda_durable_function_log_buffer_size, + trace_experimental_features_enabled, ], option: [span_dedup_timeout, api_key_secret_reload_interval, appsec_rules], ); @@ -227,6 +261,23 @@ impl DatadogConfigExtension for LambdaConfig { self.custom_metrics_exclude_tags .clone_from(&source.lambda_customer_metrics_exclude_tags); } + + // additional_metric_tags / additional_metric_tags_cardinality_limit are only + // honored when trace_experimental_features_enabled is true, matching the Serverless + // Compatibility Layer (datadog-trace-agent). When the gate is off, always reset + // both to their defaults so a stale/misconfigured env var can't leak through. + if self.trace_experimental_features_enabled { + if !source.trace_stats_additional_tags.is_empty() { + self.additional_metric_tags + .clone_from(&source.trace_stats_additional_tags); + } + if let Some(limit) = source.trace_stats_additional_tags_cardinality_limit { + self.additional_metric_tags_cardinality_limit = Some(limit); + } + } else { + self.additional_metric_tags.clear(); + self.additional_metric_tags_cardinality_limit = None; + } } } @@ -709,4 +760,64 @@ mod lambda_config_tests { // Default is true. assert!(config.ext.enhanced_metrics); } + + // ---- additional_metric_tags (span-derived primary tags), gated on + // trace_experimental_features_enabled, matching the Serverless Compatibility Layer + // (datadog-trace-agent) ---- + + #[test] + fn additional_metric_tags_ignored_when_experimental_features_disabled() { + let config = load(|jail| { + jail.set_env("DD_TRACE_STATS_ADDITIONAL_TAGS", "region,tenant_id"); + Ok(()) + }); + assert!(!config.ext.trace_experimental_features_enabled); + assert!(config.ext.additional_metric_tags.is_empty()); + } + + #[test] + fn additional_metric_tags_from_env_when_trace_experimental_features_enabled() { + let config = load(|jail| { + jail.set_env("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "true"); + jail.set_env("DD_TRACE_STATS_ADDITIONAL_TAGS", "region, tenant_id"); + Ok(()) + }); + assert!(config.ext.trace_experimental_features_enabled); + assert_eq!( + config.ext.additional_metric_tags, + vec!["region".to_string(), "tenant_id".to_string()] + ); + } + + #[test] + fn additional_metric_tags_cardinality_limit_ignored_when_experimental_features_disabled() { + let config = load(|jail| { + jail.set_env("DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT", "5"); + Ok(()) + }); + assert_eq!(config.ext.additional_metric_tags_cardinality_limit, None); + } + + #[test] + fn additional_metric_tags_cardinality_limit_from_env_when_experimental_gate_enabled() { + let config = load(|jail| { + jail.set_env("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "true"); + jail.set_env("DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT", "5"); + Ok(()) + }); + assert_eq!(config.ext.additional_metric_tags_cardinality_limit, Some(5)); + } + + #[test] + fn additional_metric_tags_cardinality_limit_invalid_value_falls_back_to_none() { + let config = load(|jail| { + jail.set_env("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "true"); + jail.set_env( + "DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT", + "not-a-number", + ); + Ok(()) + }); + assert_eq!(config.ext.additional_metric_tags_cardinality_limit, None); + } } diff --git a/bottlecap/src/traces/stats_concentrator_service.rs b/bottlecap/src/traces/stats_concentrator_service.rs index e8af8da00..f9ada9038 100644 --- a/bottlecap/src/traces/stats_concentrator_service.rs +++ b/bottlecap/src/traces/stats_concentrator_service.rs @@ -285,7 +285,17 @@ impl StatsConcentratorService { pub fn new(config: Arc) -> (Self, StatsConcentratorHandle) { let (tx, rx) = mpsc::unbounded_channel(); let handle = StatsConcentratorHandle::new(tx); - let cardinality_limits = CardinalityLimitConfig::default(); + // Overriding `additional_tags_limit` from `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT` + // is resolved here, once, so the limits the collapse warnings quote are the same values + // the concentrator enforces. + let cardinality_limits = config + .ext + .additional_metric_tags_cardinality_limit + .map(|additional_tags_limit| CardinalityLimitConfig { + additional_tags_limit, + ..Default::default() + }) + .unwrap_or_default(); let concentrator = SpanConcentrator::new( Duration::from_nanos(BUCKET_DURATION_NS), SystemTime::now(), @@ -297,18 +307,21 @@ impl StatsConcentratorService { .iter() .map(ToString::to_string) .collect(), - // Use libdatadog's default cardinality limits, matching the trace agent and - // the Serverless Compatibility Layer: 7000 whole-key, 1024 resource, 512 http - // endpoint, 512 peer tags, 100 additional tags. Keys beyond a limit collapse - // into the `tracer_blocked_value` overflow bucket, which bounds concentrator - // memory and the /v0.6/stats payload inside a memory-capped Lambda. + // Use libdatadog's default cardinality limits except for `additional_tags_limit`, + // which is overridden by `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT` when + // set (matching the Serverless Compatibility Layer / `datadog-trace-agent`). + // Defaults: 7000 whole-key, 1024 resource, 512 http endpoint, 512 peer tags, 100 + // additional tags. Keys beyond a limit collapse into the `tracer_blocked_value` + // overflow bucket, which bounds concentrator memory and the /v0.6/stats payload + // inside a memory-capped Lambda. // - // Passed explicitly rather than as `None` (which libdatadog resolves with - // `unwrap_or_default()`, so the two are equivalent) so that the limits the - // collapse warnings quote are provably the ones in force. + // Passed as `Some` of the resolved value rather than the raw `Option` (which + // libdatadog would resolve with `unwrap_or_default()`, so the two are equivalent) + // so that the limits the collapse warnings quote are provably the ones in force. Some(cardinality_limits), - // No additional stats tag keys: aggregate on the default key fields only. - Vec::new(), + // Span meta keys included as additional aggregation dimensions, from + // DD_TRACE_STATS_ADDITIONAL_TAGS (only set when experimental_features_enabled). + config.ext.additional_metric_tags.clone(), ); let service: StatsConcentratorService = Self { concentrator, @@ -579,6 +592,62 @@ mod tests { ); } + /// `additional_metric_tags` (populated from `DD_TRACE_STATS_ADDITIONAL_TAGS`, gated on + /// `DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED`) should surface matching span `meta` keys as + /// `ClientGroupedStats.additional_metric_tags` on export. + #[tokio::test] + async fn test_additional_metric_tags_populated_when_configured() { + let mut config = Config::default(); + config.ext.additional_metric_tags = vec!["datacenter".to_string()]; + let config = Arc::new(config); + let (service, handle) = StatsConcentratorService::new(config); + tokio::spawn(service.run()); + + let span = create_span_kind_span("client", vec![("datacenter", "us-east-1")]); + handle.add(&span).unwrap(); + + let result = handle.flush(true).await.unwrap(); + let payload = result.expect("Expected stats for the client span, but got None."); + let all_stats: Vec<_> = payload.stats.iter().flat_map(|b| &b.stats).collect(); + assert!( + all_stats + .iter() + .any(|s| s.additional_metric_tags == vec!["datacenter:us-east-1".to_string()]), + "Expected additional_metric_tags to contain datacenter:us-east-1, got: {:?}", + all_stats + .iter() + .map(|s| &s.additional_metric_tags) + .collect::>() + ); + } + + /// When `additional_metric_tags` is unset (the default), `additional_metric_tags` on the + /// exported stats must remain empty even if the span has a meta key that would otherwise + /// match a commonly-used tag name. + #[tokio::test] + async fn test_additional_metric_tags_empty_by_default() { + let config = Arc::new(Config::default()); + let (service, handle) = StatsConcentratorService::new(config); + tokio::spawn(service.run()); + + let span = create_span_kind_span("client", vec![("datacenter", "us-east-1")]); + handle.add(&span).unwrap(); + + let result = handle.flush(true).await.unwrap(); + let payload = result.expect("Expected stats for the client span, but got None."); + let all_stats: Vec<_> = payload.stats.iter().flat_map(|b| &b.stats).collect(); + assert!( + all_stats + .iter() + .all(|s| s.additional_metric_tags.is_empty()), + "Expected additional_metric_tags to be empty by default, got: {:?}", + all_stats + .iter() + .map(|s| &s.additional_metric_tags) + .collect::>() + ); + } + /// The concentrator uses `CardinalityLimitConfig::default()`, so exceeding those limits must /// collapse the excess aggregation keys into the `tracer_blocked_value` overflow key instead /// of growing without bound. 7,001 distinct resources exceeds both the default From ff85116c762ba1cb60ab50ab4bddeb209160294f Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 21 Aug 2026 15:55:27 -0400 Subject: [PATCH 2/7] [APMSVLS-485] fix(traces): validate additional tags cardinality limit libdatadog warns about out-of-range cardinality limits but still applies them. Validate DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT before passing it through: - 0 collapsed every additional metric tag into tracer_blocked_value; now falls back to libdatadog's default of 100. - Values at or above the whole-key limit (7000) are clamped to 6999, mainly to silence libdatadog's misconfiguration warning. Per-field limits are applied before the whole-key limit, so such a value is effectively unbounded rather than inert; either way, reaching it needs ~7k distinct tag combinations in a single 10s bucket, which will not happen in a Lambda invocation. Both log a warning naming the effective value. --- .../src/traces/stats_concentrator_service.rs | 88 ++++++++++++++++--- 1 file changed, 77 insertions(+), 11 deletions(-) diff --git a/bottlecap/src/traces/stats_concentrator_service.rs b/bottlecap/src/traces/stats_concentrator_service.rs index f9ada9038..ff33f2e75 100644 --- a/bottlecap/src/traces/stats_concentrator_service.rs +++ b/bottlecap/src/traces/stats_concentrator_service.rs @@ -175,6 +175,45 @@ fn is_sentinel_tag(tag: &str) -> bool { tag.split_once(':').map_or(tag, |(key, _)| key) == TRACER_BLOCKED_VALUE } +/// Build the `CardinalityLimitConfig` override for a user-supplied +/// `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT`, or `None` to keep libdatadog's defaults. +/// +/// libdatadog only warns about out-of-range limits, it still applies them, so validate here: +/// `0` would collapse *every* additional tag into the `tracer_blocked_value` sentinel, and any +/// value at or above `whole_key_limit` is inert because the whole-key limit collapses the key +/// first. Both are almost certainly misconfigurations rather than intent. +fn resolve_cardinality_limits(configured_limit: Option) -> Option { + let defaults = CardinalityLimitConfig::default(); + // `saturating_sub` keeps the clamp below the whole-key limit so it stays effective. + let max_effective_limit = defaults.whole_key_limit.saturating_sub(1); + + let additional_tags_limit = match configured_limit? { + 0 => { + warn!( + "DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT=0 would collapse all additional \ + metric tags into `tracer_blocked_value`; using the default of {} instead.", + defaults.additional_tags_limit + ); + return None; + } + limit if limit > max_effective_limit => { + warn!( + "DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT={limit} is at or above the \ + whole-key cardinality limit ({}), which would make it ineffective; clamping to \ + {max_effective_limit}.", + defaults.whole_key_limit + ); + max_effective_limit + } + limit => limit, + }; + + Some(CardinalityLimitConfig { + additional_tags_limit, + ..defaults + }) +} + #[derive(Debug, thiserror::Error)] pub enum StatsError { #[error("Failed to send command to concentrator: {0}")] @@ -285,17 +324,12 @@ impl StatsConcentratorService { pub fn new(config: Arc) -> (Self, StatsConcentratorHandle) { let (tx, rx) = mpsc::unbounded_channel(); let handle = StatsConcentratorHandle::new(tx); - // Overriding `additional_tags_limit` from `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT` - // is resolved here, once, so the limits the collapse warnings quote are the same values - // the concentrator enforces. - let cardinality_limits = config - .ext - .additional_metric_tags_cardinality_limit - .map(|additional_tags_limit| CardinalityLimitConfig { - additional_tags_limit, - ..Default::default() - }) - .unwrap_or_default(); + // Resolved once, here, so the limits the collapse warnings quote are the same values the + // concentrator enforces. `unwrap_or_default()` mirrors what libdatadog does with a `None` + // override. + let cardinality_limits = + resolve_cardinality_limits(config.ext.additional_metric_tags_cardinality_limit) + .unwrap_or_default(); let concentrator = SpanConcentrator::new( Duration::from_nanos(BUCKET_DURATION_NS), SystemTime::now(), @@ -648,6 +682,38 @@ mod tests { ); } + /// libdatadog only warns about out-of-range cardinality limits and still applies them, so + /// `resolve_cardinality_limits` has to reject the two misconfigurations that would silently + /// break stats: `0` (collapses every additional tag) and any value at or above the whole-key + /// limit (inert, because the whole-key limit collapses the key first). + #[test] + fn test_resolve_cardinality_limits() { + let defaults = CardinalityLimitConfig::default(); + + // Unset: keep libdatadog's defaults entirely. + assert_eq!(resolve_cardinality_limits(None), None); + + // 0 would collapse everything, fall back to the defaults. + assert_eq!(resolve_cardinality_limits(Some(0)), None); + + // In-range values are applied, leaving the other limits at their defaults. + let resolved = resolve_cardinality_limits(Some(5)).expect("expected an override"); + assert_eq!(resolved.additional_tags_limit, 5); + assert_eq!(resolved.whole_key_limit, defaults.whole_key_limit); + assert_eq!(resolved.resource_limit, defaults.resource_limit); + + // At or above the whole-key limit is clamped so it stays effective. + let clamped = resolve_cardinality_limits(Some(defaults.whole_key_limit)) + .expect("expected an override"); + assert_eq!(clamped.additional_tags_limit, defaults.whole_key_limit - 1); + let clamped_high = + resolve_cardinality_limits(Some(usize::MAX)).expect("expected an override"); + assert_eq!( + clamped_high.additional_tags_limit, + defaults.whole_key_limit - 1 + ); + } + /// The concentrator uses `CardinalityLimitConfig::default()`, so exceeding those limits must /// collapse the excess aggregation keys into the `tracer_blocked_value` overflow key instead /// of growing without bound. 7,001 distinct resources exceeds both the default From 2c1385146a210228699fff43aecf0f7da82f3f72 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 21 Aug 2026 16:12:31 -0400 Subject: [PATCH 3/7] [APMSVLS-485] fix(traces): warn when additional tag keys exceed cap libdatadog aggregates on at most 4 additional metric tag keys, sorting alphabetically and dropping the rest, so excess keys are chosen by alphabetical accident rather than by anything the user expressed. Its warning names the dropped keys but not the kept ones, the selection rule, or the env var; restate all three in bottlecap's voice. Truncation itself stays in libdatadog. Also clarify the cardinality limit warnings. The Go trace agent reads 0 as "no cap", so spell out that 0 does not mean unlimited here and point at unsetting DD_TRACE_STATS_ADDITIONAL_TAGS to actually disable the dimension. Per-field limits apply before the whole-key limit, so a limit at or above the whole-key limit is effectively unbounded rather than inert; the clamp mainly silences libdatadog's misconfiguration warning. --- .../src/traces/stats_concentrator_service.rs | 104 +++++++++++++++++- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/bottlecap/src/traces/stats_concentrator_service.rs b/bottlecap/src/traces/stats_concentrator_service.rs index ff33f2e75..450063f65 100644 --- a/bottlecap/src/traces/stats_concentrator_service.rs +++ b/bottlecap/src/traces/stats_concentrator_service.rs @@ -175,13 +175,28 @@ fn is_sentinel_tag(tag: &str) -> bool { tag.split_once(':').map_or(tag, |(key, _)| key) == TRACER_BLOCKED_VALUE } +/// Maximum number of additional metric tag keys libdatadog will aggregate on. +/// +/// TODO: mirrors `ADDITIONAL_METRIC_TAGS_MAX_KEYS` in libdatadog's +/// `libdd-trace-stats/src/span_concentrator/mod.rs`, which is private. Hand-copied here only to +/// warn about excess keys in bottlecap's own terms; libdatadog still owns the actual truncation. +const MAX_ADDITIONAL_METRIC_TAG_KEYS: usize = 4; + /// Build the `CardinalityLimitConfig` override for a user-supplied /// `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT`, or `None` to keep libdatadog's defaults. /// -/// libdatadog only warns about out-of-range limits, it still applies them, so validate here: -/// `0` would collapse *every* additional tag into the `tracer_blocked_value` sentinel, and any -/// value at or above `whole_key_limit` is inert because the whole-key limit collapses the key -/// first. Both are almost certainly misconfigurations rather than intent. +/// libdatadog only warns about out-of-range limits, it still applies them, so validate here. +/// +/// `0` is the dangerous one: libdatadog would collapse *every* additional tag into the +/// `tracer_blocked_value` sentinel. Note the Go trace agent reads `0` as "no cap" instead, so a +/// user carrying that setting over would otherwise silently lose every tag value. Falling back to +/// the default keeps aggregation working; "unbounded" is deliberately not offered, since #1332 +/// bounded this precisely to cap concentrator memory in a memory-capped Lambda. +/// +/// Values at or above `whole_key_limit` are clamped mainly to silence libdatadog's +/// misconfiguration warning. Per-field limits are applied *before* the whole-key limit, so such a +/// value is not strictly inert, but reaching it needs ~7k distinct tag combinations inside one +/// 10s bucket, which will not happen in a Lambda invocation. fn resolve_cardinality_limits(configured_limit: Option) -> Option { let defaults = CardinalityLimitConfig::default(); // `saturating_sub` keeps the clamp below the whole-key limit so it stays effective. @@ -191,7 +206,9 @@ fn resolve_cardinality_limits(configured_limit: Option) -> Option { warn!( "DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT=0 would collapse all additional \ - metric tags into `tracer_blocked_value`; using the default of {} instead.", + metric tags into `tracer_blocked_value`; using the default of {} instead. Note \ + that 0 does not mean unlimited here; to stop aggregating on additional tags, \ + unset DD_TRACE_STATS_ADDITIONAL_TAGS instead.", defaults.additional_tags_limit ); return None; @@ -199,7 +216,7 @@ fn resolve_cardinality_limits(configured_limit: Option) -> Option max_effective_limit => { warn!( "DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT={limit} is at or above the \ - whole-key cardinality limit ({}), which would make it ineffective; clamping to \ + whole-key cardinality limit ({}), so it is effectively unbounded; clamping to \ {max_effective_limit}.", defaults.whole_key_limit ); @@ -214,6 +231,43 @@ fn resolve_cardinality_limits(configured_limit: Option) -> Option Option<(Vec<&str>, Vec<&str>)> { + let mut normalized: Vec<&str> = keys.iter().map(String::as_str).collect(); + normalized.sort_unstable(); + normalized.dedup(); + + if normalized.len() <= MAX_ADDITIONAL_METRIC_TAG_KEYS { + return None; + } + let dropped = normalized.split_off(MAX_ADDITIONAL_METRIC_TAG_KEYS); + Some((normalized, dropped)) +} + #[derive(Debug, thiserror::Error)] pub enum StatsError { #[error("Failed to send command to concentrator: {0}")] @@ -324,6 +378,7 @@ impl StatsConcentratorService { pub fn new(config: Arc) -> (Self, StatsConcentratorHandle) { let (tx, rx) = mpsc::unbounded_channel(); let handle = StatsConcentratorHandle::new(tx); + warn_on_excess_additional_metric_tag_keys(&config.ext.additional_metric_tags); // Resolved once, here, so the limits the collapse warnings quote are the same values the // concentrator enforces. `unwrap_or_default()` mirrors what libdatadog does with a `None` // override. @@ -714,6 +769,43 @@ mod tests { ); } + /// libdatadog silently keeps only the first `MAX_ADDITIONAL_METRIC_TAG_KEYS` keys after + /// sorting alphabetically, so which keys survive is an alphabetical accident rather than + /// anything the user expressed. Verify the split we report matches that rule. + #[test] + fn test_split_additional_metric_tag_keys() { + let keys = + |keys: &[&str]| -> Vec { keys.iter().map(ToString::to_string).collect() }; + + // Within the cap: nothing is dropped. + assert_eq!(split_additional_metric_tag_keys(&[]), None); + assert_eq!( + split_additional_metric_tag_keys(&keys(&["region", "shard", "zone", "tenant_id"])), + None + ); + + // Duplicates collapse first, so this stays within the cap. + assert_eq!( + split_additional_metric_tag_keys(&keys(&["region", "region", "shard"])), + None + ); + + // Over the cap: alphabetical order decides, so `zone` loses despite being listed first. + assert_eq!( + split_additional_metric_tag_keys(&keys(&[ + "zone", + "tenant_id", + "region", + "shard", + "customer" + ])), + Some(( + vec!["customer", "region", "shard", "tenant_id"], + vec!["zone"] + )) + ); + } + /// The concentrator uses `CardinalityLimitConfig::default()`, so exceeding those limits must /// collapse the excess aggregation keys into the `tracer_blocked_value` overflow key instead /// of growing without bound. 7,001 distinct resources exceeds both the default From b71154da8d73022861ebf8fad68d7560e0e19f4b Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 21 Aug 2026 19:31:26 -0400 Subject: [PATCH 4/7] refactor(traces): derive the dropped-key warning from the concentrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. 🤖 --- .../src/traces/stats_concentrator_service.rs | 139 +++++++++--------- 1 file changed, 73 insertions(+), 66 deletions(-) diff --git a/bottlecap/src/traces/stats_concentrator_service.rs b/bottlecap/src/traces/stats_concentrator_service.rs index 450063f65..904db2bab 100644 --- a/bottlecap/src/traces/stats_concentrator_service.rs +++ b/bottlecap/src/traces/stats_concentrator_service.rs @@ -175,13 +175,6 @@ fn is_sentinel_tag(tag: &str) -> bool { tag.split_once(':').map_or(tag, |(key, _)| key) == TRACER_BLOCKED_VALUE } -/// Maximum number of additional metric tag keys libdatadog will aggregate on. -/// -/// TODO: mirrors `ADDITIONAL_METRIC_TAGS_MAX_KEYS` in libdatadog's -/// `libdd-trace-stats/src/span_concentrator/mod.rs`, which is private. Hand-copied here only to -/// warn about excess keys in bottlecap's own terms; libdatadog still owns the actual truncation. -const MAX_ADDITIONAL_METRIC_TAG_KEYS: usize = 4; - /// Build the `CardinalityLimitConfig` override for a user-supplied /// `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT`, or `None` to keep libdatadog's defaults. /// @@ -190,8 +183,8 @@ const MAX_ADDITIONAL_METRIC_TAG_KEYS: usize = 4; /// `0` is the dangerous one: libdatadog would collapse *every* additional tag into the /// `tracer_blocked_value` sentinel. Note the Go trace agent reads `0` as "no cap" instead, so a /// user carrying that setting over would otherwise silently lose every tag value. Falling back to -/// the default keeps aggregation working; "unbounded" is deliberately not offered, since #1332 -/// bounded this precisely to cap concentrator memory in a memory-capped Lambda. +/// the default keeps aggregation working; "unbounded" is deliberately not offered, since these +/// limits exist precisely to cap concentrator memory inside a memory-capped Lambda. /// /// Values at or above `whole_key_limit` are clamped mainly to silence libdatadog's /// misconfiguration warning. Per-field limits are applied *before* the whole-key limit, so such a @@ -233,39 +226,34 @@ fn resolve_cardinality_limits(configured_limit: Option) -> Option Option<(Vec<&str>, Vec<&str>)> { - let mut normalized: Vec<&str> = keys.iter().map(String::as_str).collect(); - normalized.sort_unstable(); - normalized.dedup(); - - if normalized.len() <= MAX_ADDITIONAL_METRIC_TAG_KEYS { - return None; +/// libdatadog normalizes the requested keys (sort, dedup, truncate to its own private cap) and +/// exposes the survivors via `SpanConcentrator::additional_metric_tag_keys()`, so `kept` is asked +/// for rather than recomputed — no hand-copied cap and no mirrored normalization to drift out of +/// sync with upstream. Excess keys are dropped by alphabetical accident rather than by anything +/// the user expressed, and libdatadog's own warning names the dropped keys but not the kept ones, +/// the selection rule, or the env var, so restate all three here. Truncation itself is left to +/// libdatadog; this only reports it. +fn warn_on_excess_additional_metric_tag_keys(requested: &[String], kept: &[String]) { + let mut dropped: Vec<&str> = requested + .iter() + .map(String::as_str) + .filter(|key| !kept.iter().any(|k| k == key)) + .collect(); + if dropped.is_empty() { + return; } - let dropped = normalized.split_off(MAX_ADDITIONAL_METRIC_TAG_KEYS); - Some((normalized, dropped)) + // The request may repeat a dropped key; report each once, ordered as libdatadog sorts them. + dropped.sort_unstable(); + dropped.dedup(); + + warn!( + "DD_TRACE_STATS_ADDITIONAL_TAGS lists {} unique keys but at most {} are aggregated on. \ + Keys are sorted alphabetically and the rest dropped, so stats will use {kept:?} and \ + ignore {dropped:?}. Reduce the list to at most {} keys to choose explicitly.", + kept.len() + dropped.len(), + kept.len(), + kept.len(), + ); } #[derive(Debug, thiserror::Error)] @@ -378,7 +366,6 @@ impl StatsConcentratorService { pub fn new(config: Arc) -> (Self, StatsConcentratorHandle) { let (tx, rx) = mpsc::unbounded_channel(); let handle = StatsConcentratorHandle::new(tx); - warn_on_excess_additional_metric_tag_keys(&config.ext.additional_metric_tags); // Resolved once, here, so the limits the collapse warnings quote are the same values the // concentrator enforces. `unwrap_or_default()` mirrors what libdatadog does with a `None` // override. @@ -412,6 +399,12 @@ impl StatsConcentratorService { // DD_TRACE_STATS_ADDITIONAL_TAGS (only set when experimental_features_enabled). config.ext.additional_metric_tags.clone(), ); + // After construction, so the kept keys can be read back off the concentrator rather than + // predicted. + warn_on_excess_additional_metric_tag_keys( + &config.ext.additional_metric_tags, + concentrator.additional_metric_tag_keys(), + ); let service: StatsConcentratorService = Self { concentrator, rx, @@ -769,40 +762,54 @@ mod tests { ); } - /// libdatadog silently keeps only the first `MAX_ADDITIONAL_METRIC_TAG_KEYS` keys after - /// sorting alphabetically, so which keys survive is an alphabetical accident rather than - /// anything the user expressed. Verify the split we report matches that rule. + /// The dropped keys are derived by diffing the request against what the concentrator actually + /// kept, so this asserts on libdatadog's real normalization rather than on a mirrored copy of + /// it: build a concentrator with the requested keys and check which survive. + /// + /// Which keys survive is an alphabetical accident rather than anything the user expressed, + /// which is the whole reason the warning exists. #[test] - fn test_split_additional_metric_tag_keys() { - let keys = - |keys: &[&str]| -> Vec { keys.iter().map(ToString::to_string).collect() }; + fn test_kept_and_dropped_additional_metric_tag_keys() { + let concentrator_keys = |requested: &[&str]| -> Vec { + let concentrator = SpanConcentrator::new( + Duration::from_nanos(BUCKET_DURATION_NS), + SystemTime::now(), + Vec::new(), + Vec::new(), + None, + requested.iter().map(ToString::to_string).collect(), + ); + concentrator.additional_metric_tag_keys().to_vec() + }; - // Within the cap: nothing is dropped. - assert_eq!(split_additional_metric_tag_keys(&[]), None); + // Within the cap: everything is kept, so nothing is dropped. + assert!(concentrator_keys(&[]).is_empty()); assert_eq!( - split_additional_metric_tag_keys(&keys(&["region", "shard", "zone", "tenant_id"])), - None + concentrator_keys(&["region", "shard", "zone", "tenant_id"]), + vec!["region", "shard", "tenant_id", "zone"], + "Within the cap every key is kept, sorted." ); - // Duplicates collapse first, so this stays within the cap. + // Duplicates collapse, so this stays within the cap. assert_eq!( - split_additional_metric_tag_keys(&keys(&["region", "region", "shard"])), - None + concentrator_keys(&["region", "region", "shard"]), + vec!["region", "shard"] ); // Over the cap: alphabetical order decides, so `zone` loses despite being listed first. + let requested = ["zone", "tenant_id", "region", "shard", "customer"]; + let kept = concentrator_keys(&requested); + assert_eq!(kept, vec!["customer", "region", "shard", "tenant_id"]); + + let dropped: Vec<&str> = requested + .iter() + .copied() + .filter(|key| !kept.iter().any(|k| k == key)) + .collect(); assert_eq!( - split_additional_metric_tag_keys(&keys(&[ - "zone", - "tenant_id", - "region", - "shard", - "customer" - ])), - Some(( - vec!["customer", "region", "shard", "tenant_id"], - vec!["zone"] - )) + dropped, + vec!["zone"], + "The warning reports exactly the keys the concentrator did not keep." ); } From 36d1ef181d334af98970b423417e7c06ff958693 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 25 Aug 2026 08:53:09 -0400 Subject: [PATCH 5/7] [APMSVLS-485] fix(traces): honor additional stats tags set in datadog.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additional trace stats tags configured in datadog.yaml were silently dropped whenever the experimental-features gate was enabled through the environment instead. Config sources merge one at a time (datadog.yaml first, then env vars), and the gate was checked during each merge, so the yaml values were cleared before the env-var pass could turn the gate on. The gate now runs once, after every source has merged, so the gate and the values it gates can come from different sources in either order. 🤖 --- bottlecap/src/config/mod.rs | 89 +++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 18 deletions(-) diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index ee294f23d..2661bf1f4 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -28,7 +28,9 @@ pub type Config = datadog_agent_config::Config; #[inline] #[must_use] pub fn get_config(config_directory: &Path) -> Config { - get_config_with_extension::(config_directory) + let mut config = get_config_with_extension::(config_directory); + config.ext.apply_experimental_features_gate(); + config } // --------------------------------------------------------------------------- // LambdaConfig — bottlecap's `ConfigExtension` for the shared @@ -262,19 +264,32 @@ impl DatadogConfigExtension for LambdaConfig { .clone_from(&source.lambda_customer_metrics_exclude_tags); } - // additional_metric_tags / additional_metric_tags_cardinality_limit are only - // honored when trace_experimental_features_enabled is true, matching the Serverless - // Compatibility Layer (datadog-trace-agent). When the gate is off, always reset - // both to their defaults so a stale/misconfigured env var can't leak through. - if self.trace_experimental_features_enabled { - if !source.trace_stats_additional_tags.is_empty() { - self.additional_metric_tags - .clone_from(&source.trace_stats_additional_tags); - } - if let Some(limit) = source.trace_stats_additional_tags_cardinality_limit { - self.additional_metric_tags_cardinality_limit = Some(limit); - } - } else { + // trace_stats_additional_tags (source) → additional_metric_tags (config), and likewise + // for the cardinality limit. Merged unconditionally here: `merge_from` runs once per + // config source (datadog.yaml, then env vars), so gating on + // `trace_experimental_features_enabled` at this point would discard a value read from + // datadog.yaml whenever the gate itself only arrives with the later env-var pass. + // `apply_experimental_features_gate` applies the gate once, after every source has + // merged. + if !source.trace_stats_additional_tags.is_empty() { + self.additional_metric_tags + .clone_from(&source.trace_stats_additional_tags); + } + if let Some(limit) = source.trace_stats_additional_tags_cardinality_limit { + self.additional_metric_tags_cardinality_limit = Some(limit); + } + } +} + +impl LambdaConfig { + /// Drop `additional_metric_tags` / `additional_metric_tags_cardinality_limit` unless + /// `trace_experimental_features_enabled` is set, matching the Serverless Compatibility + /// Layer (`datadog-trace-agent`). + /// + /// Applied after all config sources have merged, not inside `merge_from`, so that the gate + /// and the values it gates can come from different sources in either order. + fn apply_experimental_features_gate(&mut self) { + if !self.trace_experimental_features_enabled { self.additional_metric_tags.clear(); self.additional_metric_tags_cardinality_limit = None; } @@ -285,9 +300,7 @@ impl DatadogConfigExtension for LambdaConfig { #[cfg(test)] #[allow(clippy::unwrap_used)] mod lambda_config_tests { - use datadog_agent_config::{ - Config as UpstreamConfig, flush_strategy::PeriodicStrategy, get_config_with_extension, - }; + use datadog_agent_config::{Config as UpstreamConfig, flush_strategy::PeriodicStrategy}; use figment::Jail; use super::*; @@ -299,7 +312,9 @@ mod lambda_config_tests { Jail::expect_with(|jail| { jail.clear_env(); jail_setup(jail)?; - result = Some(get_config_with_extension::(Path::new(""))); + // `get_config`, not `get_config_with_extension`, so the post-merge + // `apply_experimental_features_gate` step is covered too. + result = Some(get_config(Path::new(""))); Ok(()) }); result.unwrap() @@ -820,4 +835,42 @@ mod lambda_config_tests { }); assert_eq!(config.ext.additional_metric_tags_cardinality_limit, None); } + + /// The gate and the values it gates may come from different config sources. Sources merge + /// one at a time (datadog.yaml first, then env vars), so gating during the merge would + /// drop the yaml values before the env-var pass ever enables the gate. + #[test] + fn additional_metric_tags_from_yaml_survive_an_env_only_experimental_gate() { + let config = load(|jail| { + jail.create_file( + "datadog.yaml", + "trace_stats_additional_tags: \"region,zone\"\n\ + trace_stats_additional_tags_cardinality_limit: 7\n", + )?; + jail.set_env("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "true"); + Ok(()) + }); + assert_eq!( + config.ext.additional_metric_tags, + vec!["region".to_string(), "zone".to_string()] + ); + assert_eq!(config.ext.additional_metric_tags_cardinality_limit, Some(7)); + } + + /// The mirror of the above: an env-var gate of `false` must still win over yaml values. + #[test] + fn additional_metric_tags_from_yaml_dropped_when_env_disables_the_gate() { + let config = load(|jail| { + jail.create_file( + "datadog.yaml", + "trace_experimental_features_enabled: true\n\ + trace_stats_additional_tags: \"region,zone\"\n\ + trace_stats_additional_tags_cardinality_limit: 7\n", + )?; + jail.set_env("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "false"); + Ok(()) + }); + assert!(config.ext.additional_metric_tags.is_empty()); + assert_eq!(config.ext.additional_metric_tags_cardinality_limit, None); + } } From cb461ce2d3124c0844e19f3ea33be067f9fd6de7 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 25 Aug 2026 08:53:21 -0400 Subject: [PATCH 6/7] [APMSVLS-485] fix(traces): recommend the right remedy per collapsed stats field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When too many distinct additional metric tag sets collapsed, the warning advised removing request ids and path parameters from resource names, which has no effect on additional-tag cardinality. That case now points at the knobs that do help: listing fewer keys in DD_TRACE_STATS_ADDITIONAL_TAGS, choosing keys with fewer distinct values, or raising DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT. The other fields keep the reduce-cardinality advice. 🤖 --- .../src/traces/stats_concentrator_service.rs | 44 +++++++++++++------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/bottlecap/src/traces/stats_concentrator_service.rs b/bottlecap/src/traces/stats_concentrator_service.rs index 904db2bab..19a0741b7 100644 --- a/bottlecap/src/traces/stats_concentrator_service.rs +++ b/bottlecap/src/traces/stats_concentrator_service.rs @@ -110,20 +110,42 @@ impl CollapsedFields { self.0 & field != 0 } - /// Each field's bit, the noun to use when reporting it, and the limit that governs it. - fn reportable(limits: &CardinalityLimitConfig) -> [(u8, &'static str, usize); 4] { + /// Each field's bit, the noun to use when reporting it, the limit that governs it, and the + /// remediation to recommend. + /// + /// `additional_tags` is the only field with a customer-facing knob, so it is the only one + /// whose message names an environment variable. The rest name none deliberately: libdatadog's + /// own message blames `DD_TRACE_STATS_CARDINALITY_LIMIT`, which bottlecap does not read at + /// all, so reducing cardinality in the application is the only real remediation. + fn reportable(limits: &CardinalityLimitConfig) -> [(u8, &'static str, usize, &'static str); 4] { + const REDUCE_CARDINALITY: &str = "Reduce cardinality to keep trace stats accurate; \ + request ids or path parameters embedded in resource names are the usual cause."; + const TUNE_ADDITIONAL_TAGS: &str = "List fewer keys in DD_TRACE_STATS_ADDITIONAL_TAGS, pick keys with fewer distinct \ + values, or raise DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT."; [ - (Self::RESOURCE, "resource names", limits.resource_limit), + ( + Self::RESOURCE, + "resource names", + limits.resource_limit, + REDUCE_CARDINALITY, + ), ( Self::HTTP_ENDPOINT, "HTTP endpoints", limits.http_endpoint_limit, + REDUCE_CARDINALITY, + ), + ( + Self::PEER_TAGS, + "peer tag sets", + limits.peer_tags_limit, + REDUCE_CARDINALITY, ), - (Self::PEER_TAGS, "peer tag sets", limits.peer_tags_limit), ( Self::ADDITIONAL_TAGS, "additional metric tag sets", limits.additional_tags_limit, + TUNE_ADDITIONAL_TAGS, ), ] } @@ -523,15 +545,11 @@ impl StatsConcentratorService { ); } - // Names no environment variable, deliberately: the per-field limits are not - // customer-tunable in bottlecap, and both candidate knobs would mislead. libdatadog's own - // message blames `DD_TRACE_STATS_CARDINALITY_LIMIT`, which bottlecap does not read at all, - // and `DD_TRACE_STATS_ADDITIONAL_TAGS_CARDINALITY_LIMIT` governs only `additional_tags` - // (and only once the additional-tags feature is enabled). Reducing cardinality in the - // application is the only real remediation, so that is what this recommends. + // The remediation is per field: see `CollapsedFields::reportable` for which fields name + // an environment variable and why the others do not. let bucket_secs = Duration::from_nanos(BUCKET_DURATION_NS).as_secs(); let observed = observe_collapsed_fields(buckets); - for (field, noun, limit) in CollapsedFields::reportable(&self.cardinality_limits) { + for (field, noun, limit, remedy) in CollapsedFields::reportable(&self.cardinality_limits) { if !observed.contains(field) || self.reported_collapsed_fields.contains(field) { continue; } @@ -539,9 +557,7 @@ impl StatsConcentratorService { warn!( "Trace stats saw more than {limit} distinct {noun} in a {bucket_secs}s bucket; \ the excess is aggregated under '{TRACER_BLOCKED_VALUE}', so those stats are no \ - longer attributable. Reduce cardinality to keep trace stats accurate; request \ - ids or path parameters embedded in resource names are the usual cause. Warned \ - once per sandbox." + longer attributable. {remedy} Warned once per sandbox." ); } } From b27a5a3780680b51379664a8dca9f79ccd5edd8f Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 25 Aug 2026 09:45:30 -0400 Subject: [PATCH 7/7] style(traces): remove em dash from dropped-key warning doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 --- bottlecap/src/traces/stats_concentrator_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bottlecap/src/traces/stats_concentrator_service.rs b/bottlecap/src/traces/stats_concentrator_service.rs index 19a0741b7..63a2b20a5 100644 --- a/bottlecap/src/traces/stats_concentrator_service.rs +++ b/bottlecap/src/traces/stats_concentrator_service.rs @@ -250,7 +250,7 @@ fn resolve_cardinality_limits(configured_limit: Option) -> Option