diff --git a/crates/datadog-agent-config/src/deserializers/helpers.rs b/crates/datadog-agent-config/src/deserializers/helpers.rs index 058c0ce1..f0dee862 100644 --- a/crates/datadog-agent-config/src/deserializers/helpers.rs +++ b/crates/datadog-agent-config/src/deserializers/helpers.rs @@ -175,6 +175,84 @@ where .collect()) } +/// Deserialize a string into a list of entries, choosing exactly one delimiter: +/// if the string contains a comma, split on commas only; otherwise, split on the +/// literal space character (not whitespace in general, so tabs stay inside entries). +/// Each entry is trimmed and empty entries are dropped, all on the Rust side. +/// This is deliberately NOT full Go parity: Go's `comma_then_space_separated` +/// (pkg/config/helper/env.go) keeps empty entries. +pub fn deserialize_comma_then_space_separated_string<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let s: String = String::deserialize(deserializer)?; + Ok(split_comma_then_space(&s)) +} + +fn split_comma_then_space(s: &str) -> Vec { + let parts: Vec<&str> = if s.contains(',') { + s.split(',').collect() + } else { + s.split(' ').collect() + }; + parts + .iter() + .map(|part| part.trim().to_string()) + .filter(|part| !part.is_empty()) + .collect() +} + +/// Deserialize `apm_config.features` from YAML, accepting two shapes: +/// - a native list, whose string elements are used as-is +/// - a string, split with the same comma-then-space rules as `DD_APM_FEATURES` +/// (comma wins as delimiter when present, otherwise the literal space +/// character splits; entries are trimmed and empty entries are dropped) +/// +/// A native list with mixed element types is rejected entirely: any +/// non-string element is logged and the whole list falls back to `[]`. +/// This mirrors the Go agent, where `GetStringSlice` goes through +/// `cast.ToStringSlice`, which returns an empty slice when any element of +/// the list is not a string (the error is swallowed, without a warning). +/// +/// Any other shape, or a non-string list element, is logged and ignored. +/// This diverges from the Go agent, whose YAML path does not comma-split +/// strings for this setting. +pub fn deserialize_comma_then_space_separated_string_or_list<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + match Value::deserialize(deserializer)? { + Value::Array(items) => { + let mut features = Vec::with_capacity(items.len()); + for item in items { + match item { + Value::String(s) => features.push(s), + other => { + warn!( + "Failed to parse features: expected a string element, got: {}, dropping the entire list", + other + ); + return Ok(Vec::new()); + } + } + } + Ok(features) + } + Value::String(s) => Ok(split_comma_then_space(&s)), + other => { + warn!( + "Failed to parse features: expected a list or a string, got: {}, ignoring", + other + ); + Ok(Vec::new()) + } + } +} + pub fn deserialize_key_value_pair_array_to_hashmap<'de, D>( deserializer: D, ) -> Result, D::Error> diff --git a/crates/datadog-agent-config/src/sources/env.rs b/crates/datadog-agent-config/src/sources/env.rs index 83fbe7d6..fd3fe89e 100644 --- a/crates/datadog-agent-config/src/sources/env.rs +++ b/crates/datadog-agent-config/src/sources/env.rs @@ -10,9 +10,10 @@ use crate::{ additional_endpoints::deserialize_additional_endpoints, apm_replace_rule::deserialize_apm_replace_rules, deserialize_apm_filter_tags, deserialize_array_from_comma_separated_string, - deserialize_key_value_pairs, deserialize_option_lossless, - deserialize_optional_bool_from_anything, deserialize_optional_string, - deserialize_string_or_int, deserialize_trace_propagation_style, deserialize_with_default, + deserialize_comma_then_space_separated_string, deserialize_key_value_pairs, + deserialize_option_lossless, deserialize_optional_bool_from_anything, + deserialize_optional_string, deserialize_string_or_int, deserialize_trace_propagation_style, + deserialize_with_default, log_level::LogLevel, logs_additional_endpoints::{LogsAdditionalEndpoint, deserialize_logs_additional_endpoints}, merge_hashmap, merge_option, merge_option_to_value, merge_string, merge_vec, @@ -65,6 +66,8 @@ pub struct EnvConfig { /// @env `DD_PROXY_NO_PROXY` /// /// Specify hosts the Agent should connect to directly, bypassing the proxy. + // Comma-only splitting, which diverges from Go: the Go agent parses NO_PROXY + // with comma+space via strings.FieldsFunc and drops empty entries. #[serde(deserialize_with = "deserialize_array_from_comma_separated_string")] pub proxy_no_proxy: Vec, /// @env `DD_HTTP_PROTOCOL` @@ -231,7 +234,7 @@ pub struct EnvConfig { #[serde(deserialize_with = "deserialize_option_lossless")] pub apm_config_compression_level: Option, /// @env `DD_APM_FEATURES` - #[serde(deserialize_with = "deserialize_array_from_comma_separated_string")] + #[serde(deserialize_with = "deserialize_comma_then_space_separated_string")] pub apm_features: Vec, /// @env `DD_APM_ADDITIONAL_ENDPOINTS` /// @@ -939,7 +942,7 @@ mod tests { jail.set_env("DD_APM_CONFIG_COMPRESSION_LEVEL", "2"); jail.set_env( "DD_APM_FEATURES", - "enable_otlp_compute_top_level_by_span_kind,enable_stats_by_span_kind", + "enable_otlp_compute_top_level_by_span_kind enable_stats_by_span_kind", ); jail.set_env("DD_APM_ADDITIONAL_ENDPOINTS", "{\"https://trace.agent.datadoghq.com\": [\"apikey2\", \"apikey3\"], \"https://trace.agent.datadoghq.eu\": [\"apikey4\"]}"); jail.set_env( @@ -1263,6 +1266,40 @@ mod tests { }); } + #[test] + fn test_apm_features_comma_then_space_separated_from_env() { + figment::Jail::expect_with(|jail| { + jail.clear_env(); + + let inputs_and_expected = [ + ("a b", vec!["a".to_string(), "b".to_string()]), + ("a,b", vec!["a".to_string(), "b".to_string()]), + // Comma wins as delimiter, space stays inside the entry. + ("a,b c", vec!["a".to_string(), "b c".to_string()]), + // Trimming plus empty-entry dropping. + (" a ,, b ", vec!["a".to_string(), "b".to_string()]), + // Split on the literal space char, then trim. + (" a b ", vec!["a".to_string(), "b".to_string()]), + ("", vec![]), + (" ", vec![]), + // Tab is not a delimiter; it stays inside the entry. + ("a\tb", vec!["a\tb".to_string()]), + ]; + + for (input, expected) in inputs_and_expected { + jail.set_env("DD_APM_FEATURES", input); + + let mut config: Config = Config::default(); + EnvConfigSource + .load(&mut config) + .expect("Failed to load config"); + + assert_eq!(config.apm_features, expected, "input: {input:?}"); + } + Ok(()) + }); + } + #[test] fn test_dogstatsd_config_defaults_to_none() { figment::Jail::expect_with(|jail| { diff --git a/crates/datadog-agent-config/src/sources/yaml.rs b/crates/datadog-agent-config/src/sources/yaml.rs index b8f10d4c..8ca5f49d 100644 --- a/crates/datadog-agent-config/src/sources/yaml.rs +++ b/crates/datadog-agent-config/src/sources/yaml.rs @@ -3,6 +3,7 @@ use std::{collections::HashMap, path::PathBuf}; use crate::{ Config, ConfigError, ConfigExtension, ConfigSource, ProcessingRule, TracePropagationStyle, additional_endpoints::deserialize_additional_endpoints, deserialize_apm_replace_rules, + deserialize_comma_then_space_separated_string_or_list, deserialize_key_value_pair_array_to_hashmap, deserialize_option_lossless, deserialize_optional_bool_from_anything, deserialize_optional_string, deserialize_processing_rules, deserialize_string_or_int, deserialize_trace_propagation_style, @@ -173,7 +174,7 @@ pub struct ApmConfig { pub obfuscation: Option, #[serde(deserialize_with = "deserialize_option_lossless")] pub compression_level: Option, - #[serde(deserialize_with = "deserialize_with_default")] + #[serde(deserialize_with = "deserialize_comma_then_space_separated_string_or_list")] pub features: Vec, #[serde(deserialize_with = "deserialize_additional_endpoints")] pub additional_endpoints: HashMap>, @@ -1166,6 +1167,137 @@ otlp_config: }); } + #[test] + fn test_apm_features_comma_then_space_separated_from_yaml() { + figment::Jail::expect_with(|jail| { + jail.clear_env(); + + // (yaml features value, expected Vec) + let inputs_and_expected = [ + ("a,b", vec!["a".to_string(), "b".to_string()]), + ("a b", vec!["a".to_string(), "b".to_string()]), + // Comma wins as delimiter, space stays inside the entry. + ("a,b c", vec!["a".to_string(), "b c".to_string()]), + // Trimming plus empty-entry dropping. + (" a , b ", vec!["a".to_string(), "b".to_string()]), + ("", vec![]), + // Tab is not a delimiter; it stays inside the entry. + ("a\tb", vec!["a\tb".to_string()]), + ]; + + for (value, expected) in inputs_and_expected { + jail.create_file( + "datadog.yaml", + &format!("apm_config:\n features: {value:?}"), + )?; + + let mut config: Config = Config::default(); + let yaml_config_source = YamlConfigSource { + path: Path::new("datadog.yaml").to_path_buf(), + }; + yaml_config_source + .load(&mut config) + .expect("Failed to load config"); + + assert_eq!(config.apm_features, expected, "value: {value:?}"); + } + + Ok(()) + }); + } + + #[test] + fn test_apm_features_yaml_list_still_supported() { + figment::Jail::expect_with(|jail| { + jail.clear_env(); + jail.create_file( + "datadog.yaml", + r#" +apm_config: + features: + - "feature_one" + - "feature_two" +"#, + )?; + + let mut config: Config = Config::default(); + let yaml_config_source = YamlConfigSource { + path: Path::new("datadog.yaml").to_path_buf(), + }; + yaml_config_source + .load(&mut config) + .expect("Failed to load config"); + + assert_eq!( + config.apm_features, + vec!["feature_one".to_string(), "feature_two".to_string()] + ); + + Ok(()) + }); + } + + #[test] + fn test_apm_features_yaml_mixed_type_list_dropped_entirely() { + figment::Jail::expect_with(|jail| { + jail.clear_env(); + jail.create_file( + "datadog.yaml", + r#" +apm_config: + features: + - "feature_one" + - 2 + - "feature_two" +"#, + )?; + + let mut config: Config = Config::default(); + let yaml_config_source = YamlConfigSource { + path: Path::new("datadog.yaml").to_path_buf(), + }; + yaml_config_source + .load(&mut config) + .expect("Failed to load config"); + + // Mirrors the Go agent: GetStringSlice goes through cast.ToStringSlice, + // which returns an empty slice when any element is not a string. + assert_eq!(config.apm_features, Vec::::new()); + + Ok(()) + }); + } + + #[test] + fn test_apm_features_env_overrides_yaml() { + figment::Jail::expect_with(|jail| { + jail.clear_env(); + jail.create_file( + "datadog.yaml", + r#" +apm_config: + features: "yaml_feature" +"#, + )?; + jail.set_env("DD_APM_FEATURES", "env_feature"); + + let mut config: Config = Config::default(); + let yaml_config_source = YamlConfigSource { + path: Path::new("datadog.yaml").to_path_buf(), + }; + yaml_config_source + .load(&mut config) + .expect("Failed to load yaml config"); + crate::EnvConfigSource + .load(&mut config) + .expect("Failed to load env config"); + + assert_eq!(config.apm_features, vec!["env_feature".to_string()]); + + Ok(()) + }); + } + #[test] fn test_yaml_dogstatsd_config() { figment::Jail::expect_with(|jail| {