error handling, simplify filelogsettings creation

crates/stackable-telemetry/CHANGELOG.md

@@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file.
 ### Added
 
 - Introduce common `Settings` and subscriber specific settings ([#901]).
-- Added support for logging to files ([#933]).
+- Add support for logging to files ([#933]).
 
 ### Changed
 

crates/stackable-telemetry/src/tracing/mod.rs

@@ -1,5 +1,5 @@
 //! This module contains functionality to initialise tracing Subscribers for
-//! console output, file output and OpenTelemetry OTLP export for traces and logs.
+//! console output, file output, and OpenTelemetry OTLP export for traces and logs.
 //!
 //! It is intended to be used by the Stackable Data Platform operators and
 //! webhooks, but it should be generic enough to be used in any application.
@@ -16,7 +16,7 @@ use opentelemetry_sdk::{
 use opentelemetry_semantic_conventions::resource;
 use snafu::{ResultExt as _, Snafu};
 use tracing::subscriber::SetGlobalDefaultError;
-use tracing_appender::rolling::{RollingFileAppender, Rotation};
+use tracing_appender::rolling::{InitError, RollingFileAppender, Rotation};
 use tracing_subscriber::{filter::Directive, layer::SubscriberExt, EnvFilter, Layer, Registry};
 
 use settings::{ConsoleLogSettings, FileLogSettings, OtlpLogSettings, OtlpTraceSettings};
@@ -39,6 +39,9 @@ pub enum Error {
 
     #[snafu(display("unable to set the global default subscriber"))]
     SetGlobalDefaultSubscriber { source: SetGlobalDefaultError },
+
+    #[snafu(display("failed to initialize rolling file appender"))]
+    InitRollingFileAppender { source: InitError },
 }
 
 /// Easily initialize a set of preconfigured [`Subscriber`][1] layers.
@@ -195,7 +198,7 @@ impl Tracing {
                 .filename_suffix("tracing-rs.json")
                 .max_log_files(6)
                 .build(&self.file_log_settings.file_log_dir)
-                .expect("failed to initialize rolling file appender");
+                .context(InitRollingFileAppenderSnafu)?;
 
             layers.push(
                 tracing_subscriber::fmt::layer()
@@ -560,8 +563,7 @@ mod test {
                     .with_environment_variable("ABC_FILE")
                     .with_default_level(LevelFilter::INFO)
                     .enabled(true)
-                    .file_log_settings_builder()
-                    .with_file_log_dir(String::from("/abc_file_dir"))
+                    .file_log_settings_builder(String::from("/abc_file_dir"))
                     .build(),
             )
             .with_otlp_log_exporter(

crates/stackable-telemetry/src/tracing/settings/file_log.rs

@@ -2,7 +2,7 @@
 
 use std::{ops::Deref, path::PathBuf};
 
-use super::{Build, Settings, SettingsBuilder};
+use super::Settings;
 
 /// Configure specific settings for the File Log subscriber.
 #[derive(Debug, Default, PartialEq)]
@@ -22,106 +22,22 @@ impl Deref for FileLogSettings {
     }
 }
 
-/// This trait is only used for the typestate builder and cannot be implemented
-/// outside of this crate.
-///
-/// The only reason it has pub visibility is because it needs to be at least as
-/// visible as the types that use it.
-#[doc(hidden)]
-pub trait BuilderState: private::Sealed {}
-
-/// This private module holds the [`Sealed`][1] trait that is used by the
-/// [`BuilderState`], so that it cannot be implemented outside of this crate.
-///
-/// We impl Sealed for any types that will use the trait that we want to
-/// restrict impls on. In this case, the [`BuilderState`] trait.
-///
-/// [1]: private::Sealed
-#[doc(hidden)]
-mod private {
-    use super::*;
-
-    pub trait Sealed {}
-
-    impl Sealed for builder_state::PreLogDir {}
-    impl Sealed for builder_state::Config {}
-}
-
-/// This module holds the possible states that the builder is in.
-///
-/// Each state will implement [`BuilderState`] (with no methods), and the
-/// Builder struct ([`FileLogSettingsBuilder`][1]) itself will be implemented with
-/// each state as a generic parameter.
-/// This allows only the methods to be called when the builder is in the
-/// applicable state.
-///
-/// [1]: super::FileLogSettingsBuilder
-#[doc(hidden)]
-pub mod builder_state {
-    /// The initial state, before the log directory path is set.
-    #[derive(Default)]
-    pub struct PreLogDir;
-
-    /// The state that allows you to configure the [`FileLogSettings`][1].
-    ///
-    /// [1]: super::FileLogSettings
-    #[derive(Default)]
-    pub struct Config;
-}
-
-// Make the states usable
-#[doc(hidden)]
-impl BuilderState for builder_state::PreLogDir {}
-
-#[doc(hidden)]
-impl BuilderState for builder_state::Config {}
-
 /// For building [`FileLogSettings`].
 ///
 /// <div class="warning">
 /// Do not use directly, instead use the [`Settings::builder`] associated function.
 /// </div>
-pub struct FileLogSettingsBuilder<S: BuilderState> {
+pub struct FileLogSettingsBuilder {
     pub(crate) common_settings: Settings,
-    pub(crate) file_log_dir: Option<PathBuf>,
-
-    /// Allow the generic to be used (needed for impls).
-    _marker: std::marker::PhantomData<S>,
+    pub(crate) file_log_dir: PathBuf,
 }
 
-impl FileLogSettingsBuilder<builder_state::PreLogDir> {
-    /// Set the directory for log files.
-    ///
-    /// A directory is required for using the File Log subscriber.
-    pub fn with_file_log_dir(self, path: String) -> FileLogSettingsBuilder<builder_state::Config> {
-        FileLogSettingsBuilder {
-            common_settings: self.common_settings,
-            file_log_dir: Some(PathBuf::from(path)),
-            _marker: std::marker::PhantomData,
-        }
-    }
-}
-
-impl FileLogSettingsBuilder<builder_state::Config> {
+impl FileLogSettingsBuilder {
     /// Consumes self and returns a valid [`FileLogSettings`] instance.
     pub fn build(self) -> FileLogSettings {
         FileLogSettings {
             common_settings: self.common_settings,
-            file_log_dir: self
-                .file_log_dir
-                .expect("file_log_dir must be configured at this point"),
-        }
-    }
-}
-
-/// This implementation is used to turn the common settings builder into the file log specific
-/// settings builder via the [`SettingsBuilder::file_log_settings_builder`] function.
-impl From<SettingsBuilder> for FileLogSettingsBuilder<builder_state::PreLogDir> {
-    fn from(value: SettingsBuilder) -> Self {
-        Self {
-            common_settings: value.build(),
-            file_log_dir: None,
-            _marker: std::marker::PhantomData,
+            file_log_dir: self.file_log_dir,
         }
     }
 }
@@ -146,8 +62,7 @@ mod test {
             .with_environment_variable("hello")
             .with_default_level(LevelFilter::DEBUG)
             .enabled(true)
-            .file_log_settings_builder()
-            .with_file_log_dir(String::from("/logs"))
+            .file_log_settings_builder(String::from("/logs"))
             .build();
 
         assert_eq!(expected, result);

crates/stackable-telemetry/src/tracing/settings/mod.rs

@@ -1,5 +1,7 @@
 //! Subscriber settings.
 
+use std::path::Path;
+
 use tracing::level_filters::LevelFilter;
 
 pub mod console_log;
@@ -109,8 +111,14 @@ impl SettingsBuilder {
     }
 
     /// Set specific [`FileLogSettings`].
-    pub fn file_log_settings_builder(self) -> FileLogSettingsBuilder<builder_state::PreLogDir> {
-        self.into()
+    pub fn file_log_settings_builder<P>(self, path: P) -> FileLogSettingsBuilder
+    where
+        P: AsRef<Path>,
+    {
+        FileLogSettingsBuilder {
+            common_settings: self.build(),
+            file_log_dir: path.as_ref().to_path_buf(),
+        }
     }
 
     /// Set specific [`OtlpLogSettings`].