From 32dc0e89cf24514b45498f55e8d61f67e2d71504 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:15:33 -0400 Subject: [PATCH 01/27] feat(rc): parse tracing_sampling_rate field on LibConfig Tracks presence vs. explicit null via `missing_field_and_null_value`, mirroring the pattern already used for `tracing_sampling_rules`. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index f9b8a2b2..e14b1855 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -202,7 +202,17 @@ struct LibConfig { rename = "tracing_sampling_rules" )] tracing_sampling_rules: Option, - // Add other APM tracing config fields as needed (e.g., tracing_header_tags, etc.) + + /// Global trace sample rate (0.0–1.0) pushed via Remote Config. + /// `None` means the field was absent (no change intended). + /// `Some(Value::Null)` means the field was explicitly `null` (clear the override). + /// `Some(Value::Number)` means a concrete rate was provided. + #[serde( + deserialize_with = "missing_field_and_null_value", + default, + rename = "tracing_sampling_rate" + )] + tracing_sampling_rate: Option, } /// TUF targets metadata @@ -2121,4 +2131,32 @@ mod tests { assert!(tracing_config.lib_config.tracing_sampling_rules.is_none()); } + + #[test] + fn test_deserialize_tracing_sampling_rate_concrete() { + let config_json = r#"{"id": "42", "lib_config": {"tracing_sampling_rate": 0.25}}"#; + let tracing_config: ApmTracingConfig = serde_json::from_str(config_json).unwrap(); + assert_eq!( + tracing_config.lib_config.tracing_sampling_rate, + Some(serde_json::json!(0.25)) + ); + } + + #[test] + fn test_deserialize_tracing_sampling_rate_null() { + let config_json = r#"{"id": "42", "lib_config": {"tracing_sampling_rate": null}}"#; + let tracing_config: ApmTracingConfig = serde_json::from_str(config_json).unwrap(); + assert_eq!( + tracing_config.lib_config.tracing_sampling_rate, + Some(serde_json::Value::Null) + ); + } + + #[test] + fn test_deserialize_tracing_sampling_rate_missing() { + // Field absent entirely. + let config_json = r#"{"id": "42", "lib_config": {}}"#; + let tracing_config: ApmTracingConfig = serde_json::from_str(config_json).unwrap(); + assert!(tracing_config.lib_config.tracing_sampling_rate.is_none()); + } } From a3ccbd285963d1e86cb6640cd12419a4b29c7db5 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:17:58 -0400 Subject: [PATCH 02/27] fix(rc): honor tracing_sampling_rate from APM_TRACING payload When the Remote Config payload contains tracing_sampling_rate (alone or alongside tracing_sampling_rules), append a wildcard catch-all rule with provenance=dynamic so libdd-sampling tags traces it samples as REMOTE_DYNAMIC_TRACE_SAMPLING_RULE (-12) instead of LOCAL_USER (-3). Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 82 +++++++++++++++---- 1 file changed, 66 insertions(+), 16 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index e14b1855..e0f5d4a6 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -790,18 +790,54 @@ struct ApmTracingHandler; impl ProductHandler for ApmTracingHandler { fn process_config(&self, config_json: &[u8], config: &Arc) -> Result<()> { - // Parse the config to extract sampling rules as raw JSON let tracing_config: ApmTracingConfig = serde_json::from_slice(config_json) .map_err(|e| anyhow::anyhow!("Failed to parse APM tracing config: {}", e))?; - // Extract sampling rules if present - if let Some(rules_value) = tracing_config.lib_config.tracing_sampling_rules { - if !rules_value.is_null() { - // Convert the raw JSON value to string for the config method - let rules_json = serde_json::to_string(&rules_value) + let lib = tracing_config.lib_config; + + let any_field_present = + lib.tracing_sampling_rules.is_some() || lib.tracing_sampling_rate.is_some(); + let rate: Option = lib.tracing_sampling_rate.as_ref().and_then(|v| v.as_f64()); + let rules_value = match lib.tracing_sampling_rules { + Some(v) if !v.is_null() => Some(v), + _ => None, + }; + + match (rules_value, rate) { + (None, None) => { + if any_field_present { + crate::dd_debug!( + "RemoteConfigClient: APM tracing config received with null sampling fields, clearing remote override" + ); + config.clear_remote_sampling_rules(Some(tracing_config.id)); + } else { + crate::dd_debug!( + "RemoteConfigClient: APM tracing config received but no tracing_sampling_rules or tracing_sampling_rate present" + ); + } + } + (rules_opt, rate_opt) => { + let mut rules: Vec = match rules_opt { + Some(serde_json::Value::Array(arr)) => arr, + Some(other) => { + return Err(anyhow::anyhow!( + "tracing_sampling_rules must be a JSON array, got: {}", + other + )); + } + None => Vec::new(), + }; + if let Some(r) = rate_opt { + rules.push( + serde_json::json!({ "sample_rate": r, "provenance": "dynamic" }), + ); + } + + let rules_json = serde_json::to_string(&serde_json::Value::Array(rules)) .map_err(|e| anyhow::anyhow!("Failed to serialize sampling rules: {}", e))?; - match config.update_sampling_rules_from_remote(&rules_json, Some(tracing_config.id)) + match config + .update_sampling_rules_from_remote(&rules_json, Some(tracing_config.id)) { Ok(()) => { crate::dd_debug!( @@ -815,16 +851,7 @@ impl ProductHandler for ApmTracingHandler { ); } } - } else { - crate::dd_debug!( - "RemoteConfigClient: APM tracing config received but tracing_sampling_rules is null" - ); - config.clear_remote_sampling_rules(Some(tracing_config.id)); } - } else { - crate::dd_debug!( - "RemoteConfigClient: APM tracing config received but no tracing_sampling_rules present" - ); } Ok(()) @@ -901,6 +928,10 @@ mod tests { use proptest::prelude::*; use test_case::test_case; + fn build_config_for_handler() -> Arc { + Arc::new(Config::builder().build()) + } + #[test] fn test_client_capabilities() { let caps = ClientCapabilities::new(); @@ -2159,4 +2190,23 @@ mod tests { let tracing_config: ApmTracingConfig = serde_json::from_str(config_json).unwrap(); assert!(tracing_config.lib_config.tracing_sampling_rate.is_none()); } + + #[test] + fn test_handler_applies_only_rate_as_wildcard_rule() { + // RC sends only tracing_sampling_rate -> handler installs a single + // wildcard rule with provenance "dynamic". + let config = build_config_for_handler(); + let payload = br#"{ + "id": "rc-rate-only", + "lib_config": {"tracing_sampling_rate": 0.25} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 1, "expected exactly one synthesized wildcard rule"); + assert_eq!(rules[0].sample_rate, 0.25); + assert!(rules[0].service.is_none()); + assert!(rules[0].name.is_none()); + assert!(rules[0].resource.is_none()); + assert!(rules[0].tags.is_empty()); + } } From d58f4cab18b6cd163c646dfa574d2ca2d19b3773 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:18:30 -0400 Subject: [PATCH 03/27] test(rc): guard wildcard rate rule appended after rules Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index e0f5d4a6..a860c9d8 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -2209,4 +2209,27 @@ mod tests { assert!(rules[0].resource.is_none()); assert!(rules[0].tags.is_empty()); } + + #[test] + fn test_handler_appends_rate_after_rules() { + // RC sends both rules and a rate -> rate becomes the last (wildcard) rule. + let config = build_config_for_handler(); + let payload = br#"{ + "id": "rc-both", + "lib_config": { + "tracing_sampling_rate": 0.1, + "tracing_sampling_rules": [ + {"sample_rate": 0.9, "service": "auth"} + ] + } + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 2); + assert_eq!(rules[0].sample_rate, 0.9); + assert_eq!(rules[0].service.as_deref(), Some("auth")); + assert_eq!(rules[1].sample_rate, 0.1); + assert!(rules[1].service.is_none()); + assert!(rules[1].tags.is_empty()); + } } From a8487c257c58549fb84be6aac5207e154051af4a Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:19:05 -0400 Subject: [PATCH 04/27] test(rc): guard explicit-null on either field clears prior RC override Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index a860c9d8..b62cc513 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -2232,4 +2232,49 @@ mod tests { assert!(rules[1].service.is_none()); assert!(rules[1].tags.is_empty()); } + + #[test] + fn test_handler_null_fields_clear_prior_override() { + // 1. Install rules via RC. + let config = build_config_for_handler(); + let install = br#"{ + "id": "rc-install", + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(install, &config).unwrap(); + assert_eq!(config.trace_sampling_rules().len(), 1); + + // 2. Send explicit null for both fields -> override cleared. + let clear = br#"{ + "id": "rc-clear", + "lib_config": { + "tracing_sampling_rate": null, + "tracing_sampling_rules": null + } + }"#; + ApmTracingHandler.process_config(clear, &config).unwrap(); + // After clearing, trace_sampling_rules() returns the local-config default + // (empty unless DD_TRACE_SAMPLING_RULES is set in the test environment). + assert_eq!(config.trace_sampling_rules().len(), 0); + } + + #[test] + fn test_handler_null_rate_only_clears_prior_override() { + // Explicit null on tracing_sampling_rate (with tracing_sampling_rules absent) + // must clear a prior remote override. + let config = build_config_for_handler(); + let install = br#"{ + "id": "rc-install", + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(install, &config).unwrap(); + assert_eq!(config.trace_sampling_rules().len(), 1); + + let clear = br#"{ + "id": "rc-clear", + "lib_config": {"tracing_sampling_rate": null} + }"#; + ApmTracingHandler.process_config(clear, &config).unwrap(); + assert_eq!(config.trace_sampling_rules().len(), 0); + } } From d1c21fe449f4e7e9d18dc9da2414c1b04dece7b0 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:21:16 -0400 Subject: [PATCH 05/27] fix(rc): normalize RC list-shape tags before libdd-sampling parse Remote Config encodes sampling rule tags as a list of {key, value_glob} objects, but libdd-sampling's SamplingRuleConfig only deserializes map-shape tags. Walk the RC payload and normalize each rule's tags to map-shape before serializing for libdatadog. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 155 +++++++++++++++++- 1 file changed, 148 insertions(+), 7 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index b62cc513..d5d8eb33 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -185,6 +185,47 @@ where Ok(Some(serde_json::Value::deserialize(deserializer)?)) } +/// Normalizes the `tags` field on each sampling rule from the RC wire shape +/// (list of `{key, value_glob}` objects) into the shape `libdd-sampling`'s +/// `SamplingRuleConfig` accepts (a `{key: value}` map). Map-shape tags are +/// left untouched. Rules without `tags`, or with `tags` of an unexpected +/// type, are left untouched. +/// +/// Malformed list entries (missing `key` or `value_glob`, or non-string values) +/// are skipped rather than failing the whole update — RC schema validation is +/// the agent's job; the tracer should be lenient. +fn normalize_rc_tags(rules: &mut serde_json::Value) { + let Some(arr) = rules.as_array_mut() else { + return; + }; + for rule in arr { + let Some(obj) = rule.as_object_mut() else { + continue; + }; + let Some(tags) = obj.get("tags") else { + continue; + }; + let serde_json::Value::Array(entries) = tags else { + // Map-shape (or null/etc.) — leave it for libdatadog to handle. + continue; + }; + let mut map = serde_json::Map::with_capacity(entries.len()); + for entry in entries { + let (Some(key), Some(value)) = ( + entry.get("key").and_then(|v| v.as_str()), + entry.get("value_glob").and_then(|v| v.as_str()), + ) else { + continue; + }; + map.insert( + key.to_string(), + serde_json::Value::String(value.to_string()), + ); + } + obj.insert("tags".to_string(), serde_json::Value::Object(map)); + } +} + /// Configuration payload for APM tracing /// Based on the apm-tracing.json schema from dd-go /// See: https://github.com/DataDog/dd-go/blob/prod/remote-config/apps/rc-schema-validation/schemas/apm-tracing.json @@ -817,7 +858,7 @@ impl ProductHandler for ApmTracingHandler { } } (rules_opt, rate_opt) => { - let mut rules: Vec = match rules_opt { + let rules: Vec = match rules_opt { Some(serde_json::Value::Array(arr)) => arr, Some(other) => { return Err(anyhow::anyhow!( @@ -827,17 +868,24 @@ impl ProductHandler for ApmTracingHandler { } None => Vec::new(), }; + + // Normalize RC list-shape tags into the map shape libdd-sampling accepts. + let mut rules_value = serde_json::Value::Array(rules); + normalize_rc_tags(&mut rules_value); + let mut rules: Vec = match rules_value { + serde_json::Value::Array(arr) => arr, + // Unreachable — normalize_rc_tags never changes the outer type. + _ => unreachable!("normalize_rc_tags preserves the array wrapper"), + }; + if let Some(r) = rate_opt { - rules.push( - serde_json::json!({ "sample_rate": r, "provenance": "dynamic" }), - ); + rules.push(serde_json::json!({ "sample_rate": r, "provenance": "dynamic" })); } let rules_json = serde_json::to_string(&serde_json::Value::Array(rules)) .map_err(|e| anyhow::anyhow!("Failed to serialize sampling rules: {}", e))?; - match config - .update_sampling_rules_from_remote(&rules_json, Some(tracing_config.id)) + match config.update_sampling_rules_from_remote(&rules_json, Some(tracing_config.id)) { Ok(()) => { crate::dd_debug!( @@ -2202,7 +2250,11 @@ mod tests { }"#; ApmTracingHandler.process_config(payload, &config).unwrap(); let rules = config.trace_sampling_rules().to_vec(); - assert_eq!(rules.len(), 1, "expected exactly one synthesized wildcard rule"); + assert_eq!( + rules.len(), + 1, + "expected exactly one synthesized wildcard rule" + ); assert_eq!(rules[0].sample_rate, 0.25); assert!(rules[0].service.is_none()); assert!(rules[0].name.is_none()); @@ -2277,4 +2329,93 @@ mod tests { ApmTracingHandler.process_config(clear, &config).unwrap(); assert_eq!(config.trace_sampling_rules().len(), 0); } + + #[test] + fn test_handler_rc_rules_with_list_tags_applied() { + // Bug B regression guard: RC sends tags as list-of-objects; the handler + // must normalize to a map before passing to libdatadog. + let config = build_config_for_handler(); + let payload = br#"{ + "id": "rc-list-tags", + "lib_config": { + "tracing_sampling_rules": [ + { + "sample_rate": 0.5, + "service": "svc", + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region", "value_glob": "us-east-1"} + ] + } + ] + } + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 1); + assert_eq!(rules[0].sample_rate, 0.5); + assert_eq!(rules[0].service.as_deref(), Some("svc")); + assert_eq!(rules[0].tags.get("env").map(String::as_str), Some("prod")); + assert_eq!( + rules[0].tags.get("region").map(String::as_str), + Some("us-east-1") + ); + } + + #[test] + fn test_normalize_rc_tags_passes_through_map_shape() { + // Map-shape tags must be left untouched. + let mut rules = serde_json::json!([ + {"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}} + ]); + normalize_rc_tags(&mut rules); + assert_eq!( + rules, + serde_json::json!([ + {"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}} + ]) + ); + } + + #[test] + fn test_normalize_rc_tags_converts_list_shape() { + let mut rules = serde_json::json!([ + { + "sample_rate": 0.5, + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region", "value_glob": "us-east-1"} + ] + } + ]); + normalize_rc_tags(&mut rules); + let tags = rules[0]["tags"] + .as_object() + .expect("tags should be an object after normalization"); + assert_eq!(tags.get("env").and_then(|v| v.as_str()), Some("prod")); + assert_eq!( + tags.get("region").and_then(|v| v.as_str()), + Some("us-east-1") + ); + } + + #[test] + fn test_normalize_rc_tags_drops_malformed_list_entries() { + // A list entry missing `value_glob` or `key` is dropped silently — + // we trust the agent / RC schema to deliver well-formed entries, but + // we don't want one bad entry to kill the entire update. + let mut rules = serde_json::json!([ + { + "sample_rate": 0.5, + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region"} + ] + } + ]); + normalize_rc_tags(&mut rules); + let tags = rules[0]["tags"].as_object().unwrap(); + assert_eq!(tags.get("env").and_then(|v| v.as_str()), Some("prod")); + assert!(tags.get("region").is_none()); + } } From 8713abbe6972363ce5517a39fd351455b80be961 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:25:59 -0400 Subject: [PATCH 06/27] refactor(rc): take normalize_rc_tags by slice, drop unreachable!() Review feedback: the previous wrap-as-Value / unwrap-via-unreachable!() dance violated the CONTRIBUTING guideline against panics in non-test code. Have normalize_rc_tags accept &mut [serde_json::Value] directly and call it on the rules Vec in place. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 57 +++++++------------ 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index d5d8eb33..36a7380e 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -194,11 +194,8 @@ where /// Malformed list entries (missing `key` or `value_glob`, or non-string values) /// are skipped rather than failing the whole update — RC schema validation is /// the agent's job; the tracer should be lenient. -fn normalize_rc_tags(rules: &mut serde_json::Value) { - let Some(arr) = rules.as_array_mut() else { - return; - }; - for rule in arr { +fn normalize_rc_tags(rules: &mut [serde_json::Value]) { + for rule in rules { let Some(obj) = rule.as_object_mut() else { continue; }; @@ -858,7 +855,7 @@ impl ProductHandler for ApmTracingHandler { } } (rules_opt, rate_opt) => { - let rules: Vec = match rules_opt { + let mut rules: Vec = match rules_opt { Some(serde_json::Value::Array(arr)) => arr, Some(other) => { return Err(anyhow::anyhow!( @@ -870,13 +867,7 @@ impl ProductHandler for ApmTracingHandler { }; // Normalize RC list-shape tags into the map shape libdd-sampling accepts. - let mut rules_value = serde_json::Value::Array(rules); - normalize_rc_tags(&mut rules_value); - let mut rules: Vec = match rules_value { - serde_json::Value::Array(arr) => arr, - // Unreachable — normalize_rc_tags never changes the outer type. - _ => unreachable!("normalize_rc_tags preserves the array wrapper"), - }; + normalize_rc_tags(&mut rules); if let Some(r) = rate_opt { rules.push(serde_json::json!({ "sample_rate": r, "provenance": "dynamic" })); @@ -2365,12 +2356,12 @@ mod tests { #[test] fn test_normalize_rc_tags_passes_through_map_shape() { // Map-shape tags must be left untouched. - let mut rules = serde_json::json!([ - {"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}} - ]); + let mut rules: Vec = vec![ + serde_json::json!({"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}}), + ]; normalize_rc_tags(&mut rules); assert_eq!( - rules, + serde_json::Value::Array(rules), serde_json::json!([ {"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}} ]) @@ -2379,15 +2370,13 @@ mod tests { #[test] fn test_normalize_rc_tags_converts_list_shape() { - let mut rules = serde_json::json!([ - { - "sample_rate": 0.5, - "tags": [ - {"key": "env", "value_glob": "prod"}, - {"key": "region", "value_glob": "us-east-1"} - ] - } - ]); + let mut rules: Vec = vec![serde_json::json!({ + "sample_rate": 0.5, + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region", "value_glob": "us-east-1"} + ] + })]; normalize_rc_tags(&mut rules); let tags = rules[0]["tags"] .as_object() @@ -2404,15 +2393,13 @@ mod tests { // A list entry missing `value_glob` or `key` is dropped silently — // we trust the agent / RC schema to deliver well-formed entries, but // we don't want one bad entry to kill the entire update. - let mut rules = serde_json::json!([ - { - "sample_rate": 0.5, - "tags": [ - {"key": "env", "value_glob": "prod"}, - {"key": "region"} - ] - } - ]); + let mut rules: Vec = vec![serde_json::json!({ + "sample_rate": 0.5, + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region"} + ] + })]; normalize_rc_tags(&mut rules); let tags = rules[0]["tags"].as_object().unwrap(); assert_eq!(tags.get("env").and_then(|v| v.as_str()), Some("prod")); From e159a46a73f86eebb429b14f34dec7348eddebf7 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Thu, 21 May 2026 10:28:20 -0400 Subject: [PATCH 07/27] fix(rc): fail closed on malformed adaptive-sampling RC payloads Two correctness fixes surfaced by adversarial review: 1. normalize_rc_tags previously dropped malformed list entries silently. A rule with bad tags could lose constraints and become broader than intended (worst case: a 0% drop rule becomes a service-wide or wildcard drop). Now: if any list entry is malformed, leave the rule's tags in their original (rejected) list shape so libdatadog's parse rejects the entire update. 2. tracing_sampling_rate of a non-null, non-number type (e.g. string) previously fell through to None and, with any_field_present=true, silently cleared the active remote override. Now: reject with an anyhow::Error so the RC dispatcher reports apply_state=3 and the prior policy survives. Regression tests cover both: prior override is preserved when a new update is malformed. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 121 +++++++++++++++--- 1 file changed, 105 insertions(+), 16 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index 36a7380e..c674277a 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -189,11 +189,14 @@ where /// (list of `{key, value_glob}` objects) into the shape `libdd-sampling`'s /// `SamplingRuleConfig` accepts (a `{key: value}` map). Map-shape tags are /// left untouched. Rules without `tags`, or with `tags` of an unexpected -/// type, are left untouched. +/// type, are left untouched (libdatadog's parse will reject if necessary). /// -/// Malformed list entries (missing `key` or `value_glob`, or non-string values) -/// are skipped rather than failing the whole update — RC schema validation is -/// the agent's job; the tracer should be lenient. +/// If any list entry is malformed (missing `key`/`value_glob`, or non-string +/// values), the rule's tags are left in their original (list) shape. This +/// fails closed: libdatadog will reject the list-shape parse, the RC update +/// is dropped, and the agent is informed via apply_state=3. We deliberately +/// do not drop bad entries silently — doing so could broaden a tag-constrained +/// rule into a less-constrained (or fully wildcard) rule. fn normalize_rc_tags(rules: &mut [serde_json::Value]) { for rule in rules { let Some(obj) = rule.as_object_mut() else { @@ -207,19 +210,25 @@ fn normalize_rc_tags(rules: &mut [serde_json::Value]) { continue; }; let mut map = serde_json::Map::with_capacity(entries.len()); + let mut all_ok = true; for entry in entries { let (Some(key), Some(value)) = ( entry.get("key").and_then(|v| v.as_str()), entry.get("value_glob").and_then(|v| v.as_str()), ) else { - continue; + all_ok = false; + break; }; map.insert( key.to_string(), serde_json::Value::String(value.to_string()), ); } - obj.insert("tags".to_string(), serde_json::Value::Object(map)); + if all_ok { + obj.insert("tags".to_string(), serde_json::Value::Object(map)); + } + // else: leave the original list-shape tags in place; libdatadog's + // parse will reject and the RC update is rejected as a whole. } } @@ -835,7 +844,21 @@ impl ProductHandler for ApmTracingHandler { let any_field_present = lib.tracing_sampling_rules.is_some() || lib.tracing_sampling_rate.is_some(); - let rate: Option = lib.tracing_sampling_rate.as_ref().and_then(|v| v.as_f64()); + + // tracing_sampling_rate must be either null (clear) or a JSON number. + // Any other present-but-non-numeric value (string, bool, object) is a + // malformed payload — reject rather than silently treating it as a + // clear, which would wipe an active remote sampling policy. + let rate: Option = match &lib.tracing_sampling_rate { + None | Some(serde_json::Value::Null) => None, + Some(serde_json::Value::Number(n)) => n.as_f64(), + Some(other) => { + return Err(anyhow::anyhow!( + "tracing_sampling_rate must be a JSON number or null, got: {}", + other + )); + } + }; let rules_value = match lib.tracing_sampling_rules { Some(v) if !v.is_null() => Some(v), _ => None, @@ -2389,20 +2412,86 @@ mod tests { } #[test] - fn test_normalize_rc_tags_drops_malformed_list_entries() { - // A list entry missing `value_glob` or `key` is dropped silently — - // we trust the agent / RC schema to deliver well-formed entries, but - // we don't want one bad entry to kill the entire update. - let mut rules: Vec = vec![serde_json::json!({ + fn test_normalize_rc_tags_leaves_malformed_list_untouched() { + // If any list entry is malformed (missing key/value_glob), the rule's + // tags are left in their original list shape — libdatadog's parse will + // then reject the update as a whole. We must not drop bad entries + // silently, which could broaden a tag-constrained rule. + let original = serde_json::json!({ "sample_rate": 0.5, "tags": [ {"key": "env", "value_glob": "prod"}, {"key": "region"} ] - })]; + }); + let mut rules: Vec = vec![original.clone()]; normalize_rc_tags(&mut rules); - let tags = rules[0]["tags"].as_object().unwrap(); - assert_eq!(tags.get("env").and_then(|v| v.as_str()), Some("prod")); - assert!(tags.get("region").is_none()); + // Tags remain in the original (rejected) list shape. + assert_eq!(rules[0], original); + } + + #[test] + fn test_handler_malformed_tags_rejects_update() { + // Bug B fail-closed guard: a sampling rule with malformed list-shape + // tags must not be installed in a broadened form. With the prior + // override of sample_rate=0.5 in place, sending a rule with one bad + // tag entry must leave the prior override intact. + let config = build_config_for_handler(); + // 1. Install a working override. + let install = br#"{ + "id": "rc-install", + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(install, &config).unwrap(); + assert_eq!(config.trace_sampling_rules().len(), 1); + + // 2. Send a rule with a malformed tag entry. + let bad = br#"{ + "id": "rc-bad-tags", + "lib_config": { + "tracing_sampling_rules": [ + { + "sample_rate": 0.0, + "service": "svc", + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region"} + ] + } + ] + } + }"#; + // The libdatadog parse rejects list-shape tags, so the update fails + // internally and is logged at dd_debug! — process_config still returns + // Ok. The key invariant is that the prior remote override is not + // overwritten or cleared. + ApmTracingHandler.process_config(bad, &config).unwrap(); + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 1, "prior override must remain installed"); + assert_eq!(rules[0].sample_rate, 0.5); + } + + #[test] + fn test_handler_non_numeric_rate_rejects_update() { + // A schema-drifted rate (e.g. string) must be rejected as a malformed + // payload, not silently treated as a clear that would wipe an active + // remote override. + let config = build_config_for_handler(); + let install = br#"{ + "id": "rc-install", + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(install, &config).unwrap(); + assert_eq!(config.trace_sampling_rules().len(), 1); + + let bad = br#"{ + "id": "rc-bad-rate", + "lib_config": {"tracing_sampling_rate": "0.5"} + }"#; + let result = ApmTracingHandler.process_config(bad, &config); + assert!(result.is_err(), "non-numeric rate must be rejected"); + // Prior override survives. + assert_eq!(config.trace_sampling_rules().len(), 1); + assert_eq!(config.trace_sampling_rules()[0].sample_rate, 0.5); } } From 8464dd340e319f21ec1c36e6c7df0ec78d5721a4 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 09:30:04 -0400 Subject: [PATCH 08/27] feat(config): expose env-side trace_sampling_rules accessor Adds Config::local_trace_sampling_rules() which returns the env/code/ default value of trace_sampling_rules, ignoring any Remote Config override. The RC handler needs this to compose env rules with the synthetic catch-all rule built from RC's tracing_sampling_rate. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/configuration.rs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index f1f1bcf4..32208d82 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -458,6 +458,13 @@ impl ConfigItemWithOverride { ConfigItemRef::Ref(self.config_item.value()) } } + + /// Returns the env/code/default value, ignoring any remote-config override. + /// Use this when the caller must compose with RC-delivered values without + /// losing the locally-configured value to the override. + fn local_value(&self) -> ConfigItemRef<'_, T> { + ConfigItemRef::Ref(self.config_item.value()) + } } impl ConfigurationProvider @@ -1350,6 +1357,15 @@ impl Config { self.trace_sampling_rules.value() } + /// Returns the locally-configured (env/code/default) trace sampling rules, + /// ignoring any Remote Config override. Used by the RC handler to compose + /// env rules with RC-delivered values without losing them. + pub(crate) fn local_trace_sampling_rules( + &self, + ) -> impl Deref + use<'_> { + self.trace_sampling_rules.local_value() + } + /// Returns the maximum number of traces per second (rate limit). pub fn trace_rate_limit(&self) -> i32 { *self.trace_rate_limit.value() @@ -3242,6 +3258,39 @@ mod tests { assert_eq!(config.trace_sampling_rules.get_config_id(), None); } + #[test] + fn test_local_trace_sampling_rules_bypasses_remote_override() { + let config = Config::builder() + .set_trace_sampling_rules(vec![SamplingRuleConfig { + sample_rate: 0.5, + name: Some("env_name".to_string()), + ..SamplingRuleConfig::default() + }]) + .build(); + + // With no RC override, local == public. + assert_eq!(config.local_trace_sampling_rules().len(), 1); + assert_eq!(config.trace_sampling_rules().len(), 1); + + // Set an RC override. + config + .update_sampling_rules_from_remote( + r#"[{"sample_rate":0.9,"service":"svc","provenance":"customer"}]"#, + None, + ) + .unwrap(); + + // The public accessor reflects the override; the local one still + // returns the env-side value. + assert_eq!(config.trace_sampling_rules().len(), 1); + assert_eq!(config.trace_sampling_rules()[0].sample_rate, 0.9); + + let local = config.local_trace_sampling_rules(); + assert_eq!(local.len(), 1); + assert_eq!(local[0].sample_rate, 0.5); + assert_eq!(local[0].name.as_deref(), Some("env_name")); + } + #[test] fn test_telemetry_config_from_sources() { let mut sources = CompositeSource::new(); From 3dce356543ab8ee4e31fbb3ca86ad44cc97109f2 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 09:33:40 -0400 Subject: [PATCH 09/27] feat(config): add DD_TRACE_SAMPLE_RATE to supported-configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated entry only — the Config field, accessor, and sampler wiring land in the next commits. DD_TRACE_SAMPLE_RATE is the env-var counterpart of the RC tracing_sampling_rate: a global rate applied when no explicit rule matches a span. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/supported_configurations.rs | 2 ++ supported-configurations.json | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/datadog-opentelemetry/src/core/configuration/supported_configurations.rs b/datadog-opentelemetry/src/core/configuration/supported_configurations.rs index 40f0892a..8640a29a 100644 --- a/datadog-opentelemetry/src/core/configuration/supported_configurations.rs +++ b/datadog-opentelemetry/src/core/configuration/supported_configurations.rs @@ -33,6 +33,7 @@ pub(crate) enum SupportedConfigurations { DD_TRACE_PROPAGATION_STYLE_EXTRACT, DD_TRACE_PROPAGATION_STYLE_INJECT, DD_TRACE_RATE_LIMIT, + DD_TRACE_SAMPLE_RATE, DD_TRACE_SAMPLING_RULES, DD_TRACE_STATS_COMPUTATION_ENABLED, DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH, @@ -119,6 +120,7 @@ impl SupportedConfigurations { "DD_TRACE_PROPAGATION_STYLE_INJECT" } SupportedConfigurations::DD_TRACE_RATE_LIMIT => "DD_TRACE_RATE_LIMIT", + SupportedConfigurations::DD_TRACE_SAMPLE_RATE => "DD_TRACE_SAMPLE_RATE", SupportedConfigurations::DD_TRACE_SAMPLING_RULES => "DD_TRACE_SAMPLING_RULES", SupportedConfigurations::DD_TRACE_STATS_COMPUTATION_ENABLED => { "DD_TRACE_STATS_COMPUTATION_ENABLED" diff --git a/supported-configurations.json b/supported-configurations.json index 5fecec73..22b668ab 100644 --- a/supported-configurations.json +++ b/supported-configurations.json @@ -203,6 +203,14 @@ "propertyKeys": ["trace_rate_limit"] } ], + "DD_TRACE_SAMPLE_RATE": [ + { + "version": "A", + "type": "decimal", + "default": "1.0", + "propertyKeys": ["trace_sample_rate"] + } + ], "DD_TRACE_SAMPLING_RULES": [ { "version": "A", From d658e2bc729855eaa87d4b8aa7ec55eafb22aa76 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 09:39:51 -0400 Subject: [PATCH 10/27] feat(config): parse DD_TRACE_SAMPLE_RATE and expose accessor Adds Config::trace_sample_rate() returning the env-configured global trace sample rate. Defaults to 1.0 when unset. The next commit wires this into the sampler as a catch-all rule so it actually takes effect on every span that doesn't match an explicit rule. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/configuration.rs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index 32208d82..6e5e282d 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -934,6 +934,10 @@ pub struct Config { /// If a rule matches, the trace is sampled with the associated sample rate. trace_sampling_rules: SamplingRulesConfigItem, + /// Global trace sample rate (DD_TRACE_SAMPLE_RATE). Applied as a catch-all + /// sample rate when no explicit sampling rule matches. + trace_sample_rate: ConfigItem, + /// Maximum number of spans to sample per second /// Only applied if trace_sampling_rules are matched trace_rate_limit: ConfigItem, @@ -1137,6 +1141,7 @@ impl Config { // Use the initialized ConfigItem trace_sampling_rules: sampling_rules_item, + trace_sample_rate: cisu.update_parsed(default.trace_sample_rate), trace_rate_limit: cisu.update_parsed(default.trace_rate_limit), enabled: cisu.update_parsed(default.enabled), @@ -1226,6 +1231,7 @@ impl Config { &self.dogstatsd_agent_port, &self.dogstatsd_agent_url, &self.trace_sampling_rules, + &self.trace_sample_rate, &self.trace_rate_limit, &self.enabled, &self.log_level_filter, @@ -1371,6 +1377,12 @@ impl Config { *self.trace_rate_limit.value() } + /// Returns the configured global trace sample rate (DD_TRACE_SAMPLE_RATE). + /// Applied as a catch-all sample rate when no explicit sampling rule matches. + pub fn trace_sample_rate(&self) -> f64 { + *self.trace_sample_rate.value() + } + /// Returns whether tracing is enabled. pub fn enabled(&self) -> bool { *self.enabled.value() @@ -1723,6 +1735,7 @@ impl std::fmt::Debug for Config { .field("trace_agent_url", &self.trace_agent_url) .field("dogstatsd_agent_url", &self.dogstatsd_agent_url) .field("trace_sampling_rules", &self.trace_sampling_rules) + .field("trace_sample_rate", &self.trace_sample_rate) .field("trace_rate_limit", &self.trace_rate_limit) .field("enabled", &self.enabled) .field("log_level_filter", &self.log_level_filter) @@ -1800,6 +1813,7 @@ fn default_config() -> Config { SupportedConfigurations::DD_TRACE_SAMPLING_RULES, ParsedSamplingRules::default(), // Empty rules by default ), + trace_sample_rate: ConfigItem::new(SupportedConfigurations::DD_TRACE_SAMPLE_RATE, 1.0_f64), trace_rate_limit: ConfigItem::new(SupportedConfigurations::DD_TRACE_RATE_LIMIT, 100), enabled: ConfigItem::new(SupportedConfigurations::DD_TRACE_ENABLED, true), log_level_filter: ConfigItem::new( @@ -2350,6 +2364,17 @@ impl ConfigBuilder { self } + /// Global trace sample rate. Applied as a catch-all sample rate when no + /// explicit sampling rule matches. + /// + /// **Default**: `1.0` + /// + /// Env variable: `DD_TRACE_SAMPLE_RATE` + pub fn set_trace_sample_rate(&mut self, rate: f64) -> &mut Self { + self.config.trace_sample_rate.set_code(rate); + self + } + /// A list of propagation styles to use for both extraction and injection. Supported values are /// `datadog` and `tracecontext`. /// @@ -3635,4 +3660,21 @@ mod tests { assert_eq!(config.remote_config_poll_interval(), 0.2); } + + #[test] + fn test_trace_sample_rate_defaults_to_one_when_unset() { + let config = Config::builder().build(); + assert_eq!(config.trace_sample_rate(), 1.0); + } + + #[test] + fn test_trace_sample_rate_parses_from_env() { + let mut sources = CompositeSource::new(); + sources.add_source(HashMapSource::from_iter( + [("DD_TRACE_SAMPLE_RATE", "0.25")], + ConfigSourceOrigin::EnvVar, + )); + let config = Config::builder_with_sources(&sources).build(); + assert_eq!(config.trace_sample_rate(), 0.25); + } } From cff0275796fd38d51b86024de582ff9af60db2f5 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 09:45:47 -0400 Subject: [PATCH 11/27] feat(sampling): apply DD_TRACE_SAMPLE_RATE as implicit catch-all When DD_TRACE_SAMPLE_RATE is set (and is not the default 1.0), the sampler now sees an implicit catch-all rule appended after the locally-configured DD_TRACE_SAMPLING_RULES. The catch-all carries "local" provenance so spans it samples get DM "-3" (LOCAL_USER_TRACE_SAMPLING_RULE). The same composition is used when Remote Config clears its override, restoring the env-rate behavior. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/configuration.rs | 83 +++++++++++++++++-- datadog-opentelemetry/src/sampler.rs | 8 +- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index 6e5e282d..2e45c509 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use libdd_telemetry::data::Configuration; -use std::collections::{HashSet, VecDeque}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt::Display; use std::ops::Deref; use std::path::Path; @@ -1612,17 +1612,43 @@ impl Config { } } - pub(crate) fn clear_remote_sampling_rules(&self, config_id: Option) { - self.trace_sampling_rules.unset_override_value(); - self.trace_sampling_rules.set_config_id(config_id); - - // Fallback rules are locally defined, so "local" provenance is correct - let internal: Vec = self - .trace_sampling_rules() + /// Composes the rules that the sampler should see in the absence of any + /// active Remote Config override: locally-configured rules followed by an + /// implicit catch-all that applies `DD_TRACE_SAMPLE_RATE`. + /// + /// The catch-all is appended only when the env rate is finite and not the + /// default of 1.0 — a 1.0 catch-all would match every unmatched span at + /// 100% sampling, which is identical to having no catch-all (libdatadog's + /// fallback path samples at 1.0 when no rule matches), so we omit it to + /// avoid noisy `_dd.p.dm` tags on the wire. + pub(crate) fn effective_initial_rules(&self) -> Vec { + let mut rules: Vec = self + .local_trace_sampling_rules() .iter() .cloned() .map(Into::into) .collect(); + let env_rate = self.trace_sample_rate(); + if env_rate.is_finite() && (env_rate - 1.0_f64).abs() > f64::EPSILON { + rules.push(libdd_sampling::SamplingRuleConfig { + sample_rate: env_rate, + service: None, + name: None, + resource: None, + tags: HashMap::new(), + // "local" maps to LOCAL_USER_TRACE_SAMPLING_RULE (DM -3) per + // libdd-sampling/src/datadog_sampler.rs. + provenance: "local".to_string(), + }); + } + rules + } + + pub(crate) fn clear_remote_sampling_rules(&self, config_id: Option) { + self.trace_sampling_rules.unset_override_value(); + self.trace_sampling_rules.set_config_id(config_id); + + let internal = self.effective_initial_rules(); self.remote_config_callbacks .lock() .unwrap() @@ -3081,6 +3107,47 @@ mod tests { assert_eq!(received[0].provenance, "local"); } + #[test] + fn test_clear_remote_rules_includes_env_rate_catch_all() { + // When DD_TRACE_SAMPLE_RATE is set and the remote override is cleared, + // the callback must receive [env_rules..., catch_all(env_rate)] so the + // sampler applies the env rate to unmatched spans. + let mut config_builder = Config::builder(); + config_builder.set_trace_sample_rate(0.25); + config_builder.set_trace_sampling_rules(vec![SamplingRuleConfig { + sample_rate: 0.5, + name: Some("env_name".to_string()), + ..SamplingRuleConfig::default() + }]); + let config = config_builder.build(); + + let received = Arc::new(Mutex::new(Vec::::new())); + let clone = received.clone(); + config.set_sampling_rules_callback(move |update| { + let RemoteConfigUpdate::SamplingRules(rules) = update; + *clone.lock().unwrap() = rules.clone(); + }); + + // Install a remote override, then clear it. + config + .update_sampling_rules_from_remote( + r#"[{"sample_rate":0.9,"provenance":"customer"}]"#, + None, + ) + .unwrap(); + config.clear_remote_sampling_rules(None); + + let got = received.lock().unwrap(); + assert_eq!(got.len(), 2, "expected env rule + env catch-all"); + assert_eq!(got[0].sample_rate, 0.5); + assert_eq!(got[0].name.as_deref(), Some("env_name")); + assert_eq!(got[1].sample_rate, 0.25); + assert!(got[1].name.is_none()); + assert!(got[1].service.is_none()); + // env catch-all carries local provenance (default mapping → DM -3). + assert_eq!(got[1].provenance, "local"); + } + #[test] fn test_public_sampling_rule_config_ignores_provenance_in_json() { // The public SamplingRuleConfig should silently ignore a "provenance" field in JSON, diff --git a/datadog-opentelemetry/src/sampler.rs b/datadog-opentelemetry/src/sampler.rs index 00c6a69b..ad940258 100644 --- a/datadog-opentelemetry/src/sampler.rs +++ b/datadog-opentelemetry/src/sampler.rs @@ -45,13 +45,7 @@ impl Sampler { // This is an Option to allow benchmarking different parts of sampling trace_registry: Option, ) -> Self { - let internal_configs: Vec = cfg - .trace_sampling_rules() - .iter() - .cloned() - .map(Into::into) - .collect(); - let rules = SamplingRule::from_configs(internal_configs); + let rules = SamplingRule::from_configs(cfg.effective_initial_rules()); let sampler = DatadogSampler::new(rules, cfg.trace_rate_limit()); Self { cfg, From 9ba849ba433c837e1a017160ffd3ca773eb1bd46 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 09:51:56 -0400 Subject: [PATCH 12/27] fix(rc): synthetic catch-all uses default provenance, not dynamic The global RC tracing_sampling_rate is a local-user fallback in the RC schema (DM -3), not a remote-dynamic rule (DM -12). The system- tests parametric scenario asserts this with the "legacy behavior" comment in test_trace_sampling_rules_override_rate. Omit the provenance field from the synthetic catch-all JSON; libdd-sampling's serde default fills it as "default", which maps to LOCAL_USER_TRACE_SAMPLING_RULE (DM -3) per libdd-sampling/src/datadog_sampler.rs. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 41 ++++++++++++++++++- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index c674277a..8ed621a4 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -893,7 +893,11 @@ impl ProductHandler for ApmTracingHandler { normalize_rc_tags(&mut rules); if let Some(r) = rate_opt { - rules.push(serde_json::json!({ "sample_rate": r, "provenance": "dynamic" })); + // The global RC rate is a "local-user-like" fallback: it must + // produce DM "-3" (LOCAL_USER), not "-12" (REMOTE_DYNAMIC). + // Omit `provenance`; libdd-sampling deserializes it as + // "default" via its serde default, which maps to DM -3. + rules.push(serde_json::json!({ "sample_rate": r })); } let rules_json = serde_json::to_string(&serde_json::Value::Array(rules)) @@ -2256,7 +2260,7 @@ mod tests { #[test] fn test_handler_applies_only_rate_as_wildcard_rule() { // RC sends only tracing_sampling_rate -> handler installs a single - // wildcard rule with provenance "dynamic". + // wildcard rule with the libdd default provenance ("default", DM -3). let config = build_config_for_handler(); let payload = br#"{ "id": "rc-rate-only", @@ -2276,6 +2280,39 @@ mod tests { assert!(rules[0].tags.is_empty()); } + #[test] + fn test_handler_synthetic_rate_rule_uses_default_provenance() { + // The synthetic catch-all built from RC's tracing_sampling_rate must + // produce DM "-3" (LOCAL_USER), not "-12" (REMOTE_DYNAMIC). Assert via + // the callback path: libdatadog converts the JSON to its internal + // SamplingRuleConfig, whose `provenance` must be the libdd default + // ("default"), not "dynamic". + use crate::core::configuration::RemoteConfigUpdate; + let config = build_config_for_handler(); + let received = Arc::new(Mutex::new(Vec::::new())); + let clone = received.clone(); + config.set_sampling_rules_callback(move |update| { + let RemoteConfigUpdate::SamplingRules(rules) = update; + *clone.lock().unwrap() = rules.clone(); + }); + + let payload = br#"{ + "id": "rc-rate-only-provenance", + "lib_config": {"tracing_sampling_rate": 0.25} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + + let got = received.lock().unwrap(); + // The callback receives the full composed chain. The catch-all is the + // last entry; it must carry the libdd default provenance ("default"). + let catch_all = got.last().expect("expected at least one rule"); + assert_eq!(catch_all.sample_rate, 0.25); + assert_eq!( + catch_all.provenance, "default", + "synthetic catch-all must use default provenance" + ); + } + #[test] fn test_handler_appends_rate_after_rules() { // RC sends both rules and a rate -> rate becomes the last (wildcard) rule. From e3debe04066f8dff34e2124bbb6eebe81dac9f05 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 10:00:48 -0400 Subject: [PATCH 13/27] fix(rc): respect env rules and env rate in adaptive-sampling composition Rewrites the rule/rate composition in ApmTracingHandler::process_config to match the documented precedence model: - RC explicit rules replace env rules (existing behavior, unchanged). - RC rate-only preserves env rules: [env_rules..., catch_all(rc_rate)]. - The catch-all rate is rc_rate when present, otherwise the env-side DD_TRACE_SAMPLE_RATE if it's non-default. Fixes test_trace_sampling_rate_with_sampling_rules, test_trace_sampling_rules_override_rate, and test_trace_sampling_rules_with_tags in the parametric scenario. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 176 +++++++++++++++++- 1 file changed, 175 insertions(+), 1 deletion(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index 8ed621a4..0b907395 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -878,6 +878,17 @@ impl ProductHandler for ApmTracingHandler { } } (rules_opt, rate_opt) => { + // An explicit empty `tracing_sampling_rules: []` is treated the + // same as null/absent: RC has no rules to deliver, so env-side + // rules survive. Operators clear remote rules by sending + // `tracing_sampling_rules: null` (the conventional RC clear); + // an empty array is an unusual edge case and the lenient + // interpretation is safer than wiping env config silently. + let rc_has_explicit_rules = matches!( + rules_opt, + Some(serde_json::Value::Array(ref arr)) if !arr.is_empty() + ); + let mut rules: Vec = match rules_opt { Some(serde_json::Value::Array(arr)) => arr, Some(other) => { @@ -892,7 +903,37 @@ impl ProductHandler for ApmTracingHandler { // Normalize RC list-shape tags into the map shape libdd-sampling accepts. normalize_rc_tags(&mut rules); - if let Some(r) = rate_opt { + // Multi-source precedence: + // - If RC delivered explicit rules, env rules are replaced. + // - If RC delivered only a rate, env rules survive and apply in front of the + // synthetic catch-all. + if !rc_has_explicit_rules { + let env_rules = config.local_trace_sampling_rules(); + if !env_rules.is_empty() { + let env_json = serde_json::to_value(&*env_rules).map_err(|e| { + anyhow::anyhow!("Failed to serialize env sampling rules: {}", e) + })?; + if let serde_json::Value::Array(env_arr) = env_json { + let mut composed = env_arr; + composed.append(&mut rules); + rules = composed; + } + } + } + + // Effective catch-all rate: RC rate wins; otherwise fall back + // to DD_TRACE_SAMPLE_RATE if it's non-default (we treat 1.0 as + // "no catch-all" since libdd-sampling's no-rule path samples + // at 100% by default anyway). + let env_rate = config.trace_sample_rate(); + let catch_all_rate: Option = match rate_opt { + Some(r) => Some(r), + None if env_rate.is_finite() && (env_rate - 1.0_f64).abs() > f64::EPSILON => { + Some(env_rate) + } + None => None, + }; + if let Some(r) = catch_all_rate { // The global RC rate is a "local-user-like" fallback: it must // produce DM "-3" (LOCAL_USER), not "-12" (REMOTE_DYNAMIC). // Omit `provenance`; libdd-sampling deserializes it as @@ -990,6 +1031,7 @@ fn extract_product_and_id_from_path(path: &str) -> Option<(String, String)> { #[cfg(test)] mod tests { use super::*; + use crate::core::configuration::SamplingRuleConfig; use pretty_assertions::assert_eq; use proptest::prelude::*; use test_case::test_case; @@ -2531,4 +2573,136 @@ mod tests { assert_eq!(config.trace_sampling_rules().len(), 1); assert_eq!(config.trace_sampling_rules()[0].sample_rate, 0.5); } + + #[test] + fn test_handler_rate_only_preserves_env_rules() { + // When RC delivers only tracing_sampling_rate (no rules), env-configured + // sampling rules must still apply for matching spans. Composed chain: + // [env_rules..., catch_all(rc_rate)]. + let env_rule = SamplingRuleConfig { + sample_rate: 0.55, + name: Some("env_name".to_string()), + ..SamplingRuleConfig::default() + }; + let config = Arc::new( + Config::builder() + .set_trace_sampling_rules(vec![env_rule.clone()]) + .build(), + ); + + let payload = br#"{ + "id": "rc-rate-only-with-env-rules", + "lib_config": {"tracing_sampling_rate": 0.70} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 2, "expected env rule + synthetic catch-all"); + assert_eq!(rules[0].sample_rate, 0.55); + assert_eq!(rules[0].name.as_deref(), Some("env_name")); + assert_eq!(rules[1].sample_rate, 0.70); + assert!(rules[1].name.is_none()); + assert!(rules[1].service.is_none()); + assert!(rules[1].resource.is_none()); + assert!(rules[1].tags.is_empty()); + } + + #[test] + fn test_handler_rc_rules_replace_env_rules() { + // Contract: when RC delivers tracing_sampling_rules (with or without a + // rate), env rules are fully replaced. RC rules + catch-all(rc_rate). + let env_rule = SamplingRuleConfig { + sample_rate: 0.55, + name: Some("env_name".to_string()), + ..SamplingRuleConfig::default() + }; + let config = Arc::new( + Config::builder() + .set_trace_sampling_rules(vec![env_rule.clone()]) + .build(), + ); + + let payload = br#"{ + "id": "rc-rules-replace-env", + "lib_config": { + "tracing_sampling_rate": 0.9, + "tracing_sampling_rules": [ + {"sample_rate": 0.8, "service": "svc", "provenance": "customer"} + ] + } + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!( + rules.len(), + 2, + "env rule must be excluded when RC has rules" + ); + assert_eq!(rules[0].sample_rate, 0.8); + assert_eq!(rules[0].service.as_deref(), Some("svc")); + assert_eq!(rules[1].sample_rate, 0.9); + assert!(rules[1].service.is_none()); + assert!(rules.iter().all(|r| r.name.as_deref() != Some("env_name"))); + } + + #[test] + fn test_handler_rc_rules_only_falls_back_to_env_rate_catch_all() { + // RC delivers rules without a rate; DD_TRACE_SAMPLE_RATE is set. The + // composed chain should be [rc_rules..., catch_all(env_rate)] so + // unmatched spans still get sampled at env_rate. + let mut builder = Config::builder(); + builder.set_trace_sample_rate(0.1); + let config = Arc::new(builder.build()); + + let payload = br#"{ + "id": "rc-rules-only-with-env-rate", + "lib_config": { + "tracing_sampling_rules": [ + {"sample_rate": 0.8, "service": "svc", "provenance": "customer"} + ] + } + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 2, "expected rc rule + env-rate catch-all"); + assert_eq!(rules[0].sample_rate, 0.8); + assert_eq!(rules[0].service.as_deref(), Some("svc")); + assert_eq!(rules[1].sample_rate, 0.1); + assert!(rules[1].service.is_none()); + } + + #[test] + fn test_handler_empty_rc_rules_array_preserves_env_rules() { + // Contract: an explicit empty `tracing_sampling_rules: []` is treated as + // "RC has no rules" — env rules survive. Operators clear RC rules by + // sending `tracing_sampling_rules: null`. This test locks that behavior. + let env_rule = SamplingRuleConfig { + sample_rate: 0.55, + name: Some("env_name".to_string()), + ..SamplingRuleConfig::default() + }; + let config = Arc::new( + Config::builder() + .set_trace_sampling_rules(vec![env_rule.clone()]) + .build(), + ); + + let payload = br#"{ + "id": "rc-empty-rules", + "lib_config": { + "tracing_sampling_rate": 0.70, + "tracing_sampling_rules": [] + } + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + + let rules = config.trace_sampling_rules().to_vec(); + assert_eq!(rules.len(), 2, "expected env rule + synthetic catch-all"); + assert_eq!(rules[0].sample_rate, 0.55); + assert_eq!(rules[0].name.as_deref(), Some("env_name")); + assert_eq!(rules[1].sample_rate, 0.70); + assert!(rules[1].name.is_none()); + } } From 16f1fab0229c4977b9d1ff8d64ecb501e8f6f80e Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 10:16:01 -0400 Subject: [PATCH 14/27] fix(rc): address pre-push review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Align env-rate catch-all provenance to "default" (was "local"). Both values map to DM -3 in libdd-sampling today, but "default" is the documented libdatadog default (default_provenance()) and matches the provenance the RC-rate catch-all path produces via serde omission. - Replace silent if-let fallthrough on serialized env_rules with a hard error. serde_json::to_value of a slice always produces Value::Array, but defending against a future serializer change with a silent no-op is fragile — fail loudly instead. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/configuration.rs | 12 +++++++----- .../src/core/configuration/remote_config.rs | 13 ++++++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index 2e45c509..2a941d71 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -1636,9 +1636,10 @@ impl Config { name: None, resource: None, tags: HashMap::new(), - // "local" maps to LOCAL_USER_TRACE_SAMPLING_RULE (DM -3) per - // libdd-sampling/src/datadog_sampler.rs. - provenance: "local".to_string(), + // "default" is libdatadog's documented default provenance + // (default_provenance() in libdd-sampling) and matches the + // value the RC-rate catch-all path produces via serde omission. + provenance: "default".to_string(), }); } rules @@ -3144,8 +3145,9 @@ mod tests { assert_eq!(got[1].sample_rate, 0.25); assert!(got[1].name.is_none()); assert!(got[1].service.is_none()); - // env catch-all carries local provenance (default mapping → DM -3). - assert_eq!(got[1].provenance, "local"); + // env catch-all carries "default" provenance (libdatadog's documented + // default; maps to DM -3 in libdd-sampling). + assert_eq!(got[1].provenance, "default"); } #[test] diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index 0b907395..d416fa9e 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -913,11 +913,14 @@ impl ProductHandler for ApmTracingHandler { let env_json = serde_json::to_value(&*env_rules).map_err(|e| { anyhow::anyhow!("Failed to serialize env sampling rules: {}", e) })?; - if let serde_json::Value::Array(env_arr) = env_json { - let mut composed = env_arr; - composed.append(&mut rules); - rules = composed; - } + let serde_json::Value::Array(env_arr) = env_json else { + return Err(anyhow::anyhow!( + "BUG: serialized env sampling rules are not a JSON array" + )); + }; + let mut composed = env_arr; + composed.append(&mut rules); + rules = composed; } } From 17140e1dd28a34d8e425557dcae4d5962956b9fa Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 10:27:07 -0400 Subject: [PATCH 15/27] fix(rc): propagate RC apply errors, validate rate range, distinguish unset DD_TRACE_SAMPLE_RATE Addresses three Codex adversarial-review findings: - [HIGH] ApmTracingHandler::process_config previously swallowed errors from update_sampling_rules_from_remote, returning Ok(()) even when libdatadog rejected the payload. The RC dispatcher then recorded apply_state=2 (success) and cached the bad target. Propagate the error so apply_state=3 is reported and the target is not cached. - [MEDIUM] Distinguish unset DD_TRACE_SAMPLE_RATE from explicit 1.0. Field type changes from ConfigItem (default 1.0) to ConfigItem> (default None). The 1.0-EPSILON skip in effective_initial_rules is replaced with a plain is_some() check so explicit DD_TRACE_SAMPLE_RATE=1.0 installs a catch-all rule (and DD_TRACE_RATE_LIMIT then applies as expected). - [MEDIUM] Reject tracing_sampling_rate values outside [0.0, 1.0] rather than appending them as catch-all rules with libdatadog silently clamping. Out-of-range now produces apply_state=3. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/configuration.rs | 75 ++++++++----- .../src/core/configuration/remote_config.rs | 104 +++++++++++++----- 2 files changed, 126 insertions(+), 53 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index 2a941d71..7cfa5b37 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -876,7 +876,7 @@ impl ConfigurationValueProvider for Option, bool, u32, usize, i32, f64, ServiceName, LevelFilter, ParsedSamplingRules); -impl_config_value_provider!(option: String); +impl_config_value_provider!(option: String, f64); #[derive(Clone)] /// Configuration for the Datadog Tracer @@ -934,9 +934,10 @@ pub struct Config { /// If a rule matches, the trace is sampled with the associated sample rate. trace_sampling_rules: SamplingRulesConfigItem, - /// Global trace sample rate (DD_TRACE_SAMPLE_RATE). Applied as a catch-all - /// sample rate when no explicit sampling rule matches. - trace_sample_rate: ConfigItem, + /// Global trace sample rate (DD_TRACE_SAMPLE_RATE). `None` means unset + /// (no implicit catch-all; libdatadog's no-rule path samples at 100%). + /// `Some(rate)` installs a catch-all rule so the rate limiter applies. + trace_sample_rate: ConfigItem>, /// Maximum number of spans to sample per second /// Only applied if trace_sampling_rules are matched @@ -1141,7 +1142,8 @@ impl Config { // Use the initialized ConfigItem trace_sampling_rules: sampling_rules_item, - trace_sample_rate: cisu.update_parsed(default.trace_sample_rate), + trace_sample_rate: cisu + .update_parsed_with_transform(default.trace_sample_rate, |r: f64| Some(r)), trace_rate_limit: cisu.update_parsed(default.trace_rate_limit), enabled: cisu.update_parsed(default.enabled), @@ -1377,9 +1379,10 @@ impl Config { *self.trace_rate_limit.value() } - /// Returns the configured global trace sample rate (DD_TRACE_SAMPLE_RATE). - /// Applied as a catch-all sample rate when no explicit sampling rule matches. - pub fn trace_sample_rate(&self) -> f64 { + /// Returns the configured global trace sample rate (DD_TRACE_SAMPLE_RATE), + /// or `None` if unset. Applied as a catch-all sample rate when no explicit + /// sampling rule matches. + pub fn trace_sample_rate(&self) -> Option { *self.trace_sample_rate.value() } @@ -1628,19 +1631,20 @@ impl Config { .cloned() .map(Into::into) .collect(); - let env_rate = self.trace_sample_rate(); - if env_rate.is_finite() && (env_rate - 1.0_f64).abs() > f64::EPSILON { - rules.push(libdd_sampling::SamplingRuleConfig { - sample_rate: env_rate, - service: None, - name: None, - resource: None, - tags: HashMap::new(), - // "default" is libdatadog's documented default provenance - // (default_provenance() in libdd-sampling) and matches the - // value the RC-rate catch-all path produces via serde omission. - provenance: "default".to_string(), - }); + if let Some(env_rate) = self.trace_sample_rate() { + if env_rate.is_finite() { + rules.push(libdd_sampling::SamplingRuleConfig { + sample_rate: env_rate, + service: None, + name: None, + resource: None, + tags: HashMap::new(), + // "default" is libdatadog's documented default provenance + // (default_provenance() in libdd-sampling) and matches the + // value the RC-rate catch-all path produces via serde omission. + provenance: "default".to_string(), + }); + } } rules } @@ -1840,7 +1844,7 @@ fn default_config() -> Config { SupportedConfigurations::DD_TRACE_SAMPLING_RULES, ParsedSamplingRules::default(), // Empty rules by default ), - trace_sample_rate: ConfigItem::new(SupportedConfigurations::DD_TRACE_SAMPLE_RATE, 1.0_f64), + trace_sample_rate: ConfigItem::new(SupportedConfigurations::DD_TRACE_SAMPLE_RATE, None), trace_rate_limit: ConfigItem::new(SupportedConfigurations::DD_TRACE_RATE_LIMIT, 100), enabled: ConfigItem::new(SupportedConfigurations::DD_TRACE_ENABLED, true), log_level_filter: ConfigItem::new( @@ -2398,7 +2402,7 @@ impl ConfigBuilder { /// /// Env variable: `DD_TRACE_SAMPLE_RATE` pub fn set_trace_sample_rate(&mut self, rate: f64) -> &mut Self { - self.config.trace_sample_rate.set_code(rate); + self.config.trace_sample_rate.set_code(Some(rate)); self } @@ -3731,9 +3735,9 @@ mod tests { } #[test] - fn test_trace_sample_rate_defaults_to_one_when_unset() { + fn test_trace_sample_rate_defaults_to_none_when_unset() { let config = Config::builder().build(); - assert_eq!(config.trace_sample_rate(), 1.0); + assert_eq!(config.trace_sample_rate(), None); } #[test] @@ -3744,6 +3748,25 @@ mod tests { ConfigSourceOrigin::EnvVar, )); let config = Config::builder_with_sources(&sources).build(); - assert_eq!(config.trace_sample_rate(), 0.25); + assert_eq!(config.trace_sample_rate(), Some(0.25)); + } + + #[test] + fn test_explicit_dd_trace_sample_rate_one_installs_catch_all() { + // Codex MEDIUM fix: explicit DD_TRACE_SAMPLE_RATE=1.0 must install a + // catch-all rule so the rate limiter applies. Unset is still distinct + // (no catch-all, libdatadog's 100% fallback). + let mut builder = Config::builder(); + builder.set_trace_sample_rate(1.0); + let config = builder.build(); + + let rules = config.effective_initial_rules(); + assert_eq!(rules.len(), 1, "explicit 1.0 must install a catch-all"); + assert_eq!(rules[0].sample_rate, 1.0); + + // Unset: no catch-all. + let unset_config = Config::builder().build(); + let unset_rules = unset_config.effective_initial_rules(); + assert!(unset_rules.is_empty(), "unset must not install a catch-all"); } } diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index d416fa9e..a5ad7e49 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -851,7 +851,20 @@ impl ProductHandler for ApmTracingHandler { // clear, which would wipe an active remote sampling policy. let rate: Option = match &lib.tracing_sampling_rate { None | Some(serde_json::Value::Null) => None, - Some(serde_json::Value::Number(n)) => n.as_f64(), + Some(serde_json::Value::Number(n)) => match n.as_f64() { + Some(r) if r.is_finite() && (0.0..=1.0).contains(&r) => Some(r), + Some(r) => { + return Err(anyhow::anyhow!( + "tracing_sampling_rate must be in [0.0, 1.0], got: {}", + r + )); + } + None => { + return Err(anyhow::anyhow!( + "tracing_sampling_rate is not representable as f64" + )); + } + }, Some(other) => { return Err(anyhow::anyhow!( "tracing_sampling_rate must be a JSON number or null, got: {}", @@ -925,16 +938,12 @@ impl ProductHandler for ApmTracingHandler { } // Effective catch-all rate: RC rate wins; otherwise fall back - // to DD_TRACE_SAMPLE_RATE if it's non-default (we treat 1.0 as - // "no catch-all" since libdd-sampling's no-rule path samples - // at 100% by default anyway). + // to DD_TRACE_SAMPLE_RATE if it's set (Option distinguishes + // unset from explicit 1.0). let env_rate = config.trace_sample_rate(); let catch_all_rate: Option = match rate_opt { Some(r) => Some(r), - None if env_rate.is_finite() && (env_rate - 1.0_f64).abs() > f64::EPSILON => { - Some(env_rate) - } - None => None, + None => env_rate.filter(|r| r.is_finite()), }; if let Some(r) = catch_all_rate { // The global RC rate is a "local-user-like" fallback: it must @@ -947,20 +956,12 @@ impl ProductHandler for ApmTracingHandler { let rules_json = serde_json::to_string(&serde_json::Value::Array(rules)) .map_err(|e| anyhow::anyhow!("Failed to serialize sampling rules: {}", e))?; - match config.update_sampling_rules_from_remote(&rules_json, Some(tracing_config.id)) - { - Ok(()) => { - crate::dd_debug!( - "RemoteConfigClient: Applied sampling rules from remote config" - ); - } - Err(e) => { - crate::dd_debug!( - "RemoteConfigClient: Failed to update sampling rules: {}", - e - ); - } - } + config + .update_sampling_rules_from_remote(&rules_json, Some(tracing_config.id)) + .map_err(|e| { + anyhow::anyhow!("Failed to update sampling rules from remote: {}", e) + })?; + crate::dd_debug!("RemoteConfigClient: Applied sampling rules from remote config"); } } @@ -2543,16 +2544,65 @@ mod tests { ] } }"#; - // The libdatadog parse rejects list-shape tags, so the update fails - // internally and is logged at dd_debug! — process_config still returns - // Ok. The key invariant is that the prior remote override is not - // overwritten or cleared. - ApmTracingHandler.process_config(bad, &config).unwrap(); + // The libdatadog parse rejects list-shape tags, so process_config + // returns Err (post-Codex HIGH fix). The key invariant is that the + // prior remote override is not overwritten or cleared. + let result = ApmTracingHandler.process_config(bad, &config); + assert!(result.is_err(), "malformed tags must propagate as Err"); let rules = config.trace_sampling_rules().to_vec(); assert_eq!(rules.len(), 1, "prior override must remain installed"); assert_eq!(rules[0].sample_rate, 0.5); } + #[test] + fn test_handler_malformed_tags_returns_error_to_dispatcher() { + // After the fix for the Codex HIGH finding: when libdatadog rejects the + // composed JSON (e.g., because the synthetic catch-all rule's tags were + // left in list-shape due to a malformed entry), process_config returns + // Err so the RC dispatcher records apply_state=3 and the bad target is + // not cached. + let config = build_config_for_handler(); + let payload = br#"{ + "id": "rc-bad-tags", + "lib_config": { + "tracing_sampling_rules": [ + { + "sample_rate": 0.5, + "service": "svc", + "tags": [ + {"key": "env", "value_glob": "prod"}, + {"key": "region"} + ] + } + ] + } + }"#; + let result = ApmTracingHandler.process_config(payload, &config); + assert!(result.is_err(), "malformed tags must propagate as Err"); + } + + #[test] + fn test_handler_rejects_negative_rate() { + let config = build_config_for_handler(); + let payload = br#"{ + "id": "rc-neg-rate", + "lib_config": {"tracing_sampling_rate": -0.1} + }"#; + let result = ApmTracingHandler.process_config(payload, &config); + assert!(result.is_err(), "negative rate must be rejected"); + } + + #[test] + fn test_handler_rejects_rate_above_one() { + let config = build_config_for_handler(); + let payload = br#"{ + "id": "rc-high-rate", + "lib_config": {"tracing_sampling_rate": 1.5} + }"#; + let result = ApmTracingHandler.process_config(payload, &config); + assert!(result.is_err(), "rate > 1.0 must be rejected"); + } + #[test] fn test_handler_non_numeric_rate_rejects_update() { // A schema-drifted rate (e.g. string) must be rejected as a malformed From c5442b0557ce5df62576b37387b9f5c3a1da61f7 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 10:39:18 -0400 Subject: [PATCH 16/27] fix(config): align DD_TRACE_SAMPLE_RATE entry with Feature Parity registry v2 The Feature Parity Dashboard registers DD_TRACE_SAMPLE_RATE under multiple version keys per language. Version B (type: decimal, default: null) matches dd-trace-rs's implementation: a decimal sample rate with no default value (Option::None). The previous entry used version A (type: null, default: "undefined"), which is the slot used by older Node.js / Go implementations and caused the validator to flag dd-trace-rs as missing a matching v2 record. Co-Authored-By: Claude Opus 4.7 --- supported-configurations.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/supported-configurations.json b/supported-configurations.json index 22b668ab..db1425ab 100644 --- a/supported-configurations.json +++ b/supported-configurations.json @@ -205,9 +205,9 @@ ], "DD_TRACE_SAMPLE_RATE": [ { - "version": "A", + "version": "B", "type": "decimal", - "default": "1.0", + "default": null, "propertyKeys": ["trace_sample_rate"] } ], From cd344418412c66ff78706bf2cef78bd39eead2af Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Fri, 22 May 2026 16:38:43 -0400 Subject: [PATCH 17/27] fix(rc): advertise APM_TRACING_SAMPLE_RATE capability bit The tracer was only advertising APM_TRACING_SAMPLE_RULES (bit 29) in its RC capabilities bitfield. Datadog's APM Sampling UI checks for APM_TRACING_SAMPLE_RATE (bit 12) independently when deciding whether a service is RC-capable for the rate-only configuration form. Without bit 12 the UI shows "No remotely configurable tracer detected" for this service even though the tracer's logic supports the rate-only case end-to-end after PR #227's process_config rewrite. Add bit 12 to ClientCapabilities::new(). The unit test now asserts both bits are advertised. This also unblocks system-tests test_capability_tracing_sample_rules and test_capability_tracing_sampling_rate which had been left as missing_feature in manifests/rust.yml. Co-Authored-By: Claude Opus 4.7 --- .../src/core/configuration/remote_config.rs | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index a5ad7e49..2a42abcd 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -27,11 +27,19 @@ const DEFAULT_TIMEOUT: Duration = Duration::from_secs(3); // lowest timeout with struct ClientCapabilities(u64); impl ClientCapabilities { - /// APM_TRACING_SAMPLE_RULES capability bit position + /// APM_TRACING_SAMPLE_RATE — bit 12. Tells the backend the tracer + /// honors RC's `tracing_sampling_rate` (global rate). Without this, + /// Datadog's APM Sampling UI marks the service as "No remotely + /// configurable tracer detected" for rate-only configs even when + /// the tracer-side logic is fully wired up. + const APM_TRACING_SAMPLE_RATE: u64 = 1 << 12; + + /// APM_TRACING_SAMPLE_RULES — bit 29. Tells the backend the tracer + /// honors RC's `tracing_sampling_rules`. const APM_TRACING_SAMPLE_RULES: u64 = 1 << 29; fn new() -> Self { - Self(Self::APM_TRACING_SAMPLE_RULES) + Self(Self::APM_TRACING_SAMPLE_RATE | Self::APM_TRACING_SAMPLE_RULES) } /// Encode capabilities as base64 string @@ -1063,8 +1071,17 @@ mod tests { bytes[offset..].copy_from_slice(&decoded); let value = u64::from_be_bytes(bytes); - // Verify the capability bit is set - assert_eq!(value, ClientCapabilities::APM_TRACING_SAMPLE_RULES); + // Both APM_TRACING_SAMPLE_RATE (bit 12) and APM_TRACING_SAMPLE_RULES + // (bit 29) must be advertised; the backend uses each independently to + // decide which RC config types it will offer in the Datadog UI for + // this service. + let expected = ClientCapabilities::APM_TRACING_SAMPLE_RATE + | ClientCapabilities::APM_TRACING_SAMPLE_RULES; + assert_eq!(value, expected); + assert_eq!( + value & ClientCapabilities::APM_TRACING_SAMPLE_RATE, + ClientCapabilities::APM_TRACING_SAMPLE_RATE + ); assert_eq!( value & ClientCapabilities::APM_TRACING_SAMPLE_RULES, ClientCapabilities::APM_TRACING_SAMPLE_RULES From d379c5b5df90ba7a4a8161aa3e2fdf207e44d915 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 08:06:55 -0400 Subject: [PATCH 18/27] chore: bump libdd-sampling 1.0.0 -> 2.1.0, drop RC tags shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libdd-sampling 2.1.0 deserializes Remote Config sampling-rule tags in their wire shape (`[{key, value_glob}]`) natively, so the in-tracer list->map normalization shim (`normalize_rc_tags`) added earlier in this PR is no longer needed. Removes the shim, its single call site, and its 3 direct unit tests; bumps the workspace pin (pulls libdd-common 4.1.0 -> 4.2.0) and regenerates both lockfiles (root + instrumentation, which depends on datadog-opentelemetry by path). Behavior is unchanged from the shim version: - test_handler_rc_rules_with_list_tags_applied: list-shape tags still apply with tags preserved as a map (now via libdd-sampling). - test_handler_malformed_tags_rejects_update: the fail-closed invariant still holds — libdd-sampling 2.1.0 rejects a rule with a malformed list-shape tag entry rather than silently broadening it. The major bump's breaking surface (owned `v04_span` fields) is behind a feature the tracer does not enable (`default-features = false`), so it does not reach this crate. MSRV unchanged (2.1.0 + libdd-common 4.2.0 both require Rust 1.87.0, matching the workspace). cargo test --workspace --lib: 314 passed. clippy + nightly fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 166 +---------------- Cargo.toml | 2 +- datadog-opentelemetry/CHANGELOG.md | 4 + .../src/core/configuration/remote_config.rs | 110 +----------- instrumentation/Cargo.lock | 170 +----------------- 5 files changed, 18 insertions(+), 434 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d0935804..0f00da29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -340,16 +340,6 @@ dependencies = [ "libc", ] -[[package]] -name = "core-foundation" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2a6cd9ae233e7f62ba4e9353e81a88df7fc8a5987b8d445b4d90c879bd156f6" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "core-foundation-sys" version = "0.8.7" @@ -957,22 +947,6 @@ dependencies = [ "want", ] -[[package]] -name = "hyper-rustls" -version = "0.27.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" -dependencies = [ - "http", - "hyper", - "hyper-util", - "rustls", - "rustls-native-certs", - "tokio", - "tokio-rustls", - "tower-service", -] - [[package]] name = "hyper-timeout" version = "0.5.2" @@ -1257,9 +1231,9 @@ dependencies = [ [[package]] name = "libdd-common" -version = "4.1.0" +version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "933c5940905e9a3c8dd8c0439697b3fb327289cef7858370e6a5f440139f6314" +checksum = "4af81e6953fab2814792b8893a2cee9f16aad1b71cb3f8720b7256f28d6a2d8d" dependencies = [ "anyhow", "bytes", @@ -1273,19 +1247,15 @@ dependencies = [ "http-body", "http-body-util", "hyper", - "hyper-rustls", "hyper-util", "libc", "nix", "pin-project", "regex", - "rustls", - "rustls-native-certs", "serde", "static_assertions", "thiserror 1.0.69", "tokio", - "tokio-rustls", "tower-service", "windows-sys 0.52.0", ] @@ -1368,9 +1338,9 @@ dependencies = [ [[package]] name = "libdd-sampling" -version = "1.0.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ae8728d9d717eb1b648223c4c1da53f77627fecd6045027ccb758a72815b300" +checksum = "df229b55274db81b8a85a1b9ab14663af070358f39017ab68d26a3aada535db6" dependencies = [ "libdd-common", "lru", @@ -1653,12 +1623,6 @@ version = "11.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" -[[package]] -name = "openssl-probe" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e" - [[package]] name = "opentelemetry" version = "0.31.0" @@ -2181,20 +2145,6 @@ dependencies = [ "web-sys", ] -[[package]] -name = "ring" -version = "0.17.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" -dependencies = [ - "cc", - "cfg-if", - "getrandom 0.2.17", - "libc", - "untrusted", - "windows-sys 0.52.0", -] - [[package]] name = "rmp" version = "0.8.14" @@ -2259,52 +2209,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "rustls" -version = "0.23.40" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" -dependencies = [ - "once_cell", - "ring", - "rustls-pki-types", - "rustls-webpki", - "subtle", - "zeroize", -] - -[[package]] -name = "rustls-native-certs" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9980d917ebb0c0536119ba501e90834767bffc3d60641457fd84a1f3fd337923" -dependencies = [ - "openssl-probe", - "rustls-pki-types", - "schannel", - "security-framework", -] - -[[package]] -name = "rustls-pki-types" -version = "1.14.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30a7197ae7eb376e574fe940d068c30fe0462554a3ddbe4eca7838e049c937a9" -dependencies = [ - "zeroize", -] - -[[package]] -name = "rustls-webpki" -version = "0.103.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" -dependencies = [ - "ring", - "rustls-pki-types", - "untrusted", -] - [[package]] name = "rustversion" version = "1.0.22" @@ -2326,44 +2230,12 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "schannel" -version = "0.1.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91c1b7e4904c873ef0710c1f407dde2e6287de2bebc1bbbf7d430bb7cbffd939" -dependencies = [ - "windows-sys 0.61.2", -] - [[package]] name = "scopeguard" version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" -[[package]] -name = "security-framework" -version = "3.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d17b898a6d6948c3a8ee4372c17cb384f90d2e6e912ef00895b14fd7ab54ec38" -dependencies = [ - "bitflags", - "core-foundation 0.10.1", - "core-foundation-sys", - "libc", - "security-framework-sys", -] - -[[package]] -name = "security-framework-sys" -version = "2.17.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ce2691df843ecc5d231c0b14ece2acc3efb62c0a398c7e1d875f3983ce020e3" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "semver" version = "1.0.28" @@ -2564,12 +2436,6 @@ version = "2.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7b3c8667cd96245cbb600b8dec5680a7319edd719c5aa2b5d23c6bff94f39765" -[[package]] -name = "subtle" -version = "2.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" - [[package]] name = "syn" version = "2.0.117" @@ -2618,7 +2484,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" dependencies = [ "bitflags", - "core-foundation 0.9.4", + "core-foundation", "system-configuration-sys", ] @@ -2771,16 +2637,6 @@ dependencies = [ "syn", ] -[[package]] -name = "tokio-rustls" -version = "0.26.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" -dependencies = [ - "rustls", - "tokio", -] - [[package]] name = "tokio-stream" version = "0.1.18" @@ -3011,12 +2867,6 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" -[[package]] -name = "untrusted" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" - [[package]] name = "url" version = "2.5.8" @@ -3609,12 +3459,6 @@ dependencies = [ "synstructure", ] -[[package]] -name = "zeroize" -version = "1.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" - [[package]] name = "zerotrie" version = "0.2.4" diff --git a/Cargo.toml b/Cargo.toml index 35baf7fd..55647ab6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ libdd-telemetry = { version = "5.0.0", default-features = false } libdd-common = { version = "4.1.0", default-features = false } libdd-tinybytes = { version = "1.1.1", default-features = false } libdd-library-config = { version = "2.0.0", default-features = false } -libdd-sampling = { version = "1.0.0", default-features = false } +libdd-sampling = { version = "2.1.0", default-features = false } opentelemetry_sdk = { version = "0.31.0", features = [ "trace", "metrics", diff --git a/datadog-opentelemetry/CHANGELOG.md b/datadog-opentelemetry/CHANGELOG.md index e63c92b0..90ed90d9 100644 --- a/datadog-opentelemetry/CHANGELOG.md +++ b/datadog-opentelemetry/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Support adjusting trace sampling from Remote Configuration — sampling rules (including tag-qualified rules) and `tracing_sampling_rate` — in https://github.com/DataDog/dd-trace-rs/pull/227 + ## 0.3.3 (May 06, 2026) - Fix config, use DD_AGENT_HOST and DD_TRACE_AGENT_PORT to derive agent url if it is an empty string in env in https://github.com/DataDog/dd-trace-rs/pull/208 diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index 2a42abcd..81a6183b 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -193,53 +193,6 @@ where Ok(Some(serde_json::Value::deserialize(deserializer)?)) } -/// Normalizes the `tags` field on each sampling rule from the RC wire shape -/// (list of `{key, value_glob}` objects) into the shape `libdd-sampling`'s -/// `SamplingRuleConfig` accepts (a `{key: value}` map). Map-shape tags are -/// left untouched. Rules without `tags`, or with `tags` of an unexpected -/// type, are left untouched (libdatadog's parse will reject if necessary). -/// -/// If any list entry is malformed (missing `key`/`value_glob`, or non-string -/// values), the rule's tags are left in their original (list) shape. This -/// fails closed: libdatadog will reject the list-shape parse, the RC update -/// is dropped, and the agent is informed via apply_state=3. We deliberately -/// do not drop bad entries silently — doing so could broaden a tag-constrained -/// rule into a less-constrained (or fully wildcard) rule. -fn normalize_rc_tags(rules: &mut [serde_json::Value]) { - for rule in rules { - let Some(obj) = rule.as_object_mut() else { - continue; - }; - let Some(tags) = obj.get("tags") else { - continue; - }; - let serde_json::Value::Array(entries) = tags else { - // Map-shape (or null/etc.) — leave it for libdatadog to handle. - continue; - }; - let mut map = serde_json::Map::with_capacity(entries.len()); - let mut all_ok = true; - for entry in entries { - let (Some(key), Some(value)) = ( - entry.get("key").and_then(|v| v.as_str()), - entry.get("value_glob").and_then(|v| v.as_str()), - ) else { - all_ok = false; - break; - }; - map.insert( - key.to_string(), - serde_json::Value::String(value.to_string()), - ); - } - if all_ok { - obj.insert("tags".to_string(), serde_json::Value::Object(map)); - } - // else: leave the original list-shape tags in place; libdatadog's - // parse will reject and the RC update is rejected as a whole. - } -} - /// Configuration payload for APM tracing /// Based on the apm-tracing.json schema from dd-go /// See: https://github.com/DataDog/dd-go/blob/prod/remote-config/apps/rc-schema-validation/schemas/apm-tracing.json @@ -921,9 +874,6 @@ impl ProductHandler for ApmTracingHandler { None => Vec::new(), }; - // Normalize RC list-shape tags into the map shape libdd-sampling accepts. - normalize_rc_tags(&mut rules); - // Multi-source precedence: // - If RC delivered explicit rules, env rules are replaced. // - If RC delivered only a rate, env rules survive and apply in front of the @@ -2446,8 +2396,10 @@ mod tests { #[test] fn test_handler_rc_rules_with_list_tags_applied() { - // Bug B regression guard: RC sends tags as list-of-objects; the handler - // must normalize to a map before passing to libdatadog. + // RC sends tags as a list-of-objects ([{key, value_glob}]); libdd-sampling + // (>=2.1.0) parses that wire shape natively, so no in-tracer normalization + // is needed. Regression guard: a list-shape tagged rule must apply with its + // tags preserved as a map. let config = build_config_for_handler(); let payload = br#"{ "id": "rc-list-tags", @@ -2476,60 +2428,6 @@ mod tests { ); } - #[test] - fn test_normalize_rc_tags_passes_through_map_shape() { - // Map-shape tags must be left untouched. - let mut rules: Vec = vec![ - serde_json::json!({"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}}), - ]; - normalize_rc_tags(&mut rules); - assert_eq!( - serde_json::Value::Array(rules), - serde_json::json!([ - {"sample_rate": 0.5, "service": "svc", "tags": {"env": "prod"}} - ]) - ); - } - - #[test] - fn test_normalize_rc_tags_converts_list_shape() { - let mut rules: Vec = vec![serde_json::json!({ - "sample_rate": 0.5, - "tags": [ - {"key": "env", "value_glob": "prod"}, - {"key": "region", "value_glob": "us-east-1"} - ] - })]; - normalize_rc_tags(&mut rules); - let tags = rules[0]["tags"] - .as_object() - .expect("tags should be an object after normalization"); - assert_eq!(tags.get("env").and_then(|v| v.as_str()), Some("prod")); - assert_eq!( - tags.get("region").and_then(|v| v.as_str()), - Some("us-east-1") - ); - } - - #[test] - fn test_normalize_rc_tags_leaves_malformed_list_untouched() { - // If any list entry is malformed (missing key/value_glob), the rule's - // tags are left in their original list shape — libdatadog's parse will - // then reject the update as a whole. We must not drop bad entries - // silently, which could broaden a tag-constrained rule. - let original = serde_json::json!({ - "sample_rate": 0.5, - "tags": [ - {"key": "env", "value_glob": "prod"}, - {"key": "region"} - ] - }); - let mut rules: Vec = vec![original.clone()]; - normalize_rc_tags(&mut rules); - // Tags remain in the original (rejected) list shape. - assert_eq!(rules[0], original); - } - #[test] fn test_handler_malformed_tags_rejects_update() { // Bug B fail-closed guard: a sampling rule with malformed list-shape diff --git a/instrumentation/Cargo.lock b/instrumentation/Cargo.lock index 75ef16f4..cd57b7fd 100644 --- a/instrumentation/Cargo.lock +++ b/instrumentation/Cargo.lock @@ -244,22 +244,6 @@ dependencies = [ "unicode-xid", ] -[[package]] -name = "core-foundation" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2a6cd9ae233e7f62ba4e9353e81a88df7fc8a5987b8d445b4d90c879bd156f6" -dependencies = [ - "core-foundation-sys", - "libc", -] - -[[package]] -name = "core-foundation-sys" -version = "0.8.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" - [[package]] name = "cpufeatures" version = "0.2.17" @@ -777,22 +761,6 @@ dependencies = [ "want", ] -[[package]] -name = "hyper-rustls" -version = "0.27.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" -dependencies = [ - "http", - "hyper", - "hyper-util", - "rustls", - "rustls-native-certs", - "tokio", - "tokio-rustls", - "tower-service", -] - [[package]] name = "hyper-timeout" version = "0.5.2" @@ -1113,9 +1081,9 @@ dependencies = [ [[package]] name = "libdd-common" -version = "4.1.0" +version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "933c5940905e9a3c8dd8c0439697b3fb327289cef7858370e6a5f440139f6314" +checksum = "4af81e6953fab2814792b8893a2cee9f16aad1b71cb3f8720b7256f28d6a2d8d" dependencies = [ "anyhow", "bytes", @@ -1129,19 +1097,15 @@ dependencies = [ "http-body", "http-body-util", "hyper", - "hyper-rustls", "hyper-util", "libc", "nix", "pin-project", "regex", - "rustls", - "rustls-native-certs", "serde", "static_assertions", "thiserror 1.0.69", "tokio", - "tokio-rustls", "tower-service", "windows-sys 0.52.0", ] @@ -1224,9 +1188,9 @@ dependencies = [ [[package]] name = "libdd-sampling" -version = "1.0.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ae8728d9d717eb1b648223c4c1da53f77627fecd6045027ccb758a72815b300" +checksum = "df229b55274db81b8a85a1b9ab14663af070358f39017ab68d26a3aada535db6" dependencies = [ "libdd-common", "lru", @@ -1480,12 +1444,6 @@ version = "11.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" -[[package]] -name = "openssl-probe" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e" - [[package]] name = "opentelemetry" version = "0.31.0" @@ -1869,20 +1827,6 @@ dependencies = [ "web-sys", ] -[[package]] -name = "ring" -version = "0.17.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" -dependencies = [ - "cc", - "cfg-if", - "getrandom 0.2.17", - "libc", - "untrusted", - "windows-sys 0.52.0", -] - [[package]] name = "rmp" version = "0.8.15" @@ -1943,52 +1887,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "rustls" -version = "0.23.40" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" -dependencies = [ - "once_cell", - "ring", - "rustls-pki-types", - "rustls-webpki", - "subtle", - "zeroize", -] - -[[package]] -name = "rustls-native-certs" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9980d917ebb0c0536119ba501e90834767bffc3d60641457fd84a1f3fd337923" -dependencies = [ - "openssl-probe", - "rustls-pki-types", - "schannel", - "security-framework", -] - -[[package]] -name = "rustls-pki-types" -version = "1.14.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30a7197ae7eb376e574fe940d068c30fe0462554a3ddbe4eca7838e049c937a9" -dependencies = [ - "zeroize", -] - -[[package]] -name = "rustls-webpki" -version = "0.103.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" -dependencies = [ - "ring", - "rustls-pki-types", - "untrusted", -] - [[package]] name = "rustversion" version = "1.0.22" @@ -2010,38 +1908,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "schannel" -version = "0.1.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91c1b7e4904c873ef0710c1f407dde2e6287de2bebc1bbbf7d430bb7cbffd939" -dependencies = [ - "windows-sys 0.61.2", -] - -[[package]] -name = "security-framework" -version = "3.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" -dependencies = [ - "bitflags", - "core-foundation", - "core-foundation-sys", - "libc", - "security-framework-sys", -] - -[[package]] -name = "security-framework-sys" -version = "2.17.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ce2691df843ecc5d231c0b14ece2acc3efb62c0a398c7e1d875f3983ce020e3" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "semver" version = "1.0.28" @@ -2198,12 +2064,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" -[[package]] -name = "subtle" -version = "2.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" - [[package]] name = "syn" version = "2.0.117" @@ -2340,16 +2200,6 @@ dependencies = [ "syn", ] -[[package]] -name = "tokio-rustls" -version = "0.26.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" -dependencies = [ - "rustls", - "tokio", -] - [[package]] name = "tokio-stream" version = "0.1.18" @@ -2566,12 +2416,6 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" -[[package]] -name = "untrusted" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" - [[package]] name = "url" version = "2.5.8" @@ -3091,12 +2935,6 @@ dependencies = [ "synstructure", ] -[[package]] -name = "zeroize" -version = "1.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" - [[package]] name = "zerotrie" version = "0.2.4" From f448c3b77fcc4dc9e8f1749a719b06b4e90be10d Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 09:30:14 -0400 Subject: [PATCH 19/27] fix(config): reject out-of-range DD_TRACE_SAMPLE_RATE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A finite but out-of-range DD_TRACE_SAMPLE_RATE (e.g. -0.5 or 1.5) was stored verbatim and installed as a catch-all sampling rule. libdd-sampling clamps the decision rate internally, so a negative value silently dropped all unmatched traffic and a value > 1.0 kept all of it, while still enabling the rate limiter — worse than ignoring the bad config. Adds `validate_trace_sample_rate`: only finite values in [0.0, 1.0] are kept; anything else is logged and treated as unset (no catch-all). Applied to both the env-var ingestion path and the programmatic `set_trace_sample_rate` setter, mirroring the range check already enforced on RC's `tracing_sampling_rate`. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/configuration/configuration.rs | 84 ++++++++++++++++++- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index f3184a16..f21c0541 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -149,6 +149,23 @@ fn parse_temporality(s: String) -> Option 1.0 would keep all of it). Mirrors the +/// range check applied to RC's `tracing_sampling_rate`. +fn validate_trace_sample_rate(rate: f64) -> Option { + if rate.is_finite() && (0.0..=1.0).contains(&rate) { + Some(rate) + } else { + crate::dd_warn!( + "DD_TRACE_SAMPLE_RATE must be in [0.0, 1.0], got {rate}; treating as unset" + ); + None + } +} + enum ConfigItemRef<'a, T> { Ref(&'a T), ArcRef(arc_swap::Guard>>), @@ -1190,8 +1207,10 @@ impl Config { // Use the initialized ConfigItem trace_sampling_rules: sampling_rules_item, - trace_sample_rate: cisu - .update_parsed_with_transform(default.trace_sample_rate, |r: f64| Some(r)), + trace_sample_rate: cisu.update_parsed_with_transform( + default.trace_sample_rate, + validate_trace_sample_rate, + ), trace_rate_limit: cisu.update_parsed(default.trace_rate_limit), enabled: cisu.update_parsed(default.enabled), @@ -2466,7 +2485,11 @@ impl ConfigBuilder { /// /// Env variable: `DD_TRACE_SAMPLE_RATE` pub fn set_trace_sample_rate(&mut self, rate: f64) -> &mut Self { - self.config.trace_sample_rate.set_code(Some(rate)); + // Only accept finite values in [0.0, 1.0]; an out-of-range value is + // logged and left unset rather than installed as a catch-all rule. + if let Some(rate) = validate_trace_sample_rate(rate) { + self.config.trace_sample_rate.set_code(Some(rate)); + } self } @@ -3843,4 +3866,59 @@ mod tests { let unset_rules = unset_config.effective_initial_rules(); assert!(unset_rules.is_empty(), "unset must not install a catch-all"); } + + #[test] + fn test_out_of_range_dd_trace_sample_rate_env_is_treated_as_unset() { + // Codex fix: a finite-but-out-of-range DD_TRACE_SAMPLE_RATE must be + // rejected at ingestion (treated as unset), not installed as a + // catch-all that libdd-sampling would clamp (negative -> drop all, + // >1.0 -> keep all) while still enabling the rate limiter. + for bad in ["1.5", "-0.5", "2", "inf", "-inf", "nan"] { + let mut sources = CompositeSource::new(); + sources.add_source(HashMapSource::from_iter( + [("DD_TRACE_SAMPLE_RATE", bad)], + ConfigSourceOrigin::EnvVar, + )); + let config = Config::builder_with_sources(&sources).build(); + assert_eq!( + config.trace_sample_rate(), + None, + "DD_TRACE_SAMPLE_RATE={bad} must be treated as unset" + ); + assert!( + config.effective_initial_rules().is_empty(), + "DD_TRACE_SAMPLE_RATE={bad} must not install a catch-all rule" + ); + } + } + + #[test] + fn test_in_range_boundary_dd_trace_sample_rate_env_accepted() { + // Boundaries 0.0 and 1.0 are valid and must be accepted. + for (val, expected) in [("0.0", 0.0), ("1.0", 1.0)] { + let mut sources = CompositeSource::new(); + sources.add_source(HashMapSource::from_iter( + [("DD_TRACE_SAMPLE_RATE", val)], + ConfigSourceOrigin::EnvVar, + )); + let config = Config::builder_with_sources(&sources).build(); + assert_eq!(config.trace_sample_rate(), Some(expected)); + } + } + + #[test] + fn test_out_of_range_set_trace_sample_rate_is_ignored() { + // The programmatic (code) setter must apply the same validation as the + // env path: an out-of-range rate is ignored, leaving the rate unset. + let mut builder = Config::builder(); + builder.set_trace_sample_rate(42.0); + let config = builder.build(); + assert_eq!(config.trace_sample_rate(), None); + assert!(config.effective_initial_rules().is_empty()); + + // A valid rate is still accepted. + let mut ok_builder = Config::builder(); + ok_builder.set_trace_sample_rate(0.3); + assert_eq!(ok_builder.build().trace_sample_rate(), Some(0.3)); + } } From 47fba734268719f81f821f536b4579101883f788 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 09:30:16 -0400 Subject: [PATCH 20/27] fix(rc): ignore APM_TRACING configs whose service_target mismatches ApmTracingConfig dropped the payload's `service_target`, so process_config applied any delivered config unconditionally. A stale or mistargeted RC payload could install another service's or env's sampling policy on this tracer. Deserializes `service_target` and, before mutating any sampler state, skips the config when a specific (non-`*`) service or env does not match this tracer's `config.service()` / `config.env()`. A wildcard or absent target still applies. The backend RC predicate already scopes delivery; this is defense-in-depth, matching dd-trace-py and dd-trace-go. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/configuration/remote_config.rs | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index 81a6183b..9702cacb 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -200,6 +200,24 @@ where struct ApmTracingConfig { id: String, lib_config: LibConfig, // lib_config is a required property + /// The service/env this config targets. The backend RC predicate already + /// filters delivery by target, so this is a defense-in-depth guard: a + /// stale or mistargeted payload must never install another service's or + /// env's sampling policy on this tracer. A `*` (or absent) component + /// applies regardless of the tracer's value. Mirrors dd-trace-py/go. + #[serde(default)] + service_target: Option, +} + +/// `service_target` block of an APM_TRACING RC payload (apm-tracing.json). Both +/// fields are optional here so a malformed/partial target degrades to "applies" +/// for the missing component rather than erroring the whole update. +#[derive(Debug, Clone, Deserialize)] +struct ServiceTarget { + #[serde(default)] + service: Option, + #[serde(default)] + env: Option, } #[derive(Debug, Clone, Deserialize)] @@ -801,6 +819,36 @@ impl ProductHandler for ApmTracingHandler { let tracing_config: ApmTracingConfig = serde_json::from_slice(config_json) .map_err(|e| anyhow::anyhow!("Failed to parse APM tracing config: {}", e))?; + // Defense-in-depth target check: only apply a config whose service_target + // matches this tracer's service/env. The backend predicate already scopes + // delivery, but a stale or mistargeted payload must never install another + // service's/env's policy. A `*` (or absent) component applies regardless. + // Mismatch => ignore (no sampler mutation), mirroring dd-trace-py. + if let Some(target) = &tracing_config.service_target { + let tracer_service = config.service(); + if let Some(svc) = target.service.as_deref() { + if svc != "*" && svc != &*tracer_service { + crate::dd_debug!( + "RemoteConfigClient: ignoring APM_TRACING config targeting service {:?} (tracer service is {:?})", + svc, + &*tracer_service + ); + return Ok(()); + } + } + if let Some(target_env) = target.env.as_deref() { + let tracer_env = config.env().unwrap_or(""); + if target_env != "*" && target_env != tracer_env { + crate::dd_debug!( + "RemoteConfigClient: ignoring APM_TRACING config targeting env {:?} (tracer env is {:?})", + target_env, + tracer_env + ); + return Ok(()); + } + } + } + let lib = tracing_config.lib_config; let any_field_present = @@ -2673,4 +2721,96 @@ mod tests { assert_eq!(rules[1].sample_rate, 0.70); assert!(rules[1].name.is_none()); } + + fn build_config_for_handler_with_target(service: &str, env: &str) -> Arc { + let mut builder = Config::builder(); + builder.set_service(service.to_string()); + builder.set_env(env.to_string()); + Arc::new(builder.build()) + } + + #[test] + fn test_handler_service_target_match_applies() { + // A config whose service_target matches the tracer's service/env applies. + let config = build_config_for_handler_with_target("svc-a", "env-a"); + let payload = br#"{ + "id": "rc-target-match", + "service_target": {"service": "svc-a", "env": "env-a"}, + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 1, + "matching service_target must apply" + ); + } + + #[test] + fn test_handler_service_target_service_mismatch_ignored() { + // Codex fix: a config targeting a DIFFERENT service must never mutate + // this tracer's sampler state. + let config = build_config_for_handler_with_target("svc-a", "env-a"); + let payload = br#"{ + "id": "rc-other-svc", + "service_target": {"service": "svc-b", "env": "env-a"}, + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 0, + "config for another service must be ignored" + ); + } + + #[test] + fn test_handler_service_target_env_mismatch_ignored() { + let config = build_config_for_handler_with_target("svc-a", "env-a"); + let payload = br#"{ + "id": "rc-other-env", + "service_target": {"service": "svc-a", "env": "env-b"}, + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 0, + "config for another env must be ignored" + ); + } + + #[test] + fn test_handler_service_target_wildcard_applies() { + // A wildcard ("*") target applies regardless of the tracer's service/env. + let config = build_config_for_handler_with_target("svc-a", "env-a"); + let payload = br#"{ + "id": "rc-wildcard", + "service_target": {"service": "*", "env": "*"}, + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 1, + "wildcard service_target must apply" + ); + } + + #[test] + fn test_handler_absent_service_target_applies() { + // Absent service_target (e.g. older payloads) must still apply — the + // target check only gates when a specific, non-wildcard target is set. + let config = build_config_for_handler_with_target("svc-a", "env-a"); + let payload = br#"{ + "id": "rc-no-target", + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 1, + "payload without service_target must apply" + ); + } } From bd077a5cb289094a32eee6aa5005e6bc5c4578a5 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 09:30:16 -0400 Subject: [PATCH 21/27] style: drop needless borrow in baggage test `Baggage::get` takes `impl Into`, so `&Key::new(key)` triggers clippy::needless_borrows_for_generic_args on rust >= 1.95. Pass the owned key directly. Co-Authored-By: Claude Opus 4.8 (1M context) --- datadog-opentelemetry/src/propagation/baggage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog-opentelemetry/src/propagation/baggage.rs b/datadog-opentelemetry/src/propagation/baggage.rs index 99a5d8fb..fa533c15 100644 --- a/datadog-opentelemetry/src/propagation/baggage.rs +++ b/datadog-opentelemetry/src/propagation/baggage.rs @@ -228,7 +228,7 @@ mod tests { assert_eq!(baggage.len(), expected_keys.len(), "header: {header:?}"); for key in expected_keys { assert!( - baggage.get(&Key::new(key)).is_some(), + baggage.get(Key::new(key)).is_some(), "missing key {key} in {header:?}" ); } From 7b9919f3988b458e2e3743daf668bdfc4f58b441 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 09:32:25 -0400 Subject: [PATCH 22/27] docs(changelog): note service_target + DD_TRACE_SAMPLE_RATE fixes Co-Authored-By: Claude Opus 4.8 (1M context) --- datadog-opentelemetry/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/datadog-opentelemetry/CHANGELOG.md b/datadog-opentelemetry/CHANGELOG.md index 90ed90d9..224dbbcd 100644 --- a/datadog-opentelemetry/CHANGELOG.md +++ b/datadog-opentelemetry/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased - Support adjusting trace sampling from Remote Configuration — sampling rules (including tag-qualified rules) and `tracing_sampling_rate` — in https://github.com/DataDog/dd-trace-rs/pull/227 +- Ignore Remote Configuration sampling payloads whose `service_target` does not match the tracer's service/env, so a mistargeted config cannot change this service's sampling in https://github.com/DataDog/dd-trace-rs/pull/227 +- Reject out-of-range `DD_TRACE_SAMPLE_RATE` (outside `[0.0, 1.0]`) instead of installing it as a sampling rule in https://github.com/DataDog/dd-trace-rs/pull/227 ## 0.3.3 (May 06, 2026) From 0992d2007ac9dd2ea21046a2b6bd5e5f4367fe5e Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 09:43:22 -0400 Subject: [PATCH 23/27] fix(rc): match service_target against extra services, case-insensitively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first service_target guard compared only the primary service with exact case. Because dd-trace-rs advertises extra_services to the backend, a config legitimately scoped to an advertised extra service — or one whose service/env differs only by case (the UI warns case can differ) — would be skipped and silently acknowledged, leaving sampling unchanged. Make the guard conservative so it only skips definitely-foreign configs: - service matches the primary service OR any advertised extra service (new read-only `ExtraServicesTracker::contains_service`, no queue drain) - service and env are compared case-insensitively Adds tests for the case-only difference and the extra-service cases. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/configuration/configuration.rs | 29 ++++++++++ .../src/core/configuration/remote_config.rs | 54 ++++++++++++++++--- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index f21c0541..ae0ec0a6 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -674,6 +674,25 @@ impl ExtraServicesTracker { } } + /// Returns true if `name` matches any tracked extra service + /// (case-insensitively), checking both the resolved set and the pending + /// queue. Read-only: does not drain the queue. + fn contains_service(&self, name: &str) -> bool { + if let Ok(set) = self.extra_services.lock() { + if set.iter().any(|s| s.eq_ignore_ascii_case(name)) { + return true; + } + } + if let Ok(queue) = self.extra_services_queue.lock() { + if let Some(ref q) = *queue { + if q.iter().any(|s| s.eq_ignore_ascii_case(name)) { + return true; + } + } + } + false + } + fn add_extra_services( &self, services: impl Iterator>, @@ -1773,6 +1792,16 @@ impl Config { self.extra_services_tracker.get_extra_services() } + /// Returns true if an RC `service_target.service` value applies to this + /// tracer: it matches the primary service or any advertised extra service, + /// compared case-insensitively. Used to guard which Remote Config sampling + /// payloads this tracer applies (a config that advertised extra service is + /// legitimately ours; service-name case can differ from the UI). + pub(crate) fn rc_service_target_matches(&self, target_service: &str) -> bool { + target_service.eq_ignore_ascii_case(&self.service()) + || self.extra_services_tracker.contains_service(target_service) + } + /// Check if remote configuration is enabled pub fn remote_config_enabled(&self) -> bool { *self.remote_config_enabled.value() diff --git a/datadog-opentelemetry/src/core/configuration/remote_config.rs b/datadog-opentelemetry/src/core/configuration/remote_config.rs index 9702cacb..923cbd41 100644 --- a/datadog-opentelemetry/src/core/configuration/remote_config.rs +++ b/datadog-opentelemetry/src/core/configuration/remote_config.rs @@ -825,20 +825,24 @@ impl ProductHandler for ApmTracingHandler { // service's/env's policy. A `*` (or absent) component applies regardless. // Mismatch => ignore (no sampler mutation), mirroring dd-trace-py. if let Some(target) = &tracing_config.service_target { - let tracer_service = config.service(); + // Only skip a config whose target is specific (non-`*`) AND does not + // apply to this tracer. Service matches the primary or any advertised + // extra service; both service and env are compared case-insensitively + // (the UI warns service-name case can differ). Being lenient here is + // deliberate: the guard must skip only configs that are definitely + // for another service/env, never a valid one for us. if let Some(svc) = target.service.as_deref() { - if svc != "*" && svc != &*tracer_service { + if svc != "*" && !config.rc_service_target_matches(svc) { crate::dd_debug!( - "RemoteConfigClient: ignoring APM_TRACING config targeting service {:?} (tracer service is {:?})", - svc, - &*tracer_service + "RemoteConfigClient: ignoring APM_TRACING config targeting service {:?} (not this tracer's service or extra services)", + svc ); return Ok(()); } } if let Some(target_env) = target.env.as_deref() { let tracer_env = config.env().unwrap_or(""); - if target_env != "*" && target_env != tracer_env { + if target_env != "*" && !target_env.eq_ignore_ascii_case(tracer_env) { crate::dd_debug!( "RemoteConfigClient: ignoring APM_TRACING config targeting env {:?} (tracer env is {:?})", target_env, @@ -2813,4 +2817,42 @@ mod tests { "payload without service_target must apply" ); } + + #[test] + fn test_handler_service_target_case_insensitive_applies() { + // service/env case can differ from what the tracer reports (the UI warns + // about this); a case-only difference must still apply, not be skipped. + let config = build_config_for_handler_with_target("svc-a", "env-a"); + let payload = br#"{ + "id": "rc-case", + "service_target": {"service": "SVC-A", "env": "ENV-A"}, + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 1, + "case-only service/env difference must still apply" + ); + } + + #[test] + fn test_handler_service_target_extra_service_applies() { + // A config targeting an advertised extra service is legitimately ours and + // must apply (the tracer reports extra_services to the backend, which can + // deliver a config scoped to one of them). + let config = build_config_for_handler_with_target("svc-a", "env-a"); + config.add_extra_services(["svc-extra"].into_iter()); + let payload = br#"{ + "id": "rc-extra", + "service_target": {"service": "svc-extra", "env": "*"}, + "lib_config": {"tracing_sampling_rate": 0.5} + }"#; + ApmTracingHandler.process_config(payload, &config).unwrap(); + assert_eq!( + config.trace_sampling_rules().len(), + 1, + "config for an advertised extra service must apply" + ); + } } From 35bba4b218485e7ae037d25ca8982cc7453d4732 Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 09:50:56 -0400 Subject: [PATCH 24/27] chore: regenerate LICENSE-3rdparty.csv after libdd-sampling bump libdd-sampling 2.1.0 / libdd-common 4.2.0 dropped the rustls TLS stack from the dependency tree, so those entries (hyper-rustls, ring, rustls*, schannel, security-framework*, tokio-rustls, untrusted, zeroize, etc.) are no longer present. Regenerated with dd-rust-license-tool 1.0.6 (the version the Lint workflow uses) so the "Valid LICENSE-3rdparty.csv" check passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- LICENSE-3rdparty.csv | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index ebda13cd..ea77aeb9 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -87,7 +87,6 @@ httparse,https://github.com/seanmonstar/httparse,MIT OR Apache-2.0,Sean McArthur httpdate,https://github.com/pyfisch/httpdate,MIT OR Apache-2.0,Pyfisch httpmock,https://github.com/httpmock/httpmock,MIT,Alexander Liesenfeld hyper,https://github.com/hyperium/hyper,MIT,Sean McArthur -hyper-rustls,https://github.com/rustls/hyper-rustls,Apache-2.0 OR ISC OR MIT,The hyper-rustls Authors hyper-timeout,https://github.com/hjr3/hyper-timeout,MIT OR Apache-2.0,Herman J. Radtke III hyper-util,https://github.com/hyperium/hyper-util,MIT,Sean McArthur icu_collections,https://github.com/unicode-org/icu4x,Unicode-3.0,The ICU4X Project Developers @@ -143,7 +142,6 @@ nu-ansi-term,https://github.com/nushell/nu-ansi-term,MIT,"ogham@bsago.me, Ryan S num-traits,https://github.com/rust-num/num-traits,MIT OR Apache-2.0,The Rust Project Developers once_cell,https://github.com/matklad/once_cell,MIT OR Apache-2.0,Aleksey Kladov oorandom,https://hg.sr.ht/~icefox/oorandom,MIT,Simon Heath -openssl-probe,https://github.com/alexcrichton/openssl-probe,MIT OR Apache-2.0,Alex Crichton opentelemetry,https://github.com/open-telemetry/opentelemetry-rust/tree/main/opentelemetry,Apache-2.0,The opentelemetry Authors opentelemetry-appender-tracing,https://github.com/open-telemetry/opentelemetry-rust/tree/main/opentelemetry-appender-tracing,Apache-2.0,The opentelemetry-appender-tracing Authors opentelemetry-http,https://github.com/open-telemetry/opentelemetry-rust/tree/main/opentelemetry-http,Apache-2.0,The opentelemetry-http Authors @@ -185,24 +183,16 @@ regex,https://github.com/rust-lang/regex,MIT OR Apache-2.0,"The Rust Project Dev regex-automata,https://github.com/rust-lang/regex,MIT OR Apache-2.0,"The Rust Project Developers, Andrew Gallant " regex-syntax,https://github.com/rust-lang/regex,MIT OR Apache-2.0,"The Rust Project Developers, Andrew Gallant " reqwest,https://github.com/seanmonstar/reqwest,MIT OR Apache-2.0,Sean McArthur -ring,https://github.com/briansmith/ring,Apache-2.0 AND ISC,The ring Authors rmp,https://github.com/3Hren/msgpack-rust,MIT,Evgeny Safronov rmp-serde,https://github.com/3Hren/msgpack-rust,MIT,Evgeny Safronov rmpv,https://github.com/3Hren/msgpack-rust,MIT,Evgeny Safronov rustc_version,https://github.com/djc/rustc-version-rs,MIT OR Apache-2.0,The rustc_version Authors rustc_version_runtime,https://github.com/seppo0010/rustc-version-runtime-rs,MIT,Sebastian Waisbrot rustix,https://github.com/bytecodealliance/rustix,Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT,"Dan Gohman , Jakub Konka " -rustls,https://github.com/rustls/rustls,Apache-2.0 OR ISC OR MIT,The rustls Authors -rustls-native-certs,https://github.com/rustls/rustls-native-certs,Apache-2.0 OR ISC OR MIT,The rustls-native-certs Authors -rustls-pki-types,https://github.com/rustls/pki-types,MIT OR Apache-2.0,The rustls-pki-types Authors -rustls-webpki,https://github.com/rustls/webpki,ISC,The rustls-webpki Authors rustversion,https://github.com/dtolnay/rustversion,MIT OR Apache-2.0,David Tolnay ryu,https://github.com/dtolnay/ryu,Apache-2.0 OR BSL-1.0,David Tolnay same-file,https://github.com/BurntSushi/same-file,Unlicense OR MIT,Andrew Gallant -schannel,https://github.com/steffengy/schannel-rs,MIT,"Steven Fackler , Steffen Butzer " scopeguard,https://github.com/bluss/scopeguard,MIT OR Apache-2.0,bluss -security-framework,https://github.com/kornelski/rust-security-framework,MIT OR Apache-2.0,"Steven Fackler , Kornel " -security-framework-sys,https://github.com/kornelski/rust-security-framework,MIT OR Apache-2.0,"Steven Fackler , Kornel " semver,https://github.com/dtolnay/semver,MIT OR Apache-2.0,David Tolnay serde,https://github.com/serde-rs/serde,MIT OR Apache-2.0,"Erick Tryzelaar , David Tolnay " serde_bytes,https://github.com/serde-rs/bytes,MIT OR Apache-2.0,David Tolnay @@ -224,7 +214,6 @@ socket2,https://github.com/rust-lang/socket2,MIT OR Apache-2.0,"Alex Crichton static_assertions,https://github.com/nvzqz/static-assertions-rs,MIT OR Apache-2.0,Nikolai Vazquez stringmetrics,https://github.com/pluots/stringmetrics,Apache-2.0,Trevor Gross -subtle,https://github.com/dalek-cryptography/subtle,BSD-3-Clause,"Isis Lovecruft , Henry de Valence " syn,https://github.com/dtolnay/syn,MIT OR Apache-2.0,David Tolnay sync_wrapper,https://github.com/Actyx/sync_wrapper,Apache-2.0,Actyx AG synstructure,https://github.com/mystor/synstructure,MIT,Nika Layzell @@ -241,7 +230,6 @@ tinystr,https://github.com/unicode-org/icu4x,Unicode-3.0,The ICU4X Project Devel tinytemplate,https://github.com/bheisler/TinyTemplate,Apache-2.0 OR MIT,Brook Heisler tokio,https://github.com/tokio-rs/tokio,MIT,Tokio Contributors tokio-macros,https://github.com/tokio-rs/tokio,MIT,Tokio Contributors -tokio-rustls,https://github.com/rustls/tokio-rustls,MIT OR Apache-2.0,The tokio-rustls Authors tokio-stream,https://github.com/tokio-rs/tokio,MIT,Tokio Contributors tokio-util,https://github.com/tokio-rs/tokio,MIT,Tokio Contributors tonic,https://github.com/hyperium/tonic,MIT,Lucio Franco @@ -262,7 +250,6 @@ unicode-ident,https://github.com/dtolnay/unicode-ident,(MIT OR Apache-2.0) AND U unicode-width,https://github.com/unicode-rs/unicode-width,MIT OR Apache-2.0,"kwantam , Manish Goregaokar " unicode-xid,https://github.com/unicode-rs/unicode-xid,MIT OR Apache-2.0,"erick.tryzelaar , kwantam , Manish Goregaokar " unsafe-libyaml,https://github.com/dtolnay/unsafe-libyaml,MIT,David Tolnay -untrusted,https://github.com/briansmith/untrusted,ISC,Brian Smith url,https://github.com/servo/rust-url,MIT OR Apache-2.0,The rust-url developers urlencoding,https://github.com/kornelski/rust_urlencoding,MIT,"Kornel , Bertram Truong " utf8_iter,https://github.com/hsivonen/utf8_iter,Apache-2.0 OR MIT,Henri Sivonen @@ -318,7 +305,6 @@ zerocopy,https://github.com/google/zerocopy,BSD-2-Clause OR Apache-2.0 OR MIT,"J zerocopy-derive,https://github.com/google/zerocopy,BSD-2-Clause OR Apache-2.0 OR MIT,"Joshua Liebow-Feeser , Jack Wrenn " zerofrom,https://github.com/unicode-org/icu4x,Unicode-3.0,Manish Goregaokar zerofrom-derive,https://github.com/unicode-org/icu4x,Unicode-3.0,Manish Goregaokar -zeroize,https://github.com/RustCrypto/utils,Apache-2.0 OR MIT,The RustCrypto Project Developers zerotrie,https://github.com/unicode-org/icu4x,Unicode-3.0,The ICU4X Project Developers zerovec,https://github.com/unicode-org/icu4x,Unicode-3.0,The ICU4X Project Developers zerovec-derive,https://github.com/unicode-org/icu4x,Unicode-3.0,Manish Goregaokar From dbd0c7ab2f9ae7f0a21cbd0a40252c7dc30cc56a Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 10:42:43 -0400 Subject: [PATCH 25/27] chore: drop premature CHANGELOG entries This repo writes CHANGELOG sections at release time (in the version-bump commit), not per feature PR; there is no running "Unreleased" section. Remove the entries added earlier in this PR so they're captured in the repo's usual style when the next release is cut. Co-Authored-By: Claude Opus 4.8 (1M context) --- datadog-opentelemetry/CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/datadog-opentelemetry/CHANGELOG.md b/datadog-opentelemetry/CHANGELOG.md index 224dbbcd..e63c92b0 100644 --- a/datadog-opentelemetry/CHANGELOG.md +++ b/datadog-opentelemetry/CHANGELOG.md @@ -1,11 +1,5 @@ # Changelog -## Unreleased - -- Support adjusting trace sampling from Remote Configuration — sampling rules (including tag-qualified rules) and `tracing_sampling_rate` — in https://github.com/DataDog/dd-trace-rs/pull/227 -- Ignore Remote Configuration sampling payloads whose `service_target` does not match the tracer's service/env, so a mistargeted config cannot change this service's sampling in https://github.com/DataDog/dd-trace-rs/pull/227 -- Reject out-of-range `DD_TRACE_SAMPLE_RATE` (outside `[0.0, 1.0]`) instead of installing it as a sampling rule in https://github.com/DataDog/dd-trace-rs/pull/227 - ## 0.3.3 (May 06, 2026) - Fix config, use DD_AGENT_HOST and DD_TRACE_AGENT_PORT to derive agent url if it is an empty string in env in https://github.com/DataDog/dd-trace-rs/pull/208 From 9fce9180c568e0975f920bf1520e7d99c70ed44b Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 11:21:26 -0400 Subject: [PATCH 26/27] Update Cargo.toml Co-authored-by: Igor Unanua --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 55647ab6..83e1c2ce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ libdd-data-pipeline = { version = "5.0.0", features = [ libdd-trace-utils = { version = "5.0.0", default-features = false } libdd-capabilities-impl = { version = "2.0.0", default-features = false } libdd-telemetry = { version = "5.0.0", default-features = false } -libdd-common = { version = "4.1.0", default-features = false } +libdd-common = { version = "4.2.0", default-features = false } libdd-tinybytes = { version = "1.1.1", default-features = false } libdd-library-config = { version = "2.0.0", default-features = false } libdd-sampling = { version = "2.1.0", default-features = false } From 699192fef36f19b50f7f888aa7be665258dd029b Mon Sep 17 00:00:00 2001 From: Brian Marks Date: Mon, 1 Jun 2026 11:23:20 -0400 Subject: [PATCH 27/27] docs: fix stale effective_initial_rules catch-all comment Addresses review feedback (@iunanua): the comment claimed the catch-all is skipped for "the default of 1.0", but DD_TRACE_SAMPLE_RATE's default is now None (unset), and an explicit 1.0 *does* install the catch-all so DD_TRACE_RATE_LIMIT applies (see test_explicit_dd_trace_sample_rate_one_ installs_catch_all). Rewrite the comment to match: append only when the rate is explicitly set; unset => no catch-all (libdatadog's 100% fallback). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/configuration/configuration.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/datadog-opentelemetry/src/core/configuration/configuration.rs b/datadog-opentelemetry/src/core/configuration/configuration.rs index ae0ec0a6..08a98c46 100644 --- a/datadog-opentelemetry/src/core/configuration/configuration.rs +++ b/datadog-opentelemetry/src/core/configuration/configuration.rs @@ -1712,11 +1712,13 @@ impl Config { /// active Remote Config override: locally-configured rules followed by an /// implicit catch-all that applies `DD_TRACE_SAMPLE_RATE`. /// - /// The catch-all is appended only when the env rate is finite and not the - /// default of 1.0 — a 1.0 catch-all would match every unmatched span at - /// 100% sampling, which is identical to having no catch-all (libdatadog's - /// fallback path samples at 1.0 when no rule matches), so we omit it to - /// avoid noisy `_dd.p.dm` tags on the wire. + /// The catch-all is appended only when `DD_TRACE_SAMPLE_RATE` is explicitly + /// set. Its default is unset (`None`), in which case nothing is appended and + /// libdatadog's no-rule fallback samples unmatched spans at 100%. An explicit + /// value — including `1.0` — does install the catch-all, so `DD_TRACE_RATE_LIMIT` + /// applies to otherwise-unmatched spans. The rate is already validated to a + /// finite value in `[0.0, 1.0]` at ingestion; the `is_finite` guard below is + /// belt-and-suspenders. pub(crate) fn effective_initial_rules(&self) -> Vec { let mut rules: Vec = self .local_trace_sampling_rules()