From 848874d89db9df7f9fd5a9181633ec398b567cc6 Mon Sep 17 00:00:00 2001 From: Techassi Date: Tue, 28 Jan 2025 15:53:46 +0100 Subject: [PATCH 01/12] wip: Make builder functions take Option --- crates/stackable-telemetry/src/tracing/mod.rs | 202 ++++++++++-------- .../src/tracing/settings/console_log.rs | 91 ++++---- .../src/tracing/settings/file_log.rs | 2 - .../src/tracing/settings/mod.rs | 25 --- .../src/tracing/settings/otlp_log.rs | 52 +++-- .../src/tracing/settings/otlp_trace.rs | 52 +++-- 6 files changed, 227 insertions(+), 197 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index 7af9206c1..93af2d09c 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -218,9 +218,9 @@ pub enum Error { pub struct Tracing { service_name: &'static str, console_log_settings: ConsoleLogSettings, - file_log_settings: FileLogSettings, - otlp_log_settings: OtlpLogSettings, - otlp_trace_settings: OtlpTraceSettings, + file_log_settings: Option, + otlp_log_settings: Option, + otlp_trace_settings: Option, logger_provider: Option, } @@ -239,22 +239,24 @@ impl Tracing { pub fn init(mut self) -> Result { let mut layers: Vec + Sync + Send>> = Vec::new(); - if self.console_log_settings.enabled { + if let ConsoleLogSettings::Enabled { + common_settings, + log_format: _, + } = &self.console_log_settings + { let env_filter_layer = env_filter_builder( - self.console_log_settings - .common_settings - .environment_variable, - self.console_log_settings.default_level, + common_settings.environment_variable, + common_settings.default_level, ); let console_output_layer = tracing_subscriber::fmt::layer().with_filter(env_filter_layer); layers.push(console_output_layer.boxed()); } - if self.file_log_settings.enabled { + if let Some(file_log_settings) = &self.file_log_settings { let env_filter_layer = env_filter_builder( - self.file_log_settings.common_settings.environment_variable, - self.file_log_settings.default_level, + file_log_settings.common_settings.environment_variable, + file_log_settings.default_level, ); let file_appender = RollingFileAppender::builder() @@ -262,7 +264,7 @@ impl Tracing { .filename_prefix(self.service_name.to_string()) .filename_suffix("tracing-rs.json") .max_log_files(6) - .build(&self.file_log_settings.file_log_dir) + .build(&file_log_settings.file_log_dir) .context(InitRollingFileAppenderSnafu)?; layers.push( @@ -274,10 +276,10 @@ impl Tracing { ); } - if self.otlp_log_settings.enabled { + if let Some(otlp_log_settings) = &self.otlp_log_settings { let env_filter_layer = env_filter_builder( - self.otlp_log_settings.environment_variable, - self.otlp_log_settings.default_level, + otlp_log_settings.environment_variable, + otlp_log_settings.default_level, ) // TODO (@NickLarsenNZ): Remove this directive once https://github.com/open-telemetry/opentelemetry-rust/issues/761 is resolved .add_directive("h2=off".parse().expect("invalid directive")); @@ -302,12 +304,10 @@ impl Tracing { self.logger_provider = Some(otel_log); } - if self.otlp_trace_settings.enabled { + if let Some(otlp_trace_settings) = &self.otlp_trace_settings { let env_filter_layer = env_filter_builder( - self.otlp_trace_settings - .common_settings - .environment_variable, - self.otlp_trace_settings.default_level, + otlp_trace_settings.common_settings.environment_variable, + otlp_trace_settings.default_level, ) // TODO (@NickLarsenNZ): Remove this directive once https://github.com/open-telemetry/opentelemetry-rust/issues/761 is resolved .add_directive("h2=off".parse().expect("invalid directive")); @@ -355,12 +355,12 @@ impl Tracing { impl Drop for Tracing { fn drop(&mut self) { tracing::debug!( - opentelemetry.tracing.enabled = self.otlp_trace_settings.enabled, - opentelemetry.logger.enabled = self.otlp_log_settings.enabled, + opentelemetry.tracing.enabled = self.otlp_trace_settings.is_some(), + opentelemetry.logger.enabled = self.otlp_log_settings.is_some(), "shutting down opentelemetry OTLP providers" ); - if self.otlp_trace_settings.enabled { + if self.otlp_trace_settings.is_some() { // NOTE (@NickLarsenNZ): This might eventually be replaced with something like SdkMeterProvider::shutdown(&self) // as has been done with the LoggerProvider (further below) // see: https://github.com/open-telemetry/opentelemetry-rust/pull/1412/files#r1409608679 @@ -434,9 +434,9 @@ impl BuilderState for builder_state::Config {} pub struct TracingBuilder { service_name: Option<&'static str>, console_log_settings: ConsoleLogSettings, - otlp_log_settings: OtlpLogSettings, - otlp_trace_settings: OtlpTraceSettings, - file_log_settings: FileLogSettings, + otlp_log_settings: Option, + otlp_trace_settings: Option, + file_log_settings: Option, /// Allow the generic to be used (needed for impls). _marker: std::marker::PhantomData, @@ -486,7 +486,7 @@ impl TracingBuilder { TracingBuilder { service_name: self.service_name, console_log_settings: self.console_log_settings, - file_log_settings: file_log_settings.into(), + file_log_settings: Some(file_log_settings.into()), otlp_log_settings: self.otlp_log_settings, otlp_trace_settings: self.otlp_trace_settings, _marker: self._marker, @@ -507,7 +507,7 @@ impl TracingBuilder { TracingBuilder { service_name: self.service_name, console_log_settings: self.console_log_settings, - otlp_log_settings: otlp_log_settings.into(), + otlp_log_settings: Some(otlp_log_settings.into()), otlp_trace_settings: self.otlp_trace_settings, file_log_settings: self.file_log_settings, _marker: self._marker, @@ -529,7 +529,7 @@ impl TracingBuilder { service_name: self.service_name, console_log_settings: self.console_log_settings, otlp_log_settings: self.otlp_log_settings, - otlp_trace_settings: otlp_trace_settings.into(), + otlp_trace_settings: Some(otlp_trace_settings.into()), file_log_settings: self.file_log_settings, _marker: self._marker, } @@ -586,74 +586,71 @@ mod test { Settings::builder() .with_environment_variable("ABC_A") .with_default_level(LevelFilter::TRACE) - .enabled(true) .build(), ) .with_console_output( Settings::builder() .with_environment_variable("ABC_B") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .build(), ) .build(); assert_eq!( trace_guard.console_log_settings, - ConsoleLogSettings { + ConsoleLogSettings::Enabled { common_settings: Settings { - enabled: true, environment_variable: "ABC_B", default_level: LevelFilter::DEBUG }, log_format: Default::default() } ); - assert!(!trace_guard.otlp_log_settings.enabled); - assert!(!trace_guard.otlp_trace_settings.enabled); - } - - #[test] - fn builder_with_console_output_double() { - let trace_guard = Tracing::builder() - .service_name("test") - .with_console_output(("ABC_A", LevelFilter::TRACE)) - .build(); - assert_eq!( - trace_guard.console_log_settings, - ConsoleLogSettings { - common_settings: Settings { - environment_variable: "ABC_A", - default_level: LevelFilter::TRACE, - enabled: true - }, - log_format: Default::default() - } - ) + assert!(trace_guard.otlp_log_settings.is_none()); + assert!(trace_guard.otlp_trace_settings.is_none()); } - #[rstest] - #[case(false)] - #[case(true)] - fn builder_with_console_output_triple(#[case] enabled: bool) { - let trace_guard = Tracing::builder() - .service_name("test") - .with_console_output(("ABC_A", LevelFilter::TRACE, enabled)) - .build(); - - assert_eq!( - trace_guard.console_log_settings, - ConsoleLogSettings { - common_settings: Settings { - environment_variable: "ABC_A", - default_level: LevelFilter::TRACE, - enabled - }, - log_format: Default::default() - } - ) - } + // #[test] + // fn builder_with_console_output_double() { + // let trace_guard = Tracing::builder() + // .service_name("test") + // .with_console_output(("ABC_A", LevelFilter::TRACE)) + // .build(); + + // assert_eq!( + // trace_guard.console_log_settings, + // ConsoleLogSettings { + // common_settings: Settings { + // environment_variable: "ABC_A", + // default_level: LevelFilter::TRACE, + // }, + // log_format: Default::default() + // } + // ) + // } + + // #[rstest] + // #[case(false)] + // #[case(true)] + // fn builder_with_console_output_triple(#[case] enabled: bool) { + // let trace_guard = Tracing::builder() + // .service_name("test") + // .with_console_output(("ABC_A", LevelFilter::TRACE, enabled)) + // .build(); + + // assert_eq!( + // trace_guard.console_log_settings, + // ConsoleLogSettings { + // common_settings: Settings { + // environment_variable: "ABC_A", + // default_level: LevelFilter::TRACE, + // enabled + // }, + // log_format: Default::default() + // } + // ) + // } #[test] fn builder_with_all() { @@ -663,14 +660,12 @@ mod test { Settings::builder() .with_environment_variable("ABC_CONSOLE") .with_default_level(LevelFilter::INFO) - .enabled(true) .build(), ) .with_file_output( Settings::builder() .with_environment_variable("ABC_FILE") .with_default_level(LevelFilter::INFO) - .enabled(true) .file_log_settings_builder(PathBuf::from("/abc_file_dir")) .build(), ) @@ -678,23 +673,20 @@ mod test { Settings::builder() .with_environment_variable("ABC_OTLP_LOG") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .build(), ) .with_otlp_trace_exporter( Settings::builder() .with_environment_variable("ABC_OTLP_TRACE") .with_default_level(LevelFilter::TRACE) - .enabled(true) .build(), ) .build(); assert_eq!( trace_guard.console_log_settings, - ConsoleLogSettings { + ConsoleLogSettings::Enabled { common_settings: Settings { - enabled: true, environment_variable: "ABC_CONSOLE", default_level: LevelFilter::INFO }, @@ -703,34 +695,64 @@ mod test { ); assert_eq!( trace_guard.file_log_settings, - FileLogSettings { + Some(FileLogSettings { common_settings: Settings { - enabled: true, environment_variable: "ABC_FILE", default_level: LevelFilter::INFO }, file_log_dir: PathBuf::from("/abc_file_dir") - } + }) ); assert_eq!( trace_guard.otlp_log_settings, - OtlpLogSettings { + Some(OtlpLogSettings { common_settings: Settings { - enabled: true, environment_variable: "ABC_OTLP_LOG", default_level: LevelFilter::DEBUG }, - } + }) ); assert_eq!( trace_guard.otlp_trace_settings, - OtlpTraceSettings { + Some(OtlpTraceSettings { common_settings: Settings { - enabled: true, environment_variable: "ABC_OTLP_TRACE", default_level: LevelFilter::TRACE } - } + }) + ); + } + + #[test] + fn builder_with_options() { + let enable_console_output = true; + let enable_otlp_trace = true; + let enable_otlp_log = false; + + let tracing_builder = Tracing::builder() + .service_name("test") + .with_console_output(enable_console_output.then(|| { + Settings::builder() + .with_environment_variable("ABC_CONSOLE") + .build() + })) + .with_otlp_trace_exporter(enable_otlp_trace.then(|| { + Settings::builder() + .with_environment_variable("ABC_OTLP_TRACE") + .build() + })) + .with_otlp_log_exporter(enable_otlp_log.then(|| { + Settings::builder() + .with_environment_variable("ABC_OTLP_LOG") + .build() + })) + .build(); + + matches!( + tracing_builder.console_log_settings, + ConsoleLogSettings::Enabled { .. } ); + assert!(tracing_builder.otlp_trace_settings.is_some()); + // assert!(tracing_builder.otlp_log_settings.is_none()); } } diff --git a/crates/stackable-telemetry/src/tracing/settings/console_log.rs b/crates/stackable-telemetry/src/tracing/settings/console_log.rs index dc26379fd..e3c35f39a 100644 --- a/crates/stackable-telemetry/src/tracing/settings/console_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/console_log.rs @@ -8,21 +8,26 @@ use super::{Settings, SettingsBuilder}; /// Configure specific settings for the Console Log subscriber. #[derive(Debug, Default, PartialEq)] -pub struct ConsoleLogSettings { - /// Common subscriber settings that apply to the Console Log Subscriber. - pub common_settings: Settings, +pub enum ConsoleLogSettings { + #[default] + Disabled, + + Enabled { + /// Common subscriber settings that apply to the Console Log Subscriber. + common_settings: Settings, - /// Console Subscriber log event output format. - pub log_format: Format, + /// Console Subscriber log event output format. + log_format: Format, + }, } -impl Deref for ConsoleLogSettings { - type Target = Settings; +// impl Deref for ConsoleLogSettings { +// type Target = Settings; - fn deref(&self) -> &Self::Target { - &self.common_settings - } -} +// fn deref(&self) -> &Self::Target { +// &self.common_settings +// } +// } /// Console Subscriber log event output formats. /// @@ -58,7 +63,7 @@ impl ConsoleLogSettingsBuilder { } pub fn build(self) -> ConsoleLogSettings { - ConsoleLogSettings { + ConsoleLogSettings::Enabled { common_settings: self.common_settings, log_format: self.log_format, } @@ -78,38 +83,50 @@ impl From for ConsoleLogSettingsBuilder { impl From for ConsoleLogSettings { fn from(common_settings: Settings) -> Self { - ConsoleLogSettings { + ConsoleLogSettings::Enabled { common_settings, - ..Default::default() + log_format: Default::default(), } } } -impl From<(&'static str, LevelFilter)> for ConsoleLogSettings { - fn from(value: (&'static str, LevelFilter)) -> Self { - Self { - common_settings: Settings { - environment_variable: value.0, - default_level: value.1, - enabled: true, - }, - ..Default::default() +impl From> for ConsoleLogSettings +where + T: Into, +{ + fn from(settings: Option) -> Self { + match settings { + Some(settings) => settings.into(), + None => ConsoleLogSettings::default(), } } } -impl From<(&'static str, LevelFilter, bool)> for ConsoleLogSettings { - fn from(value: (&'static str, LevelFilter, bool)) -> Self { - Self { - common_settings: Settings { - environment_variable: value.0, - default_level: value.1, - enabled: value.2, - }, - ..Default::default() - } - } -} +// impl From<(&'static str, LevelFilter)> for ConsoleLogSettings { +// fn from(value: (&'static str, LevelFilter)) -> Self { +// Self { +// common_settings: Settings { +// environment_variable: value.0, +// default_level: value.1, +// enabled: true, +// }, +// ..Default::default() +// } +// } +// } + +// impl From<(&'static str, LevelFilter, bool)> for ConsoleLogSettings { +// fn from(value: (&'static str, LevelFilter, bool)) -> Self { +// Self { +// common_settings: Settings { +// environment_variable: value.0, +// default_level: value.1, +// enabled: value.2, +// }, +// ..Default::default() +// } +// } +// } #[cfg(test)] mod test { @@ -119,18 +136,16 @@ mod test { #[test] fn builds_settings() { - let expected = ConsoleLogSettings { + let expected = ConsoleLogSettings::Enabled { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, - enabled: true, }, log_format: Format::Plain, }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .console_log_settings_builder() .with_log_format(Format::Plain) // color diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index 73ccbda04..cafd925d5 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -54,14 +54,12 @@ mod test { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, - enabled: true, }, file_log_dir: PathBuf::from("/logs"), }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .file_log_settings_builder(PathBuf::from("/logs")) .build(); diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index bb5a1d4ae..58b952996 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -28,12 +28,6 @@ pub struct Settings { /// The [`LevelFilter`] to fallback to if [`Self::environment_variable`] has /// not been set. pub default_level: LevelFilter, - - /// Whether or not the subscriber is enabled. - /// - /// When set to `true`, the [`tracing::Subscriber`] will be added to the - /// [`tracing_subscriber::Layer`] list. - pub enabled: bool, } impl Settings { @@ -52,7 +46,6 @@ impl Default for Settings { /// For building [`Settings`]. pub struct SettingsBuilder { environment_variable: &'static str, - enabled: bool, default_level: LevelFilter, } @@ -75,20 +68,6 @@ impl SettingsBuilder { self } - /// Enable or disable the [`tracing::Subscriber`]. - /// - /// Defaults to `false`. - // TODO (@NickLarsenNZ): Currently this has to be called to enable the - // subscriber. Eventually it should become optional, and default to on (if - // settings are supplied). Therefore, the fields in TracingBuilder to hold - // the subscriber settings should become Option so that the subscriber is - // disabled when not configured, is enabled when configured, while still - // controllable through this function. Then this can be renamed to `with_enabled` - pub fn enabled(mut self, enabled: bool) -> Self { - self.enabled = enabled; - self - } - /// Set specific [`ConsoleLogSettings`]. pub fn console_log_settings_builder(self) -> ConsoleLogSettingsBuilder { self.into() @@ -120,7 +99,6 @@ impl SettingsBuilder { Settings { environment_variable: self.environment_variable, default_level: self.default_level, - enabled: self.enabled, } } } @@ -130,7 +108,6 @@ impl Default for SettingsBuilder { Self { environment_variable: "RUST_LOG", default_level: LevelFilter::OFF, - enabled: false, } } } @@ -144,12 +121,10 @@ mod test { let expected = Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, - enabled: true, }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .build(); assert_eq!(expected, result); diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs index e885faaaa..a39f6391a 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs @@ -47,29 +47,41 @@ impl From for OtlpLogSettings { } } -impl From<(&'static str, LevelFilter)> for OtlpLogSettings { - fn from(value: (&'static str, LevelFilter)) -> Self { - Self { - common_settings: Settings { - environment_variable: value.0, - default_level: value.1, - enabled: true, - }, +impl From> for OtlpLogSettings +where + T: Into, +{ + fn from(settings: Option) -> Self { + match settings { + Some(settings) => settings.into(), + None => OtlpLogSettings::default(), } } } -impl From<(&'static str, LevelFilter, bool)> for OtlpLogSettings { - fn from(value: (&'static str, LevelFilter, bool)) -> Self { - Self { - common_settings: Settings { - environment_variable: value.0, - default_level: value.1, - enabled: value.2, - }, - } - } -} +// impl From<(&'static str, LevelFilter)> for OtlpLogSettings { +// fn from(value: (&'static str, LevelFilter)) -> Self { +// Self { +// common_settings: Settings { +// environment_variable: value.0, +// default_level: value.1, +// enabled: true, +// }, +// } +// } +// } + +// impl From<(&'static str, LevelFilter, bool)> for OtlpLogSettings { +// fn from(value: (&'static str, LevelFilter, bool)) -> Self { +// Self { +// common_settings: Settings { +// environment_variable: value.0, +// default_level: value.1, +// enabled: value.2, +// }, +// } +// } +// } #[cfg(test)] mod test { @@ -83,13 +95,11 @@ mod test { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, - enabled: true, }, }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .otlp_log_settings_builder() .build(); diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs index 98f9ab6fa..db4553857 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs @@ -47,29 +47,41 @@ impl From for OtlpTraceSettings { } } -impl From<(&'static str, LevelFilter)> for OtlpTraceSettings { - fn from(value: (&'static str, LevelFilter)) -> Self { - Self { - common_settings: Settings { - environment_variable: value.0, - default_level: value.1, - enabled: true, - }, +impl From> for OtlpTraceSettings +where + T: Into, +{ + fn from(settings: Option) -> Self { + match settings { + Some(settings) => settings.into(), + None => OtlpTraceSettings::default(), } } } -impl From<(&'static str, LevelFilter, bool)> for OtlpTraceSettings { - fn from(value: (&'static str, LevelFilter, bool)) -> Self { - Self { - common_settings: Settings { - environment_variable: value.0, - default_level: value.1, - enabled: value.2, - }, - } - } -} +// impl From<(&'static str, LevelFilter)> for OtlpTraceSettings { +// fn from(value: (&'static str, LevelFilter)) -> Self { +// Self { +// common_settings: Settings { +// environment_variable: value.0, +// default_level: value.1, +// enabled: true, +// }, +// } +// } +// } + +// impl From<(&'static str, LevelFilter, bool)> for OtlpTraceSettings { +// fn from(value: (&'static str, LevelFilter, bool)) -> Self { +// Self { +// common_settings: Settings { +// environment_variable: value.0, +// default_level: value.1, +// enabled: value.2, +// }, +// } +// } +// } #[cfg(test)] mod test { @@ -83,13 +95,11 @@ mod test { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, - enabled: true, }, }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) - .enabled(true) .otlp_trace_settings_builder() .build(); From c953debd17df71be07cd238d5453a6417f3eb57a Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 28 Jan 2025 16:32:40 +0100 Subject: [PATCH 02/12] wip: Make builder functions not take Option --- crates/stackable-telemetry/src/tracing/mod.rs | 83 +++++++++++-------- .../src/tracing/settings/console_log.rs | 31 +++++-- .../src/tracing/settings/file_log.rs | 53 +++++++++--- .../src/tracing/settings/mod.rs | 5 ++ .../src/tracing/settings/otlp_log.rs | 37 ++++++--- .../src/tracing/settings/otlp_trace.rs | 37 ++++++--- 6 files changed, 169 insertions(+), 77 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index 93af2d09c..c3eb015b3 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -218,9 +218,9 @@ pub enum Error { pub struct Tracing { service_name: &'static str, console_log_settings: ConsoleLogSettings, - file_log_settings: Option, - otlp_log_settings: Option, - otlp_trace_settings: Option, + file_log_settings: FileLogSettings, + otlp_log_settings: OtlpLogSettings, + otlp_trace_settings: OtlpTraceSettings, logger_provider: Option, } @@ -253,10 +253,14 @@ impl Tracing { layers.push(console_output_layer.boxed()); } - if let Some(file_log_settings) = &self.file_log_settings { + if let FileLogSettings::Enabled { + common_settings, + file_log_dir, + } = &self.file_log_settings + { let env_filter_layer = env_filter_builder( - file_log_settings.common_settings.environment_variable, - file_log_settings.default_level, + common_settings.environment_variable, + common_settings.default_level, ); let file_appender = RollingFileAppender::builder() @@ -264,7 +268,7 @@ impl Tracing { .filename_prefix(self.service_name.to_string()) .filename_suffix("tracing-rs.json") .max_log_files(6) - .build(&file_log_settings.file_log_dir) + .build(&file_log_dir) .context(InitRollingFileAppenderSnafu)?; layers.push( @@ -276,10 +280,10 @@ impl Tracing { ); } - if let Some(otlp_log_settings) = &self.otlp_log_settings { + if let OtlpLogSettings::Enabled { common_settings } = &self.otlp_log_settings { let env_filter_layer = env_filter_builder( - otlp_log_settings.environment_variable, - otlp_log_settings.default_level, + common_settings.environment_variable, + common_settings.default_level, ) // TODO (@NickLarsenNZ): Remove this directive once https://github.com/open-telemetry/opentelemetry-rust/issues/761 is resolved .add_directive("h2=off".parse().expect("invalid directive")); @@ -304,10 +308,11 @@ impl Tracing { self.logger_provider = Some(otel_log); } - if let Some(otlp_trace_settings) = &self.otlp_trace_settings { + if let OtlpTraceSettings::Enabled { common_settings } = &self.otlp_trace_settings { let env_filter_layer = env_filter_builder( - otlp_trace_settings.common_settings.environment_variable, - otlp_trace_settings.default_level, + // todo, deref? + common_settings.environment_variable, + common_settings.default_level, ) // TODO (@NickLarsenNZ): Remove this directive once https://github.com/open-telemetry/opentelemetry-rust/issues/761 is resolved .add_directive("h2=off".parse().expect("invalid directive")); @@ -355,12 +360,12 @@ impl Tracing { impl Drop for Tracing { fn drop(&mut self) { tracing::debug!( - opentelemetry.tracing.enabled = self.otlp_trace_settings.is_some(), - opentelemetry.logger.enabled = self.otlp_log_settings.is_some(), + opentelemetry.tracing.enabled = self.otlp_trace_settings.is_enabled(), + opentelemetry.logger.enabled = self.otlp_log_settings.is_enabled(), "shutting down opentelemetry OTLP providers" ); - if self.otlp_trace_settings.is_some() { + if self.otlp_trace_settings.is_enabled() { // NOTE (@NickLarsenNZ): This might eventually be replaced with something like SdkMeterProvider::shutdown(&self) // as has been done with the LoggerProvider (further below) // see: https://github.com/open-telemetry/opentelemetry-rust/pull/1412/files#r1409608679 @@ -434,9 +439,9 @@ impl BuilderState for builder_state::Config {} pub struct TracingBuilder { service_name: Option<&'static str>, console_log_settings: ConsoleLogSettings, - otlp_log_settings: Option, - otlp_trace_settings: Option, - file_log_settings: Option, + file_log_settings: FileLogSettings, + otlp_log_settings: OtlpLogSettings, + otlp_trace_settings: OtlpTraceSettings, /// Allow the generic to be used (needed for impls). _marker: std::marker::PhantomData, @@ -486,7 +491,7 @@ impl TracingBuilder { TracingBuilder { service_name: self.service_name, console_log_settings: self.console_log_settings, - file_log_settings: Some(file_log_settings.into()), + file_log_settings: file_log_settings.into(), otlp_log_settings: self.otlp_log_settings, otlp_trace_settings: self.otlp_trace_settings, _marker: self._marker, @@ -507,7 +512,7 @@ impl TracingBuilder { TracingBuilder { service_name: self.service_name, console_log_settings: self.console_log_settings, - otlp_log_settings: Some(otlp_log_settings.into()), + otlp_log_settings: otlp_log_settings.into(), otlp_trace_settings: self.otlp_trace_settings, file_log_settings: self.file_log_settings, _marker: self._marker, @@ -529,7 +534,7 @@ impl TracingBuilder { service_name: self.service_name, console_log_settings: self.console_log_settings, otlp_log_settings: self.otlp_log_settings, - otlp_trace_settings: Some(otlp_trace_settings.into()), + otlp_trace_settings: otlp_trace_settings.into(), file_log_settings: self.file_log_settings, _marker: self._marker, } @@ -607,8 +612,9 @@ mod test { } ); - assert!(trace_guard.otlp_log_settings.is_none()); - assert!(trace_guard.otlp_trace_settings.is_none()); + assert!(trace_guard.file_log_settings.is_disabled()); + assert!(trace_guard.otlp_log_settings.is_disabled()); + assert!(trace_guard.otlp_trace_settings.is_disabled()); } // #[test] @@ -695,37 +701,38 @@ mod test { ); assert_eq!( trace_guard.file_log_settings, - Some(FileLogSettings { + FileLogSettings::Enabled { common_settings: Settings { environment_variable: "ABC_FILE", default_level: LevelFilter::INFO }, file_log_dir: PathBuf::from("/abc_file_dir") - }) + } ); assert_eq!( trace_guard.otlp_log_settings, - Some(OtlpLogSettings { + OtlpLogSettings::Enabled { common_settings: Settings { environment_variable: "ABC_OTLP_LOG", default_level: LevelFilter::DEBUG }, - }) + } ); assert_eq!( trace_guard.otlp_trace_settings, - Some(OtlpTraceSettings { + OtlpTraceSettings::Enabled { common_settings: Settings { environment_variable: "ABC_OTLP_TRACE", default_level: LevelFilter::TRACE } - }) + } ); } #[test] fn builder_with_options() { let enable_console_output = true; + let enable_filelog_output = true; let enable_otlp_trace = true; let enable_otlp_log = false; @@ -736,6 +743,12 @@ mod test { .with_environment_variable("ABC_CONSOLE") .build() })) + .with_file_output(enable_filelog_output.then(|| { + Settings::builder() + .with_environment_variable("ABC_FILELOG") + .file_log_settings_builder("/dev/null") + .build() + })) .with_otlp_trace_exporter(enable_otlp_trace.then(|| { Settings::builder() .with_environment_variable("ABC_OTLP_TRACE") @@ -748,11 +761,9 @@ mod test { })) .build(); - matches!( - tracing_builder.console_log_settings, - ConsoleLogSettings::Enabled { .. } - ); - assert!(tracing_builder.otlp_trace_settings.is_some()); - // assert!(tracing_builder.otlp_log_settings.is_none()); + assert!(tracing_builder.console_log_settings.is_enabled()); + assert!(tracing_builder.file_log_settings.is_enabled()); + assert!(tracing_builder.otlp_trace_settings.is_enabled()); + assert!(tracing_builder.otlp_log_settings.is_disabled()); } } diff --git a/crates/stackable-telemetry/src/tracing/settings/console_log.rs b/crates/stackable-telemetry/src/tracing/settings/console_log.rs index e3c35f39a..fb9f5b1da 100644 --- a/crates/stackable-telemetry/src/tracing/settings/console_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/console_log.rs @@ -4,7 +4,7 @@ use std::ops::Deref; use tracing::level_filters::LevelFilter; -use super::{Settings, SettingsBuilder}; +use super::{Settings, SettingsBuilder, SettingsToggle}; /// Configure specific settings for the Console Log subscriber. #[derive(Debug, Default, PartialEq)] @@ -21,14 +21,6 @@ pub enum ConsoleLogSettings { }, } -// impl Deref for ConsoleLogSettings { -// type Target = Settings; - -// fn deref(&self) -> &Self::Target { -// &self.common_settings -// } -// } - /// Console Subscriber log event output formats. /// /// Currently, only [Plain][Format::Plain] is supported. @@ -46,6 +38,27 @@ pub enum Format { // LogFmt, } +impl SettingsToggle for ConsoleLogSettings { + fn is_enabled(&self) -> bool { + match self { + ConsoleLogSettings::Disabled => false, + ConsoleLogSettings::Enabled { .. } => true, + } + } + + fn is_disabled(&self) -> bool { + !self.is_enabled() + } +} + +// impl Deref for ConsoleLogSettings { +// type Target = Settings; + +// fn deref(&self) -> &Self::Target { +// &self.common_settings +// } +// } + /// For building [`ConsoleLogSettings`]. /// ///
diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index cafd925d5..e4eebfa29 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -2,26 +2,43 @@ use std::{ops::Deref, path::PathBuf}; -use super::Settings; +use super::{Settings, SettingsToggle}; /// Configure specific settings for the File Log subscriber. #[derive(Debug, Default, PartialEq)] -pub struct FileLogSettings { - /// Common subscriber settings that apply to the File Log Subscriber. - pub common_settings: Settings, +pub enum FileLogSettings { + #[default] + Disabled, + Enabled { + /// Common subscriber settings that apply to the File Log Subscriber. + common_settings: Settings, - /// Path to directory for log files. - pub file_log_dir: PathBuf, + /// Path to directory for log files. + file_log_dir: PathBuf, + }, } -impl Deref for FileLogSettings { - type Target = Settings; +impl SettingsToggle for FileLogSettings { + fn is_enabled(&self) -> bool { + match self { + FileLogSettings::Disabled => false, + FileLogSettings::Enabled { .. } => true, + } + } - fn deref(&self) -> &Self::Target { - &self.common_settings + fn is_disabled(&self) -> bool { + !self.is_enabled() } } +// impl Deref for FileLogSettings { +// type Target = Settings; + +// fn deref(&self) -> &Self::Target { +// &self.common_settings +// } +// } + /// For building [`FileLogSettings`]. /// ///
@@ -35,13 +52,25 @@ pub struct FileLogSettingsBuilder { impl FileLogSettingsBuilder { /// Consumes self and returns a valid [`FileLogSettings`] instance. pub fn build(self) -> FileLogSettings { - FileLogSettings { + FileLogSettings::Enabled { common_settings: self.common_settings, file_log_dir: self.file_log_dir, } } } +impl From> for FileLogSettings +where + T: Into, +{ + fn from(settings: Option) -> Self { + match settings { + Some(settings) => settings.into(), + None => FileLogSettings::default(), + } + } +} + #[cfg(test)] mod test { use tracing::level_filters::LevelFilter; @@ -50,7 +79,7 @@ mod test { #[test] fn builds_settings() { - let expected = FileLogSettings { + let expected = FileLogSettings::Enabled { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index 58b952996..4180ffb06 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -16,6 +16,11 @@ pub use otlp_log::*; pub mod otlp_trace; pub use otlp_trace::*; +pub trait SettingsToggle { + fn is_enabled(&self) -> bool; + fn is_disabled(&self) -> bool; +} + /// General settings that apply to any subscriber. #[derive(Debug, PartialEq)] pub struct Settings { diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs index a39f6391a..6ff11a4aa 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs @@ -4,28 +4,45 @@ use std::ops::Deref; use tracing::level_filters::LevelFilter; -use super::{Settings, SettingsBuilder}; +use super::{Settings, SettingsBuilder, SettingsToggle}; #[derive(Debug, Default, PartialEq)] -pub struct OtlpLogSettings { - pub common_settings: Settings, +pub enum OtlpLogSettings { + #[default] + Disabled, + Enabled { + common_settings: Settings, + }, } -impl Deref for OtlpLogSettings { - type Target = Settings; +impl SettingsToggle for OtlpLogSettings { + fn is_enabled(&self) -> bool { + match self { + OtlpLogSettings::Disabled => false, + OtlpLogSettings::Enabled { .. } => true, + } + } - fn deref(&self) -> &Self::Target { - &self.common_settings + fn is_disabled(&self) -> bool { + !self.is_enabled() } } +// impl Deref for OtlpLogSettings { +// type Target = Settings; + +// fn deref(&self) -> &Self::Target { +// &self.common_settings +// } +// } + pub struct OtlpLogSettingsBuilder { pub(crate) common_settings: Settings, } impl OtlpLogSettingsBuilder { pub fn build(self) -> OtlpLogSettings { - OtlpLogSettings { + OtlpLogSettings::Enabled { common_settings: self.common_settings, } } @@ -43,7 +60,7 @@ impl From for OtlpLogSettingsBuilder { impl From for OtlpLogSettings { fn from(common_settings: Settings) -> Self { - Self { common_settings } + Self::Enabled { common_settings } } } @@ -91,7 +108,7 @@ mod test { #[test] fn builds_settings() { - let expected = OtlpLogSettings { + let expected = OtlpLogSettings::Enabled { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs index db4553857..f912fe005 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs @@ -4,28 +4,45 @@ use std::ops::Deref; use tracing::level_filters::LevelFilter; -use super::{Settings, SettingsBuilder}; +use super::{Settings, SettingsBuilder, SettingsToggle}; #[derive(Debug, Default, PartialEq)] -pub struct OtlpTraceSettings { - pub common_settings: Settings, +pub enum OtlpTraceSettings { + #[default] + Disabled, + Enabled { + common_settings: Settings, + }, } -impl Deref for OtlpTraceSettings { - type Target = Settings; +impl SettingsToggle for OtlpTraceSettings { + fn is_enabled(&self) -> bool { + match self { + OtlpTraceSettings::Disabled => false, + OtlpTraceSettings::Enabled { .. } => true, + } + } - fn deref(&self) -> &Self::Target { - &self.common_settings + fn is_disabled(&self) -> bool { + !self.is_enabled() } } +// impl Deref for OtlpTraceSettings { +// type Target = Settings; + +// fn deref(&self) -> &Self::Target { +// &self.common_settings +// } +// } + pub struct OtlpTraceSettingsBuilder { pub(crate) common_settings: Settings, } impl OtlpTraceSettingsBuilder { pub fn build(self) -> OtlpTraceSettings { - OtlpTraceSettings { + OtlpTraceSettings::Enabled { common_settings: self.common_settings, } } @@ -43,7 +60,7 @@ impl From for OtlpTraceSettingsBuilder { impl From for OtlpTraceSettings { fn from(common_settings: Settings) -> Self { - Self { common_settings } + Self::Enabled { common_settings } } } @@ -91,7 +108,7 @@ mod test { #[test] fn builds_settings() { - let expected = OtlpTraceSettings { + let expected = OtlpTraceSettings::Enabled { common_settings: Settings { environment_variable: "hello", default_level: LevelFilter::DEBUG, From 7b80f17a3edccfafc11591bafe7c6da6e5894753 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 28 Jan 2025 16:44:39 +0100 Subject: [PATCH 03/12] wip: remove derefs --- .../src/tracing/settings/console_log.rs | 10 ---------- .../src/tracing/settings/file_log.rs | 10 +--------- .../src/tracing/settings/otlp_log.rs | 10 ---------- .../src/tracing/settings/otlp_trace.rs | 10 ---------- 4 files changed, 1 insertion(+), 39 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/settings/console_log.rs b/crates/stackable-telemetry/src/tracing/settings/console_log.rs index fb9f5b1da..d70e2ece3 100644 --- a/crates/stackable-telemetry/src/tracing/settings/console_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/console_log.rs @@ -1,7 +1,5 @@ //! Console Log Subscriber Settings. -use std::ops::Deref; - use tracing::level_filters::LevelFilter; use super::{Settings, SettingsBuilder, SettingsToggle}; @@ -51,14 +49,6 @@ impl SettingsToggle for ConsoleLogSettings { } } -// impl Deref for ConsoleLogSettings { -// type Target = Settings; - -// fn deref(&self) -> &Self::Target { -// &self.common_settings -// } -// } - /// For building [`ConsoleLogSettings`]. /// ///
diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index e4eebfa29..92a7c65c8 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -1,6 +1,6 @@ //! File Log Subscriber Settings. -use std::{ops::Deref, path::PathBuf}; +use std::path::PathBuf; use super::{Settings, SettingsToggle}; @@ -31,14 +31,6 @@ impl SettingsToggle for FileLogSettings { } } -// impl Deref for FileLogSettings { -// type Target = Settings; - -// fn deref(&self) -> &Self::Target { -// &self.common_settings -// } -// } - /// For building [`FileLogSettings`]. /// ///
diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs index 6ff11a4aa..716aa0bb2 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs @@ -1,7 +1,5 @@ //! OTLP Log Subscriber Settings. -use std::ops::Deref; - use tracing::level_filters::LevelFilter; use super::{Settings, SettingsBuilder, SettingsToggle}; @@ -28,14 +26,6 @@ impl SettingsToggle for OtlpLogSettings { } } -// impl Deref for OtlpLogSettings { -// type Target = Settings; - -// fn deref(&self) -> &Self::Target { -// &self.common_settings -// } -// } - pub struct OtlpLogSettingsBuilder { pub(crate) common_settings: Settings, } diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs index f912fe005..426d9bbf4 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs @@ -1,7 +1,5 @@ //! OTLP Trace Subscriber Settings. -use std::ops::Deref; - use tracing::level_filters::LevelFilter; use super::{Settings, SettingsBuilder, SettingsToggle}; @@ -28,14 +26,6 @@ impl SettingsToggle for OtlpTraceSettings { } } -// impl Deref for OtlpTraceSettings { -// type Target = Settings; - -// fn deref(&self) -> &Self::Target { -// &self.common_settings -// } -// } - pub struct OtlpTraceSettingsBuilder { pub(crate) common_settings: Settings, } From 97b9f53b86a4c23b369f9223d41ae922f079b406 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 28 Jan 2025 16:45:12 +0100 Subject: [PATCH 04/12] wip: restore the tuple impls and tests --- crates/stackable-telemetry/src/tracing/mod.rs | 81 ++++++++++--------- .../src/tracing/settings/console_log.rs | 51 ++++++------ .../src/tracing/settings/otlp_log.rs | 47 +++++------ .../src/tracing/settings/otlp_trace.rs | 47 +++++------ 4 files changed, 115 insertions(+), 111 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index c3eb015b3..471dc84d2 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -617,46 +617,47 @@ mod test { assert!(trace_guard.otlp_trace_settings.is_disabled()); } - // #[test] - // fn builder_with_console_output_double() { - // let trace_guard = Tracing::builder() - // .service_name("test") - // .with_console_output(("ABC_A", LevelFilter::TRACE)) - // .build(); - - // assert_eq!( - // trace_guard.console_log_settings, - // ConsoleLogSettings { - // common_settings: Settings { - // environment_variable: "ABC_A", - // default_level: LevelFilter::TRACE, - // }, - // log_format: Default::default() - // } - // ) - // } - - // #[rstest] - // #[case(false)] - // #[case(true)] - // fn builder_with_console_output_triple(#[case] enabled: bool) { - // let trace_guard = Tracing::builder() - // .service_name("test") - // .with_console_output(("ABC_A", LevelFilter::TRACE, enabled)) - // .build(); - - // assert_eq!( - // trace_guard.console_log_settings, - // ConsoleLogSettings { - // common_settings: Settings { - // environment_variable: "ABC_A", - // default_level: LevelFilter::TRACE, - // enabled - // }, - // log_format: Default::default() - // } - // ) - // } + #[test] + fn builder_with_console_output_double() { + let trace_guard = Tracing::builder() + .service_name("test") + .with_console_output(("ABC_A", LevelFilter::TRACE)) + .build(); + + assert_eq!( + trace_guard.console_log_settings, + ConsoleLogSettings::Enabled { + common_settings: Settings { + environment_variable: "ABC_A", + default_level: LevelFilter::TRACE, + }, + log_format: Default::default() + } + ) + } + + #[rstest] + #[case(false)] + #[case(true)] + fn builder_with_console_output_triple(#[case] enabled: bool) { + let trace_guard = Tracing::builder() + .service_name("test") + .with_console_output(("ABC_A", LevelFilter::TRACE, enabled)) + .build(); + + let expected = match enabled { + true => ConsoleLogSettings::Enabled { + common_settings: Settings { + environment_variable: "ABC_A", + default_level: LevelFilter::TRACE, + }, + log_format: Default::default(), + }, + false => ConsoleLogSettings::Disabled, + }; + + assert_eq!(trace_guard.console_log_settings, expected) + } #[test] fn builder_with_all() { diff --git a/crates/stackable-telemetry/src/tracing/settings/console_log.rs b/crates/stackable-telemetry/src/tracing/settings/console_log.rs index d70e2ece3..7b38cd3b2 100644 --- a/crates/stackable-telemetry/src/tracing/settings/console_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/console_log.rs @@ -105,31 +105,32 @@ where } } -// impl From<(&'static str, LevelFilter)> for ConsoleLogSettings { -// fn from(value: (&'static str, LevelFilter)) -> Self { -// Self { -// common_settings: Settings { -// environment_variable: value.0, -// default_level: value.1, -// enabled: true, -// }, -// ..Default::default() -// } -// } -// } - -// impl From<(&'static str, LevelFilter, bool)> for ConsoleLogSettings { -// fn from(value: (&'static str, LevelFilter, bool)) -> Self { -// Self { -// common_settings: Settings { -// environment_variable: value.0, -// default_level: value.1, -// enabled: value.2, -// }, -// ..Default::default() -// } -// } -// } +impl From<(&'static str, LevelFilter)> for ConsoleLogSettings { + fn from(value: (&'static str, LevelFilter)) -> Self { + Self::Enabled { + common_settings: Settings { + environment_variable: value.0, + default_level: value.1, + }, + log_format: Default::default(), + } + } +} + +impl From<(&'static str, LevelFilter, bool)> for ConsoleLogSettings { + fn from(value: (&'static str, LevelFilter, bool)) -> Self { + match value.2 { + true => Self::Enabled { + common_settings: Settings { + environment_variable: value.0, + default_level: value.1, + }, + log_format: Default::default(), + }, + false => Self::Disabled, + } + } +} #[cfg(test)] mod test { diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs index 716aa0bb2..91576e69e 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs @@ -66,29 +66,30 @@ where } } -// impl From<(&'static str, LevelFilter)> for OtlpLogSettings { -// fn from(value: (&'static str, LevelFilter)) -> Self { -// Self { -// common_settings: Settings { -// environment_variable: value.0, -// default_level: value.1, -// enabled: true, -// }, -// } -// } -// } - -// impl From<(&'static str, LevelFilter, bool)> for OtlpLogSettings { -// fn from(value: (&'static str, LevelFilter, bool)) -> Self { -// Self { -// common_settings: Settings { -// environment_variable: value.0, -// default_level: value.1, -// enabled: value.2, -// }, -// } -// } -// } +impl From<(&'static str, LevelFilter)> for OtlpLogSettings { + fn from(value: (&'static str, LevelFilter)) -> Self { + Self::Enabled { + common_settings: Settings { + environment_variable: value.0, + default_level: value.1, + }, + } + } +} + +impl From<(&'static str, LevelFilter, bool)> for OtlpLogSettings { + fn from(value: (&'static str, LevelFilter, bool)) -> Self { + match value.2 { + true => Self::Enabled { + common_settings: Settings { + environment_variable: value.0, + default_level: value.1, + }, + }, + false => Self::Disabled, + } + } +} #[cfg(test)] mod test { diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs index 426d9bbf4..bef912235 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs @@ -66,29 +66,30 @@ where } } -// impl From<(&'static str, LevelFilter)> for OtlpTraceSettings { -// fn from(value: (&'static str, LevelFilter)) -> Self { -// Self { -// common_settings: Settings { -// environment_variable: value.0, -// default_level: value.1, -// enabled: true, -// }, -// } -// } -// } - -// impl From<(&'static str, LevelFilter, bool)> for OtlpTraceSettings { -// fn from(value: (&'static str, LevelFilter, bool)) -> Self { -// Self { -// common_settings: Settings { -// environment_variable: value.0, -// default_level: value.1, -// enabled: value.2, -// }, -// } -// } -// } +impl From<(&'static str, LevelFilter)> for OtlpTraceSettings { + fn from(value: (&'static str, LevelFilter)) -> Self { + Self::Enabled { + common_settings: Settings { + environment_variable: value.0, + default_level: value.1, + }, + } + } +} + +impl From<(&'static str, LevelFilter, bool)> for OtlpTraceSettings { + fn from(value: (&'static str, LevelFilter, bool)) -> Self { + match value.2 { + true => Self::Enabled { + common_settings: Settings { + environment_variable: value.0, + default_level: value.1, + }, + }, + false => Self::Disabled, + } + } +} #[cfg(test)] mod test { From f2ca98decdd1fb3d2ce0e82512f2c40964eb95d3 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 28 Jan 2025 16:56:13 +0100 Subject: [PATCH 05/12] wip: fix example in docs --- crates/stackable-telemetry/src/tracing/mod.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index 471dc84d2..280484f82 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -121,6 +121,7 @@ pub enum Error { /// # use tracing_subscriber::filter::LevelFilter; /// #[tokio::main] /// async fn main() -> Result<(), Error> { +/// // Control the otlp_log subscriber at runtime /// let otlp_log_flag = false; /// /// let _tracing_guard = Tracing::builder() @@ -129,21 +130,25 @@ pub enum Error { /// Settings::builder() /// .with_environment_variable("TEST_CONSOLE") /// .with_default_level(LevelFilter::INFO) -/// .enabled(true) /// .build() /// ) -/// .with_otlp_log_exporter( +/// .with_file_output( +/// Settings::builder() +/// .with_environment_variable("TEST_FILE_LOG") +/// .with_default_level(LevelFilter::INFO) +/// .file_log_settings_builder("/tmp/logs") +/// .build() +/// ) +/// .with_otlp_log_exporter(otlp_log_flag.then(|| { /// Settings::builder() /// .with_environment_variable("TEST_OTLP_LOG") /// .with_default_level(LevelFilter::DEBUG) -/// .enabled(otlp_log_flag) /// .build() -/// ) +/// })) /// .with_otlp_trace_exporter( /// Settings::builder() /// .with_environment_variable("TEST_OTLP_TRACE") /// .with_default_level(LevelFilter::TRACE) -/// .enabled(true) /// .build() /// ) /// .build() From 85f8449a6ee446bf6d71297e24f7e2f11afc424f Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 28 Jan 2025 17:04:51 +0100 Subject: [PATCH 06/12] chore: fix word tense in past entry --- crates/stackable-telemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-telemetry/CHANGELOG.md b/crates/stackable-telemetry/CHANGELOG.md index f2162a8d3..cd99107a3 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -11,7 +11,7 @@ All notable changes to this project will be documented in this file. ### Changed -- BREAKING: Renamed `TracingBuilder` methods with long names, and prefix with `with_` ([#901]). +- BREAKING: Rename `TracingBuilder` methods with long names, and prefix with `with_` ([#901]). - BREAKING: Use the new subscriber settings in the `TracingBuilder` ([#901]). [#901]: https://github.com/stackabletech/operator-rs/pull/901 From 3a46636110c33248218d31ecf898c499bcf6a536 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 28 Jan 2025 17:08:19 +0100 Subject: [PATCH 07/12] chore: add changelog entries --- crates/stackable-telemetry/CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/stackable-telemetry/CHANGELOG.md b/crates/stackable-telemetry/CHANGELOG.md index cd99107a3..6757d230d 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -6,16 +6,23 @@ All notable changes to this project will be documented in this file. ### Added +- Allow `Option<_>` to be used to enable/disable a subscriber ([#951]). - Introduce common `Settings` and subscriber specific settings ([#901]). - Add support for logging to files ([#933]). ### Changed +- BREAKING: Change subscriber settings into an enum to indicate if the subscriber is enabled/disabled ([#951]). - BREAKING: Rename `TracingBuilder` methods with long names, and prefix with `with_` ([#901]). - BREAKING: Use the new subscriber settings in the `TracingBuilder` ([#901]). +## Removed + +- BREAKING: Remove `Deref` impls for subscriber settings and removed the `enabled` fields and `enabled()` methods ([#951]). + [#901]: https://github.com/stackabletech/operator-rs/pull/901 [#933]: https://github.com/stackabletech/operator-rs/pull/933 +[#951]: https://github.com/stackabletech/operator-rs/pull/951 ## [0.2.0] - 2024-07-10 From 0087662d897c802898e8ff584697792894a53a3d Mon Sep 17 00:00:00 2001 From: Nick <10092581+NickLarsenNZ@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:10:40 +0100 Subject: [PATCH 08/12] Apply suggestions from code review Co-authored-by: Techassi --- crates/stackable-telemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-telemetry/CHANGELOG.md b/crates/stackable-telemetry/CHANGELOG.md index 6757d230d..0cde20859 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -16,7 +16,7 @@ All notable changes to this project will be documented in this file. - BREAKING: Rename `TracingBuilder` methods with long names, and prefix with `with_` ([#901]). - BREAKING: Use the new subscriber settings in the `TracingBuilder` ([#901]). -## Removed +### Removed - BREAKING: Remove `Deref` impls for subscriber settings and removed the `enabled` fields and `enabled()` methods ([#951]). From 399b293be1369b3c2941ffaa450cccaa45bcd0b0 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 29 Jan 2025 10:40:38 +0100 Subject: [PATCH 09/12] chore: use default impl for SettingToggle::is_disabled() --- .../stackable-telemetry/src/tracing/settings/console_log.rs | 4 ---- crates/stackable-telemetry/src/tracing/settings/file_log.rs | 4 ---- crates/stackable-telemetry/src/tracing/settings/mod.rs | 4 +++- crates/stackable-telemetry/src/tracing/settings/otlp_log.rs | 4 ---- crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs | 4 ---- 5 files changed, 3 insertions(+), 17 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/settings/console_log.rs b/crates/stackable-telemetry/src/tracing/settings/console_log.rs index 7b38cd3b2..84a92c9e8 100644 --- a/crates/stackable-telemetry/src/tracing/settings/console_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/console_log.rs @@ -43,10 +43,6 @@ impl SettingsToggle for ConsoleLogSettings { ConsoleLogSettings::Enabled { .. } => true, } } - - fn is_disabled(&self) -> bool { - !self.is_enabled() - } } /// For building [`ConsoleLogSettings`]. diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index 92a7c65c8..4be58f931 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -25,10 +25,6 @@ impl SettingsToggle for FileLogSettings { FileLogSettings::Enabled { .. } => true, } } - - fn is_disabled(&self) -> bool { - !self.is_enabled() - } } /// For building [`FileLogSettings`]. diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index 4180ffb06..1cc869f9b 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -18,7 +18,9 @@ pub use otlp_trace::*; pub trait SettingsToggle { fn is_enabled(&self) -> bool; - fn is_disabled(&self) -> bool; + fn is_disabled(&self) -> bool { + !self.is_enabled() + } } /// General settings that apply to any subscriber. diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs index 91576e69e..37f9c7741 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_log.rs @@ -20,10 +20,6 @@ impl SettingsToggle for OtlpLogSettings { OtlpLogSettings::Enabled { .. } => true, } } - - fn is_disabled(&self) -> bool { - !self.is_enabled() - } } pub struct OtlpLogSettingsBuilder { diff --git a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs index bef912235..851f52113 100644 --- a/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs +++ b/crates/stackable-telemetry/src/tracing/settings/otlp_trace.rs @@ -20,10 +20,6 @@ impl SettingsToggle for OtlpTraceSettings { OtlpTraceSettings::Enabled { .. } => true, } } - - fn is_disabled(&self) -> bool { - !self.is_enabled() - } } pub struct OtlpTraceSettingsBuilder { From 53be0d1447a2c944e64ac0f164d55d49c7da87e1 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 29 Jan 2025 10:48:00 +0100 Subject: [PATCH 10/12] chore: fix clippy warning --- crates/stackable-telemetry/src/tracing/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index 280484f82..acee235aa 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -273,7 +273,7 @@ impl Tracing { .filename_prefix(self.service_name.to_string()) .filename_suffix("tracing-rs.json") .max_log_files(6) - .build(&file_log_dir) + .build(file_log_dir) .context(InitRollingFileAppenderSnafu)?; layers.push( From 935012f45e47c45758b16d4ca394964927a8242f Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Mon, 24 Feb 2025 11:29:51 +0100 Subject: [PATCH 11/12] chore(stackable-telemetry): add missing docs --- crates/stackable-telemetry/src/tracing/settings/file_log.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index 4be58f931..53c326c4e 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -7,8 +7,11 @@ use super::{Settings, SettingsToggle}; /// Configure specific settings for the File Log subscriber. #[derive(Debug, Default, PartialEq)] pub enum FileLogSettings { + /// File Log subscriber disabled. #[default] Disabled, + + /// File Log subscriber enabled. Enabled { /// Common subscriber settings that apply to the File Log Subscriber. common_settings: Settings, From ef7319b0a4313f6a89d7740e2a83a9f7e6b57901 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Mon, 24 Feb 2025 11:47:01 +0100 Subject: [PATCH 12/12] chore)stackable-telemetry): add missing docs --- crates/stackable-telemetry/src/tracing/settings/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index 1cc869f9b..cee7a9328 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -16,8 +16,12 @@ pub use otlp_log::*; pub mod otlp_trace; pub use otlp_trace::*; +/// Indicate whether a type is enabled or disabled. pub trait SettingsToggle { + /// Whether the settings are enabled or not. fn is_enabled(&self) -> bool; + + /// The opposite of [SettingsToggle::is_enabled] as a helper. fn is_disabled(&self) -> bool { !self.is_enabled() }