diff --git a/Cargo.toml b/Cargo.toml index c5c190b..3f6d62a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,7 @@ routerify = "3" socket2 = { version = "0.5.3", features = [ "all" ] } syn = "1" serde = "1" +serde_ignored = "0.1.10" serde_path_to_error = "0.1.15" serde_yaml = "0.8.26" serde_with = "3.3.0" diff --git a/foundations/Cargo.toml b/foundations/Cargo.toml index c900ca8..c906bc1 100644 --- a/foundations/Cargo.toml +++ b/foundations/Cargo.toml @@ -64,6 +64,7 @@ settings = [ "dep:serde_path_to_error", "dep:serde_yaml", "dep:serde", + "dep:serde_ignored", "dep:yaml-merge-keys", "dep:indexmap", ] @@ -172,6 +173,7 @@ rand = { workspace = true, optional = true } rustracing = { workspace = true, optional = true } rustracing_jaeger = { workspace = true, optional = true } serde = { workspace = true, optional = true, features = ["derive", "rc"] } +serde_ignored = { workspace = true, optional = true } serde_path_to_error = { workspace = true, optional = true } serde_yaml = { workspace = true, optional = true } serde_with = { workspace = true, optional = true } diff --git a/foundations/src/settings/mod.rs b/foundations/src/settings/mod.rs index b8ad830..7c1bb37 100644 --- a/foundations/src/settings/mod.rs +++ b/foundations/src/settings/mod.rs @@ -154,6 +154,8 @@ use std::fs::File; use std::io; use std::path::Path; +pub use serde_ignored::Path as IgnoredPath; + /// A macro that implements the [`Settings`] trait for a structure or an enum /// and turns Rust doc comments into serializable documentation. /// @@ -313,6 +315,28 @@ use std::path::Path; /// [`Settings`]: crate::settings::Settings pub use foundations_macros::settings; +/// If included at the end of a YAML key and the default [SettingsDeserializationOptions] are used, +/// any YAML keys ending with this suffix will not be logged if unused. +/// +/// This can be useful for [YAML key references] +/// +/// # Examples +/// When the following YAML is deserialized into a settings struct that has no field corresponding +/// to `MERGE_ME_ALLOW_UNUSED`, no warning will be logged for `MERGE_ME_ALLOW_UNUSED`. +/// ```yml +/// --- +/// MERGE_ME_ALLOW_UNUSED: &MERGE_ME +/// a: 4 +/// b: 5 +/// +/// inner: +/// << : *MERGE_ME +/// x: 0 +/// ``` +/// +/// [YAML key references]: https://yaml.org/type/merge.html +pub const YAML_KEY_ALLOW_UNUSED_SUFFIX: &str = "ALLOW_UNUSED"; + /// A trait for a YAML-serializable settings with documentation. /// /// In most cases the trait don't need to be manually implemented and can be generated by the @@ -461,24 +485,117 @@ pub fn to_yaml_file(settings: &impl Settings, path: impl AsRef) -> Bootstr )?) } -/// Parse settings from YAML string. +/// Builds [SettingsDeserializationOptions] with specified options. +/// Any left unspecified will use defaults. +pub struct SettingsDeserializationOptionsBuilder { + ignored_key_callback: Option>, +} + +impl SettingsDeserializationOptionsBuilder { + /// Initialize the builder without any options specified + pub fn new() -> Self { + SettingsDeserializationOptionsBuilder { + ignored_key_callback: None, + } + } + + /// Set the callback that's called when YAML keys are not used during deserialization + /// into settings. + /// + /// If unspecified, the default will log each key as a warning, as long as it does not + /// end with [YAML_KEY_ALLOW_UNUSED_SUFFIX]. In that case, nothing will be logged. + /// + /// # Examples + /// + /// ``` + /// use foundations::settings::{IgnoredPath, SettingsDeserializationOptionsBuilder}; + /// + /// let builder = SettingsDeserializationOptionsBuilder::new(); + /// builder.with_ignored_key_callback(Box::new(|path| { + /// // Do something + /// })); + /// + /// ``` + pub fn with_ignored_key_callback(mut self, callback: Box) -> Self { + self.ignored_key_callback = Some(callback); + self + } + + /// Creates the configured [SettingsDeserializationOptions] + /// + /// Any values left unspecified will use default values. + pub fn build(self) -> SettingsDeserializationOptions { + SettingsDeserializationOptions { + ignored_key_callback: self.ignored_key_callback.unwrap_or_else(|| { + Box::new(|path| { + #[cfg(feature = "logging")] + if let IgnoredPath::Map { parent: _, key } = &path { + if !key.ends_with(YAML_KEY_ALLOW_UNUSED_SUFFIX) { + crate::telemetry::log::warn!("Unused key in settings"; "path" => %path); + } + } + }) + }), + } + } +} + +/// Options that modify how YAML is deserialized into settings +/// +/// Created using a [SettingsDeserializationOptionsBuilder], +/// which can be obtained with [SettingsDeserializationOptions::builder()] +pub struct SettingsDeserializationOptions { + ignored_key_callback: Box, +} + +impl SettingsDeserializationOptions { + /// Returns a default [SettingsDeserializationOptionsBuilder] + pub fn builder() -> SettingsDeserializationOptionsBuilder { + SettingsDeserializationOptionsBuilder::new() + } +} + +/// Parse settings from YAML string with options customizing deserialization behavior. /// /// Note: [YAML key references] will be merged during parsing. /// +/// Note: If the `logging` feature is enabled, this will log warnings for unused keys by default. +/// This can be customized by using [from_yaml_str_with_opts] instead. +/// /// [YAML key references]: https://yaml.org/type/merge.html -pub fn from_yaml_str(data: impl AsRef) -> BootstrapResult { +pub fn from_yaml_str_with_opts( + data: impl AsRef, + mut opts: SettingsDeserializationOptions, +) -> BootstrapResult { let de = serde_yaml::Deserializer::from_str(data.as_ref()); let value: serde_yaml::Value = serde_path_to_error::deserialize(de)?; + // NOTE: merge dict key refs: https://yaml.org/type/merge.html let value = yaml_merge_keys::merge_keys_serde(value)?; - Ok(serde_path_to_error::deserialize(value)?) + let de = serde_ignored::Deserializer::new(value, &mut opts.ignored_key_callback); + Ok(serde_path_to_error::deserialize(de)?) +} + +/// Parse settings from YAML string. +/// +/// Note: [YAML key references] will be merged during parsing. +/// +/// Note: If the `logging` feature is enabled, this will log warnings for unused keys by default. +/// This can be customized using [from_yaml_str_with_opts] instead. +/// +/// [YAML key references]: https://yaml.org/type/merge.html +pub fn from_yaml_str(data: impl AsRef) -> BootstrapResult { + let options = SettingsDeserializationOptions::builder().build(); + from_yaml_str_with_opts(data, options) } /// Parse settings from YAML file. /// /// Note: [YAML key references] will be merged during parsing. /// +/// Note: If the `logging` feature is enabled, this will log warnings for unused keys by default. +/// /// [YAML key references]: https://yaml.org/type/merge.html pub fn from_file(path: impl AsRef) -> BootstrapResult { let data = std::fs::read_to_string(path)?; diff --git a/foundations/src/telemetry/log/init.rs b/foundations/src/telemetry/log/init.rs index b51b574..5a3dafa 100644 --- a/foundations/src/telemetry/log/init.rs +++ b/foundations/src/telemetry/log/init.rs @@ -29,13 +29,15 @@ type FilteredDrain = LevelFilter< static HARNESS: OnceCell = OnceCell::new(); -static NOOP_HARNESS: Lazy = Lazy::new(|| { - let root_drain = Arc::new(Discard); - let noop_log = Logger::root(Arc::clone(&root_drain), slog::o!()); +static PRE_INIT_HARNESS: Lazy = Lazy::new(|| { + let base_drain = get_base_drain(&LoggingSettings::default()) + .unwrap_or_else(|_| AsyncDrain::new(Discard).build()); + let root_drain = Arc::new(base_drain.fuse()); + let pre_init_log = Logger::root(Arc::clone(&root_drain), slog::o!()); LogHarness { root_drain, - root_log: Arc::new(parking_lot::RwLock::new(noop_log)), + root_log: Arc::new(parking_lot::RwLock::new(pre_init_log)), settings: Default::default(), log_scope_stack: Default::default(), } @@ -50,7 +52,7 @@ pub(crate) struct LogHarness { impl LogHarness { pub(crate) fn get() -> &'static Self { - HARNESS.get().unwrap_or(&NOOP_HARNESS) + HARNESS.get().unwrap_or(&PRE_INIT_HARNESS) } #[cfg(any(test, feature = "testing"))] @@ -66,10 +68,33 @@ pub(crate) fn init(service_info: &ServiceInfo, settings: &LoggingSettings) -> Bo return Ok(()); } + let base_drain = get_base_drain(settings)?; + let root_drain = get_root_drain(settings, Arc::new(base_drain.fuse())); + let root_kv = slog::o!( + "module" => FnValue(|record| { + format!("{}:{}", record.module(), record.line()) + }), + "version" => service_info.version, + "pid" => std::process::id(), + ); + + let root_log = build_log_with_drain(settings, root_kv, Arc::clone(&root_drain)); + let harness = LogHarness { + root_drain, + root_log: Arc::new(parking_lot::RwLock::new(root_log)), + settings: settings.clone(), + log_scope_stack: Default::default(), + }; + + let _ = HARNESS.set(harness); + + Ok(()) +} + +fn get_base_drain(settings: &LoggingSettings) -> Result { // NOTE: OXY-178, default is 128 (https://docs.rs/slog-async/2.7.0/src/slog_async/lib.rs.html#251) const CHANNEL_SIZE: usize = 1024; - - let base_drain = match (&settings.output, &settings.format) { + Ok(match (&settings.output, &settings.format) { (LogOutput::Terminal, LogFormat::Text) => { let drain = TextDrain::new(TermDecorator::new().stdout().build()) .build() @@ -90,28 +115,7 @@ pub(crate) fn init(service_info: &ServiceInfo, settings: &LoggingSettings) -> Bo let drain = build_json_log_drain(File::create(file)?); AsyncDrain::new(drain).chan_size(CHANNEL_SIZE).build() } - }; - - let root_drain = get_root_drain(settings, Arc::new(base_drain.fuse())); - let root_kv = slog::o!( - "module" => FnValue(|record| { - format!("{}:{}", record.module(), record.line()) - }), - "version" => service_info.version, - "pid" => std::process::id(), - ); - - let root_log = build_log_with_drain(settings, root_kv, Arc::clone(&root_drain)); - let harness = LogHarness { - root_drain, - root_log: Arc::new(parking_lot::RwLock::new(root_log)), - settings: settings.clone(), - log_scope_stack: Default::default(), - }; - - let _ = HARNESS.set(harness); - - Ok(()) + }) } fn get_root_drain( diff --git a/foundations/tests/data/settings_nested_struct_using_merge_keys.yaml b/foundations/tests/data/settings_nested_struct_using_merge_keys.yaml new file mode 100644 index 0000000..2ea6a80 --- /dev/null +++ b/foundations/tests/data/settings_nested_struct_using_merge_keys.yaml @@ -0,0 +1,10 @@ +--- +MERGE_ME_ALLOW_UNUSED: &MERGE_ME + a: 4 + b: 5 + c: 6 + d: "extraneous" + +inner: + << : *MERGE_ME +x: 0 \ No newline at end of file diff --git a/foundations/tests/settings.rs b/foundations/tests/settings.rs index 0ded87c..0ecd3c0 100644 --- a/foundations/tests/settings.rs +++ b/foundations/tests/settings.rs @@ -1,8 +1,13 @@ use foundations::settings::collections::Map; use foundations::settings::net::SocketAddr; -use foundations::settings::{settings, to_yaml_string}; +use foundations::settings::{from_yaml_str, settings, to_yaml_string}; +use foundations::telemetry::log::TestLogRecord; +use foundations::telemetry::TestTelemetryContext; +use foundations_macros::with_test_telemetry; +use slog::Level; #[settings] +#[derive(PartialEq)] struct NestedStruct { /// A field, which is named the same as another field. a: usize, @@ -14,6 +19,12 @@ struct NestedStruct { c: u32, } +#[settings] +struct NestedStructMissingB { + a: usize, + c: u32, +} + impl NestedStruct { fn default_b() -> u32 { 0xb @@ -21,6 +32,7 @@ impl NestedStruct { } #[settings] +#[derive(PartialEq)] struct SimpleStruct { /// The documentation of NestedStruct /// will be added to the keys of `inner` @@ -29,6 +41,12 @@ struct SimpleStruct { x: u32, } +#[settings] +struct SimpleStructWithInnerMissingB { + inner: NestedStructMissingB, + x: u32, +} + #[settings] struct NestedDup { inner: NestedStruct, @@ -256,3 +274,49 @@ fn vec() { assert_ser_eq!(s, "data/with_vec.yaml"); } + +#[test] +fn deserialize_from_yaml_str() { + let deserialized: SimpleStruct = + from_yaml_str(include_str!("data/settings_nested_struct.yaml")).unwrap(); + + assert_eq!( + SimpleStruct { + inner: NestedStruct { a: 0, b: 11, c: 0 }, + x: 0, + }, + deserialized + ); +} + +#[with_test_telemetry(test)] +fn warns_on_unused_key(ctx: TestTelemetryContext) { + let _: SimpleStructWithInnerMissingB = + from_yaml_str(include_str!("data/settings_nested_struct.yaml")).unwrap(); + + assert_eq!( + *ctx.log_records(), + vec![TestLogRecord { + level: Level::Warning, + message: "Unused key in settings".into(), + fields: vec![("path".into(), "inner.b".into())] + }] + ); +} + +#[with_test_telemetry(test)] +fn warns_on_unused_key_inside_merge_key(ctx: TestTelemetryContext) { + let _: SimpleStruct = from_yaml_str(include_str!( + "data/settings_nested_struct_using_merge_keys.yaml" + )) + .unwrap(); + + assert_eq!( + *ctx.log_records(), + vec![TestLogRecord { + level: Level::Warning, + message: "Unused key in settings".into(), + fields: vec![("path".into(), "inner.d".into())] + }] + ) +}