wip: Make builder functions not take Option

Author: NickLarsenNZ

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<FileLogSettings>,
-    otlp_log_settings: Option<OtlpLogSettings>,
-    otlp_trace_settings: Option<OtlpTraceSettings>,
+    file_log_settings: FileLogSettings,
+    otlp_log_settings: OtlpLogSettings,
+    otlp_trace_settings: OtlpTraceSettings,
     logger_provider: Option<LoggerProvider>,
 }
 
@@ -253,18 +253,22 @@ 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()
                 .rotation(Rotation::HOURLY)
                 .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<S: BuilderState> {
     service_name: Option<&'static str>,
     console_log_settings: ConsoleLogSettings,
-    otlp_log_settings: Option<OtlpLogSettings>,
-    otlp_trace_settings: Option<OtlpTraceSettings>,
-    file_log_settings: Option<FileLogSettings>,
+    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<S>,
@@ -486,7 +491,7 @@ impl TracingBuilder<builder_state::Config> {
         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<builder_state::Config> {
         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<builder_state::Config> {
             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());
     }
 }

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`].
 ///
 /// <div class="warning">

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`].
 ///
 /// <div class="warning">
@@ -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<T> From<Option<T>> for FileLogSettings
+where
+    T: Into<FileLogSettings>,
+{
+    fn from(settings: Option<T>) -> 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,

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 {