From eca2923a9b959674dffb339422258227fa23dea5 Mon Sep 17 00:00:00 2001 From: iunanua Date: Wed, 10 Jun 2026 10:27:11 +0200 Subject: [PATCH 1/4] Fix DynamicConfig property name --- libdd-remote-config/src/config/dynamic.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libdd-remote-config/src/config/dynamic.rs b/libdd-remote-config/src/config/dynamic.rs index c0fa77940c..f845736120 100644 --- a/libdd-remote-config/src/config/dynamic.rs +++ b/libdd-remote-config/src/config/dynamic.rs @@ -80,7 +80,7 @@ pub struct TracingSamplingRule { #[cfg_attr(feature = "test", derive(Default, Serialize))] pub struct DynamicConfig { pub(crate) tracing_header_tags: Option>, - pub(crate) tracing_sample_rate: Option, + pub(crate) tracing_sampling_rate: Option, pub(crate) log_injection_enabled: Option, pub(crate) tracing_tags: Option>, pub(crate) tracing_enabled: Option, @@ -98,8 +98,8 @@ impl From for Vec { tags.into_iter().map(|t| (t.header, t.tag_name)).collect(), )) } - if let Some(sample_rate) = value.tracing_sample_rate { - vec.push(Configs::TracingSampleRate(sample_rate)); + if let Some(sample_rate) = value.tracing_sampling_rate { + vec.push(Configs::TracingSamplingRate(sample_rate)); } if let Some(log_injection) = value.log_injection_enabled { vec.push(Configs::LogInjectionEnabled(log_injection)); @@ -129,7 +129,7 @@ impl From for Vec { #[derive(Clone)] pub enum Configs { TracingHeaderTags(HashMap), - TracingSampleRate(f64), + TracingSamplingRate(f64), LogInjectionEnabled(bool), TracingTags(Vec), // "key:val" format TracingEnabled(bool), From 1e238b0568946797d808741570330548092c1227 Mon Sep 17 00:00:00 2001 From: iunanua Date: Wed, 10 Jun 2026 16:49:25 +0200 Subject: [PATCH 2/4] Preserve absent vs explicit-null for DynamicConfig fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps every DynamicConfig field in a new `Patch` newtype that encodes JSON Merge Patch (RFC 7396) semantics: - Patch(None) — field absent on the wire (no change intended) - Patch(Some(None)) — field present as JSON null (clear prior override) - Patch(Some(Some(v))) — field present with value (set to v) `Configs` variants now carry `Option` (Some = set, None = clear), and `From for Vec` only emits a variant for fields that were actually delivered. This fixes the prior collapse where any APM_TRACING payload missing tracing_sampling_rate looked the same to consumers as `tracing_sampling_rate: null`, so a payload that updated unrelated fields (e.g. tracing_tags) would wipe an active remote sample-rate override. The struct-level `#[serde(default)]` keeps the new shape free of per-field `deserialize_with` boilerplate. Test-feature `Serialize` is a small hand-written impl that skips absent fields to keep the `dummy_dynamic_config` round-trip intact for the sidecar tests. Co-Authored-By: Claude Opus 4.7 --- datadog-sidecar/src/shm_remote_config.rs | 4 +- libdd-remote-config/src/config/dynamic.rs | 238 +++++++++++++++++++--- 2 files changed, 207 insertions(+), 35 deletions(-) diff --git a/datadog-sidecar/src/shm_remote_config.rs b/datadog-sidecar/src/shm_remote_config.rs index f0c3022d8f..118a76d65c 100644 --- a/datadog-sidecar/src/shm_remote_config.rs +++ b/datadog-sidecar/src/shm_remote_config.rs @@ -305,7 +305,7 @@ impl MultiTargetHandlers for ConfigFileStora let configs: Vec = config.lib_config.into(); for config in configs { if let Configs::DynamicInstrumentationEnabled(enabled) = config { - writer.dynamic_instrumentation = Some(enabled); + writer.dynamic_instrumentation = enabled; } } } @@ -923,7 +923,7 @@ mod tests { if let Some(cfg) = parsed.downcast::() { assert!(matches!( >::from(cfg.lib_config.clone())[0], - Configs::TracingEnabled(true) + Configs::TracingEnabled(Some(true)) )); } else { unreachable!(); diff --git a/libdd-remote-config/src/config/dynamic.rs b/libdd-remote-config/src/config/dynamic.rs index f845736120..7ff57f2934 100644 --- a/libdd-remote-config/src/config/dynamic.rs +++ b/libdd-remote-config/src/config/dynamic.rs @@ -76,67 +76,171 @@ pub struct TracingSamplingRule { pub sample_rate: f64, } -#[derive(Debug, Clone, Deserialize)] -#[cfg_attr(feature = "test", derive(Default, Serialize))] +/// Three-state field carrying JSON Merge Patch (RFC 7396) semantics: +/// +/// - `Patch(None)` — field was absent on the wire (no change intended). +/// - `Patch(Some(None))` — field was present as JSON `null` (clear any prior remote override). +/// - `Patch(Some(Some(v)))` — field was present with a value (set to `v`). +/// +/// Used by [`DynamicConfig`] for every payload field so the absent / clear / +/// set distinction survives all the way from the wire to consumers reading +/// [`Configs`]. With struct-level `#[serde(default)]` on [`DynamicConfig`], +/// missing fields fall back to [`Patch::default`] (= `Patch(None)`); present +/// fields go through this `Deserialize` impl which resolves null-vs-value via +/// the inner `Option::::deserialize` and wraps the result in `Some` to +/// mark "delivered". +#[derive(Debug, Clone, PartialEq)] +pub struct Patch(pub Option>); + +impl Patch { + /// `true` when the field was not delivered on the wire. + pub fn is_absent(&self) -> bool { + self.0.is_none() + } +} + +impl Default for Patch { + fn default() -> Self { + Patch(None) + } +} + +impl<'de, T: Deserialize<'de>> Deserialize<'de> for Patch { + fn deserialize>(deserializer: D) -> Result { + // `Deserialize::deserialize` only runs when the field WAS present on + // the wire (absent fields go through `Default` via the struct-level + // `#[serde(default)]`). Resolving null-vs-value via the inner + // `Option::::deserialize` lets us encode all three states. + Option::::deserialize(deserializer).map(|inner| Patch(Some(inner))) + } +} + +#[cfg(feature = "test")] +impl Serialize for Patch { + fn serialize(&self, serializer: S) -> Result { + // The struct-level `Serialize` impl for `DynamicConfig` skips absent + // fields before reaching this method, so we only see delivered values: + // `Some(None)` → JSON `null`, `Some(Some(v))` → `v`. + match &self.0 { + None => serializer.serialize_none(), + Some(inner) => inner.serialize(serializer), + } + } +} + +/// Dynamic configuration delivered by the APM_TRACING product. +/// +/// Every field is a [`Patch`] so the absent / clear / set distinction +/// survives the wire. Struct-level `#[serde(default)]` lets missing fields +/// fall back to `Patch::default()` without per-field `deserialize_with` +/// boilerplate. The matching `Serialize` impl (under `feature = "test"`) is +/// hand-written below so absent fields are omitted from the output without +/// needing per-field `skip_serializing_if` annotations. See [`Configs`] for +/// how the three states surface to consumers. +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(default)] pub struct DynamicConfig { - pub(crate) tracing_header_tags: Option>, - pub(crate) tracing_sampling_rate: Option, - pub(crate) log_injection_enabled: Option, - pub(crate) tracing_tags: Option>, - pub(crate) tracing_enabled: Option, - pub(crate) tracing_sampling_rules: Option>, - pub(crate) dynamic_instrumentation_enabled: Option, - pub(crate) exception_replay_enabled: Option, - pub(crate) code_origin_enabled: Option, + pub(crate) tracing_header_tags: Patch>, + pub(crate) tracing_sampling_rate: Patch, + pub(crate) log_injection_enabled: Patch, + pub(crate) tracing_tags: Patch>, + pub(crate) tracing_enabled: Patch, + pub(crate) tracing_sampling_rules: Patch>, + pub(crate) dynamic_instrumentation_enabled: Patch, + pub(crate) exception_replay_enabled: Patch, + pub(crate) code_origin_enabled: Patch, +} + +#[cfg(feature = "test")] +impl Serialize for DynamicConfig { + fn serialize(&self, serializer: S) -> Result { + // Manual impl so absent fields (`Patch(None)`) are omitted from the + // output entirely rather than serializing as JSON `null` — that + // collapse would make round-trips re-deserialize absent fields as + // explicit clears, defeating the three-state semantics. + // + // `serialize_struct`'s `len` argument is only used by formats that + // pre-allocate (bincode, etc.); JSON ignores it. Pass the maximum. + use serde::ser::SerializeStruct; + macro_rules! field { + ($s:expr, $name:ident) => { + if !self.$name.is_absent() { + $s.serialize_field(stringify!($name), &self.$name)?; + } + }; + } + let mut s = serializer.serialize_struct("DynamicConfig", 9)?; + field!(s, tracing_header_tags); + field!(s, tracing_sampling_rate); + field!(s, log_injection_enabled); + field!(s, tracing_tags); + field!(s, tracing_enabled); + field!(s, tracing_sampling_rules); + field!(s, dynamic_instrumentation_enabled); + field!(s, exception_replay_enabled); + field!(s, code_origin_enabled); + s.end() + } } impl From for Vec { fn from(value: DynamicConfig) -> Self { + // A `Configs` variant is emitted whenever the field was present on + // the wire — including the explicit-`null` case, where the inner + // `Option` is `None` to signal "clear". Absent fields produce no + // variant at all, so callers can distinguish all three states. let mut vec = vec![]; - if let Some(tags) = value.tracing_header_tags { - vec.push(Configs::TracingHeaderTags( - tags.into_iter().map(|t| (t.header, t.tag_name)).collect(), - )) + if let Patch(Some(tags)) = value.tracing_header_tags { + vec.push(Configs::TracingHeaderTags(tags.map(|tags| { + tags.into_iter().map(|t| (t.header, t.tag_name)).collect() + }))); } - if let Some(sample_rate) = value.tracing_sampling_rate { + if let Patch(Some(sample_rate)) = value.tracing_sampling_rate { vec.push(Configs::TracingSamplingRate(sample_rate)); } - if let Some(log_injection) = value.log_injection_enabled { + if let Patch(Some(log_injection)) = value.log_injection_enabled { vec.push(Configs::LogInjectionEnabled(log_injection)); } - if let Some(tags) = value.tracing_tags { + if let Patch(Some(tags)) = value.tracing_tags { vec.push(Configs::TracingTags(tags)); } - if let Some(enabled) = value.tracing_enabled { + if let Patch(Some(enabled)) = value.tracing_enabled { vec.push(Configs::TracingEnabled(enabled)); } - if let Some(sampling_rules) = value.tracing_sampling_rules { + if let Patch(Some(sampling_rules)) = value.tracing_sampling_rules { vec.push(Configs::TracingSamplingRules(sampling_rules)); } - if let Some(enabled) = value.dynamic_instrumentation_enabled { + if let Patch(Some(enabled)) = value.dynamic_instrumentation_enabled { vec.push(Configs::DynamicInstrumentationEnabled(enabled)); } - if let Some(enabled) = value.exception_replay_enabled { + if let Patch(Some(enabled)) = value.exception_replay_enabled { vec.push(Configs::ExceptionReplayEnabled(enabled)); } - if let Some(enabled) = value.code_origin_enabled { + if let Patch(Some(enabled)) = value.code_origin_enabled { vec.push(Configs::CodeOriginEnabled(enabled)); } vec } } +/// A single APM_TRACING field that the agent has expressed an opinion about. +/// +/// Each variant carries `Option`: `Some(v)` means "set this field to `v`", +/// `None` means "clear any prior remote override and fall back to local +/// config". A field that was absent on the wire produces no variant at all, +/// so callers can distinguish all three states (absent / clear / set) by +/// matching on presence-in-the-`Vec` and on the inner `Option`. #[derive(Clone)] pub enum Configs { - TracingHeaderTags(HashMap), - TracingSamplingRate(f64), - LogInjectionEnabled(bool), - TracingTags(Vec), // "key:val" format - TracingEnabled(bool), - TracingSamplingRules(Vec), - DynamicInstrumentationEnabled(bool), - ExceptionReplayEnabled(bool), - CodeOriginEnabled(bool), + TracingHeaderTags(Option>), + TracingSamplingRate(Option), + LogInjectionEnabled(Option), + TracingTags(Option>), // inner `Vec` items are "key:val" + TracingEnabled(Option), + TracingSamplingRules(Option>), + DynamicInstrumentationEnabled(Option), + ExceptionReplayEnabled(Option), + CodeOriginEnabled(Option), } pub fn parse_json(data: &[u8]) -> serde_json::error::Result { @@ -152,9 +256,77 @@ pub mod tests { action: "".to_string(), service_target: None, lib_config: DynamicConfig { - tracing_enabled: Some(enabled), + tracing_enabled: Patch(Some(Some(enabled))), ..DynamicConfig::default() }, } } + + #[test] + fn parses_absent_field_as_patch_none() { + let cfg: DynamicConfigFile = parse_json(br#"{"action": "", "lib_config": {}}"#).unwrap(); + assert!(cfg.lib_config.tracing_sampling_rate.is_absent()); + assert!(>::from(cfg.lib_config).is_empty()); + } + + #[test] + fn parses_explicit_null_as_clear_intent() { + let cfg: DynamicConfigFile = + parse_json(br#"{"action": "", "lib_config": {"tracing_sampling_rate": null}}"#) + .unwrap(); + assert_eq!(cfg.lib_config.tracing_sampling_rate, Patch(Some(None))); + let configs: Vec = cfg.lib_config.into(); + assert_eq!(configs.len(), 1); + assert!(matches!(configs[0], Configs::TracingSamplingRate(None))); + } + + #[test] + fn parses_concrete_value_as_set_intent() { + let cfg: DynamicConfigFile = + parse_json(br#"{"action": "", "lib_config": {"tracing_sampling_rate": 0.25}}"#) + .unwrap(); + assert_eq!( + cfg.lib_config.tracing_sampling_rate, + Patch(Some(Some(0.25))) + ); + let configs: Vec = cfg.lib_config.into(); + assert_eq!(configs.len(), 1); + assert!(matches!(configs[0], Configs::TracingSamplingRate(Some(r)) if r == 0.25)); + } + + #[test] + fn unrelated_field_present_does_not_emit_sampling_variants() { + // Regression guard: a payload that updates only `tracing_tags` must + // not surface as a clear for `tracing_sampling_rate` / + // `tracing_sampling_rules` (and vice versa). Each field is + // independently absent / null / set. + let cfg: DynamicConfigFile = + parse_json(br#"{"action": "", "lib_config": {"tracing_tags": ["foo:bar"]}}"#).unwrap(); + let configs: Vec = cfg.lib_config.into(); + assert_eq!(configs.len(), 1); + assert!(matches!(configs[0], Configs::TracingTags(Some(_)))); + assert!(!configs + .iter() + .any(|c| matches!(c, Configs::TracingSamplingRate(_)))); + assert!(!configs + .iter() + .any(|c| matches!(c, Configs::TracingSamplingRules(_)))); + } + + #[test] + fn round_trip_via_test_serialize_preserves_absent_vs_null() { + // The hand-written `Serialize` impl for `DynamicConfig` must skip + // absent fields entirely so the round-trip doesn't collapse + // `Patch(None)` → JSON `null` → `Patch(Some(None))`. + let mut cfg = dummy_dynamic_config(true); + cfg.lib_config.tracing_sampling_rate = Patch(Some(None)); + let json = serde_json::to_string(&cfg).unwrap(); + let parsed: DynamicConfigFile = parse_json(json.as_bytes()).unwrap(); + // absent fields stay absent + assert!(parsed.lib_config.tracing_sampling_rules.is_absent()); + // explicit-null stays explicit-null + assert_eq!(parsed.lib_config.tracing_sampling_rate, Patch(Some(None))); + // set value stays set + assert_eq!(parsed.lib_config.tracing_enabled, Patch(Some(Some(true)))); + } } From 51a7d2a881b05ba12edf35809059275ce97cc1cc Mon Sep 17 00:00:00 2001 From: iunanua Date: Mon, 15 Jun 2026 15:30:04 +0200 Subject: [PATCH 3/4] Treat absent and null as no-op for DynamicConfig fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier `Patch` design assumed JSON Merge Patch semantics for APM_TRACING — three states (absent / null / value), where null carried an explicit "clear this field" signal. The dd-trace-go reference implementation (ddtrace/tracer/remote_config.go) settles otherwise: it uses `*float64` / `*[]rcSamplingRule`, which collapse absent and null into the same `nil`, and the `mergeConfigsByPriority` function only writes when a value is present. Clearing a remote override happens at the file level: when the agent stops delivering a file, the merge re-runs without it and `HandleRC(nil)` reverts to local config. There is no in-file clear signal. Match that contract: - `DynamicConfig` fields are plain `Option` again. `None` covers both "field absent" and "field present as null"; the distinction is not meaningful at this layer. - `Configs` variants carry `T` (not `Option`). A variant is only emitted when the agent delivered a concrete value, so consumers leave any prior remote state untouched if a field is missing. - The `From for Vec` impl unwraps with `if let Some(v) = ...`, matching the dd-trace-go merge style. Kept from the prior iteration: - `pub` visibility on the struct fields and the tracing-rule types, so consumers can also access them directly without going through the `Configs` projection. - `#[serde(alias = "tracing_sample_rate")]` on `tracing_sampling_rate` for compatibility with payloads that use the legacy key. - The sidecar's "preserve dynamic_instrumentation across non-DI patches" fix (no upfront `take()`), which is correct regardless of the two- vs three-state question — and is now the *only* mechanism preserving values across incremental updates. Tests in `dynamic.rs` cover the four meaningful cases: absent → no-variant, explicit null → no-variant (indistinguishable from absent), concrete value → set variant, unrelated-field-present → no spurious sampling variants. Plus a guard for the legacy `tracing_sample_rate` alias. Co-Authored-By: Claude Opus 4.7 --- datadog-sidecar/src/shm_remote_config.rs | 4 +- libdd-remote-config/src/config/dynamic.rs | 227 +++++----------------- libdd-remote-config/src/config/mod.rs | 5 + 3 files changed, 59 insertions(+), 177 deletions(-) diff --git a/datadog-sidecar/src/shm_remote_config.rs b/datadog-sidecar/src/shm_remote_config.rs index 118a76d65c..f0c3022d8f 100644 --- a/datadog-sidecar/src/shm_remote_config.rs +++ b/datadog-sidecar/src/shm_remote_config.rs @@ -305,7 +305,7 @@ impl MultiTargetHandlers for ConfigFileStora let configs: Vec = config.lib_config.into(); for config in configs { if let Configs::DynamicInstrumentationEnabled(enabled) = config { - writer.dynamic_instrumentation = enabled; + writer.dynamic_instrumentation = Some(enabled); } } } @@ -923,7 +923,7 @@ mod tests { if let Some(cfg) = parsed.downcast::() { assert!(matches!( >::from(cfg.lib_config.clone())[0], - Configs::TracingEnabled(Some(true)) + Configs::TracingEnabled(true) )); } else { unreachable!(); diff --git a/libdd-remote-config/src/config/dynamic.rs b/libdd-remote-config/src/config/dynamic.rs index 7ff57f2934..cb744c9b57 100644 --- a/libdd-remote-config/src/config/dynamic.rs +++ b/libdd-remote-config/src/config/dynamic.rs @@ -45,7 +45,7 @@ impl DynamicConfigFile { #[derive(Debug, Clone, Deserialize)] #[cfg_attr(feature = "test", derive(Serialize))] -pub(crate) struct TracingHeaderTag { +pub struct TracingHeaderTag { pub header: String, pub tag_name: String, } @@ -76,171 +76,67 @@ pub struct TracingSamplingRule { pub sample_rate: f64, } -/// Three-state field carrying JSON Merge Patch (RFC 7396) semantics: -/// -/// - `Patch(None)` — field was absent on the wire (no change intended). -/// - `Patch(Some(None))` — field was present as JSON `null` (clear any prior remote override). -/// - `Patch(Some(Some(v)))` — field was present with a value (set to `v`). -/// -/// Used by [`DynamicConfig`] for every payload field so the absent / clear / -/// set distinction survives all the way from the wire to consumers reading -/// [`Configs`]. With struct-level `#[serde(default)]` on [`DynamicConfig`], -/// missing fields fall back to [`Patch::default`] (= `Patch(None)`); present -/// fields go through this `Deserialize` impl which resolves null-vs-value via -/// the inner `Option::::deserialize` and wraps the result in `Some` to -/// mark "delivered". -#[derive(Debug, Clone, PartialEq)] -pub struct Patch(pub Option>); - -impl Patch { - /// `true` when the field was not delivered on the wire. - pub fn is_absent(&self) -> bool { - self.0.is_none() - } -} - -impl Default for Patch { - fn default() -> Self { - Patch(None) - } -} - -impl<'de, T: Deserialize<'de>> Deserialize<'de> for Patch { - fn deserialize>(deserializer: D) -> Result { - // `Deserialize::deserialize` only runs when the field WAS present on - // the wire (absent fields go through `Default` via the struct-level - // `#[serde(default)]`). Resolving null-vs-value via the inner - // `Option::::deserialize` lets us encode all three states. - Option::::deserialize(deserializer).map(|inner| Patch(Some(inner))) - } -} - -#[cfg(feature = "test")] -impl Serialize for Patch { - fn serialize(&self, serializer: S) -> Result { - // The struct-level `Serialize` impl for `DynamicConfig` skips absent - // fields before reaching this method, so we only see delivered values: - // `Some(None)` → JSON `null`, `Some(Some(v))` → `v`. - match &self.0 { - None => serializer.serialize_none(), - Some(inner) => inner.serialize(serializer), - } - } -} - -/// Dynamic configuration delivered by the APM_TRACING product. -/// -/// Every field is a [`Patch`] so the absent / clear / set distinction -/// survives the wire. Struct-level `#[serde(default)]` lets missing fields -/// fall back to `Patch::default()` without per-field `deserialize_with` -/// boilerplate. The matching `Serialize` impl (under `feature = "test"`) is -/// hand-written below so absent fields are omitted from the output without -/// needing per-field `skip_serializing_if` annotations. See [`Configs`] for -/// how the three states surface to consumers. -#[derive(Debug, Clone, Default, Deserialize)] -#[serde(default)] +#[derive(Debug, Clone, Deserialize)] +#[cfg_attr(feature = "test", derive(Default, Serialize))] pub struct DynamicConfig { - pub(crate) tracing_header_tags: Patch>, - pub(crate) tracing_sampling_rate: Patch, - pub(crate) log_injection_enabled: Patch, - pub(crate) tracing_tags: Patch>, - pub(crate) tracing_enabled: Patch, - pub(crate) tracing_sampling_rules: Patch>, - pub(crate) dynamic_instrumentation_enabled: Patch, - pub(crate) exception_replay_enabled: Patch, - pub(crate) code_origin_enabled: Patch, -} - -#[cfg(feature = "test")] -impl Serialize for DynamicConfig { - fn serialize(&self, serializer: S) -> Result { - // Manual impl so absent fields (`Patch(None)`) are omitted from the - // output entirely rather than serializing as JSON `null` — that - // collapse would make round-trips re-deserialize absent fields as - // explicit clears, defeating the three-state semantics. - // - // `serialize_struct`'s `len` argument is only used by formats that - // pre-allocate (bincode, etc.); JSON ignores it. Pass the maximum. - use serde::ser::SerializeStruct; - macro_rules! field { - ($s:expr, $name:ident) => { - if !self.$name.is_absent() { - $s.serialize_field(stringify!($name), &self.$name)?; - } - }; - } - let mut s = serializer.serialize_struct("DynamicConfig", 9)?; - field!(s, tracing_header_tags); - field!(s, tracing_sampling_rate); - field!(s, log_injection_enabled); - field!(s, tracing_tags); - field!(s, tracing_enabled); - field!(s, tracing_sampling_rules); - field!(s, dynamic_instrumentation_enabled); - field!(s, exception_replay_enabled); - field!(s, code_origin_enabled); - s.end() - } + pub tracing_header_tags: Option>, + pub tracing_sampling_rate: Option, + pub log_injection_enabled: Option, + pub tracing_tags: Option>, + pub tracing_enabled: Option, + pub tracing_sampling_rules: Option>, + pub dynamic_instrumentation_enabled: Option, + pub exception_replay_enabled: Option, + pub code_origin_enabled: Option, } impl From for Vec { fn from(value: DynamicConfig) -> Self { - // A `Configs` variant is emitted whenever the field was present on - // the wire — including the explicit-`null` case, where the inner - // `Option` is `None` to signal "clear". Absent fields produce no - // variant at all, so callers can distinguish all three states. - let mut vec = vec![]; - if let Patch(Some(tags)) = value.tracing_header_tags { - vec.push(Configs::TracingHeaderTags(tags.map(|tags| { - tags.into_iter().map(|t| (t.header, t.tag_name)).collect() - }))); + let mut vec = Vec::with_capacity(9); + if let Some(tags) = value.tracing_header_tags { + vec.push(Configs::TracingHeaderTags( + tags.into_iter().map(|t| (t.header, t.tag_name)).collect(), + )); } - if let Patch(Some(sample_rate)) = value.tracing_sampling_rate { - vec.push(Configs::TracingSamplingRate(sample_rate)); + if let Some(sampling_rate) = value.tracing_sampling_rate { + vec.push(Configs::TracingSamplingRate(sampling_rate)); } - if let Patch(Some(log_injection)) = value.log_injection_enabled { + if let Some(log_injection) = value.log_injection_enabled { vec.push(Configs::LogInjectionEnabled(log_injection)); } - if let Patch(Some(tags)) = value.tracing_tags { + if let Some(tags) = value.tracing_tags { vec.push(Configs::TracingTags(tags)); } - if let Patch(Some(enabled)) = value.tracing_enabled { + if let Some(enabled) = value.tracing_enabled { vec.push(Configs::TracingEnabled(enabled)); } - if let Patch(Some(sampling_rules)) = value.tracing_sampling_rules { + if let Some(sampling_rules) = value.tracing_sampling_rules { vec.push(Configs::TracingSamplingRules(sampling_rules)); } - if let Patch(Some(enabled)) = value.dynamic_instrumentation_enabled { + if let Some(enabled) = value.dynamic_instrumentation_enabled { vec.push(Configs::DynamicInstrumentationEnabled(enabled)); } - if let Patch(Some(enabled)) = value.exception_replay_enabled { + if let Some(enabled) = value.exception_replay_enabled { vec.push(Configs::ExceptionReplayEnabled(enabled)); } - if let Patch(Some(enabled)) = value.code_origin_enabled { + if let Some(enabled) = value.code_origin_enabled { vec.push(Configs::CodeOriginEnabled(enabled)); } vec } } -/// A single APM_TRACING field that the agent has expressed an opinion about. -/// -/// Each variant carries `Option`: `Some(v)` means "set this field to `v`", -/// `None` means "clear any prior remote override and fall back to local -/// config". A field that was absent on the wire produces no variant at all, -/// so callers can distinguish all three states (absent / clear / set) by -/// matching on presence-in-the-`Vec` and on the inner `Option`. #[derive(Clone)] pub enum Configs { - TracingHeaderTags(Option>), - TracingSamplingRate(Option), - LogInjectionEnabled(Option), - TracingTags(Option>), // inner `Vec` items are "key:val" - TracingEnabled(Option), - TracingSamplingRules(Option>), - DynamicInstrumentationEnabled(Option), - ExceptionReplayEnabled(Option), - CodeOriginEnabled(Option), + TracingHeaderTags(HashMap), + TracingSamplingRate(f64), + LogInjectionEnabled(bool), + TracingTags(Vec), // "key:val" format + TracingEnabled(bool), + TracingSamplingRules(Vec), + DynamicInstrumentationEnabled(bool), + ExceptionReplayEnabled(bool), + CodeOriginEnabled(bool), } pub fn parse_json(data: &[u8]) -> serde_json::error::Result { @@ -256,55 +152,53 @@ pub mod tests { action: "".to_string(), service_target: None, lib_config: DynamicConfig { - tracing_enabled: Patch(Some(Some(enabled))), + tracing_enabled: Some(enabled), ..DynamicConfig::default() }, } } #[test] - fn parses_absent_field_as_patch_none() { + fn absent_field_emits_no_variant() { let cfg: DynamicConfigFile = parse_json(br#"{"action": "", "lib_config": {}}"#).unwrap(); - assert!(cfg.lib_config.tracing_sampling_rate.is_absent()); + assert!(cfg.lib_config.tracing_sampling_rate.is_none()); assert!(>::from(cfg.lib_config).is_empty()); } #[test] - fn parses_explicit_null_as_clear_intent() { + fn explicit_null_is_indistinguishable_from_absent() { + // No three-state model: null and absent both become `None`, so + // neither produces a `Configs` variant. Clearing prior remote state + // is the file-level responsibility (file removal), not an in-file + // signal. let cfg: DynamicConfigFile = parse_json(br#"{"action": "", "lib_config": {"tracing_sampling_rate": null}}"#) .unwrap(); - assert_eq!(cfg.lib_config.tracing_sampling_rate, Patch(Some(None))); - let configs: Vec = cfg.lib_config.into(); - assert_eq!(configs.len(), 1); - assert!(matches!(configs[0], Configs::TracingSamplingRate(None))); + assert!(cfg.lib_config.tracing_sampling_rate.is_none()); + assert!(>::from(cfg.lib_config).is_empty()); } #[test] - fn parses_concrete_value_as_set_intent() { + fn concrete_value_emits_set_variant() { let cfg: DynamicConfigFile = parse_json(br#"{"action": "", "lib_config": {"tracing_sampling_rate": 0.25}}"#) .unwrap(); - assert_eq!( - cfg.lib_config.tracing_sampling_rate, - Patch(Some(Some(0.25))) - ); + assert_eq!(cfg.lib_config.tracing_sampling_rate, Some(0.25)); let configs: Vec = cfg.lib_config.into(); assert_eq!(configs.len(), 1); - assert!(matches!(configs[0], Configs::TracingSamplingRate(Some(r)) if r == 0.25)); + assert!(matches!(configs[0], Configs::TracingSamplingRate(r) if r == 0.25)); } #[test] - fn unrelated_field_present_does_not_emit_sampling_variants() { + fn unrelated_field_does_not_emit_sampling_variants() { // Regression guard: a payload that updates only `tracing_tags` must - // not surface as a clear for `tracing_sampling_rate` / - // `tracing_sampling_rules` (and vice versa). Each field is - // independently absent / null / set. + // not produce a phantom `TracingSamplingRate` / `TracingSamplingRules` + // variant. Each field's absence is independent. let cfg: DynamicConfigFile = parse_json(br#"{"action": "", "lib_config": {"tracing_tags": ["foo:bar"]}}"#).unwrap(); let configs: Vec = cfg.lib_config.into(); assert_eq!(configs.len(), 1); - assert!(matches!(configs[0], Configs::TracingTags(Some(_)))); + assert!(matches!(configs[0], Configs::TracingTags(_))); assert!(!configs .iter() .any(|c| matches!(c, Configs::TracingSamplingRate(_)))); @@ -312,21 +206,4 @@ pub mod tests { .iter() .any(|c| matches!(c, Configs::TracingSamplingRules(_)))); } - - #[test] - fn round_trip_via_test_serialize_preserves_absent_vs_null() { - // The hand-written `Serialize` impl for `DynamicConfig` must skip - // absent fields entirely so the round-trip doesn't collapse - // `Patch(None)` → JSON `null` → `Patch(Some(None))`. - let mut cfg = dummy_dynamic_config(true); - cfg.lib_config.tracing_sampling_rate = Patch(Some(None)); - let json = serde_json::to_string(&cfg).unwrap(); - let parsed: DynamicConfigFile = parse_json(json.as_bytes()).unwrap(); - // absent fields stay absent - assert!(parsed.lib_config.tracing_sampling_rules.is_absent()); - // explicit-null stays explicit-null - assert_eq!(parsed.lib_config.tracing_sampling_rate, Patch(Some(None))); - // set value stays set - assert_eq!(parsed.lib_config.tracing_enabled, Patch(Some(Some(true)))); - } } diff --git a/libdd-remote-config/src/config/mod.rs b/libdd-remote-config/src/config/mod.rs index f1d9492eeb..e0201f149b 100644 --- a/libdd-remote-config/src/config/mod.rs +++ b/libdd-remote-config/src/config/mod.rs @@ -4,3 +4,8 @@ pub mod agent_config; pub mod agent_task; pub mod dynamic; + +pub use dynamic::{ + Configs, DynamicConfig, DynamicConfigFile, DynamicConfigTarget, TracingSamplingRule, + TracingSamplingRuleProvenance, TracingSamplingRuleTag, +}; From 00a69914180612f00246e289433e3b38f8eb04dd Mon Sep 17 00:00:00 2001 From: iunanua Date: Wed, 17 Jun 2026 09:27:54 +0200 Subject: [PATCH 4/4] Restore DynamicConfig --- libdd-remote-config/src/config/dynamic.rs | 20 ++++++++++---------- libdd-remote-config/src/config/mod.rs | 5 ----- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/libdd-remote-config/src/config/dynamic.rs b/libdd-remote-config/src/config/dynamic.rs index cb744c9b57..ec05529aae 100644 --- a/libdd-remote-config/src/config/dynamic.rs +++ b/libdd-remote-config/src/config/dynamic.rs @@ -45,7 +45,7 @@ impl DynamicConfigFile { #[derive(Debug, Clone, Deserialize)] #[cfg_attr(feature = "test", derive(Serialize))] -pub struct TracingHeaderTag { +pub(crate) struct TracingHeaderTag { pub header: String, pub tag_name: String, } @@ -79,15 +79,15 @@ pub struct TracingSamplingRule { #[derive(Debug, Clone, Deserialize)] #[cfg_attr(feature = "test", derive(Default, Serialize))] pub struct DynamicConfig { - pub tracing_header_tags: Option>, - pub tracing_sampling_rate: Option, - pub log_injection_enabled: Option, - pub tracing_tags: Option>, - pub tracing_enabled: Option, - pub tracing_sampling_rules: Option>, - pub dynamic_instrumentation_enabled: Option, - pub exception_replay_enabled: Option, - pub code_origin_enabled: Option, + pub(crate) tracing_header_tags: Option>, + pub(crate) tracing_sampling_rate: Option, + pub(crate) log_injection_enabled: Option, + pub(crate) tracing_tags: Option>, + pub(crate) tracing_enabled: Option, + pub(crate) tracing_sampling_rules: Option>, + pub(crate) dynamic_instrumentation_enabled: Option, + pub(crate) exception_replay_enabled: Option, + pub(crate) code_origin_enabled: Option, } impl From for Vec { diff --git a/libdd-remote-config/src/config/mod.rs b/libdd-remote-config/src/config/mod.rs index e0201f149b..f1d9492eeb 100644 --- a/libdd-remote-config/src/config/mod.rs +++ b/libdd-remote-config/src/config/mod.rs @@ -4,8 +4,3 @@ pub mod agent_config; pub mod agent_task; pub mod dynamic; - -pub use dynamic::{ - Configs, DynamicConfig, DynamicConfigFile, DynamicConfigTarget, TracingSamplingRule, - TracingSamplingRuleProvenance, TracingSamplingRuleTag, -};