From b03db8a3cad3b90724516d36bb16f401564fe3b1 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 12:38:05 +0200 Subject: [PATCH 1/8] feat(stackable-telemetry): Allow customization of the rolling file appender rotation period --- crates/stackable-telemetry/src/tracing/mod.rs | 9 ++++++--- .../src/tracing/settings/file_log.rs | 16 ++++++++++++++++ .../src/tracing/settings/mod.rs | 1 + 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index cf00503b7..41ac38501 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -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::{InitError, RollingFileAppender, Rotation}; +use tracing_appender::rolling::{InitError, RollingFileAppender}; use tracing_subscriber::{filter::Directive, layer::SubscriberExt, EnvFilter, Layer, Registry}; use crate::tracing::settings::*; @@ -275,6 +275,7 @@ impl Tracing { if let FileLogSettings::Enabled { common_settings, file_log_dir, + rotation_period, } = &self.file_log_settings { let env_filter_layer = env_filter_builder( @@ -283,7 +284,7 @@ impl Tracing { ); let file_appender = RollingFileAppender::builder() - .rotation(Rotation::HOURLY) + .rotation(rotation_period.clone()) .filename_prefix(self.service_name.to_string()) .filename_suffix("tracing-rs.json") .max_log_files(6) @@ -592,6 +593,7 @@ mod test { use rstest::rstest; use settings::Settings; use tracing::level_filters::LevelFilter; + use tracing_appender::rolling::Rotation; use super::*; @@ -726,7 +728,8 @@ mod test { environment_variable: "ABC_FILE", default_level: LevelFilter::INFO }, - file_log_dir: PathBuf::from("/abc_file_dir") + file_log_dir: PathBuf::from("/abc_file_dir"), + rotation_period: Rotation::NEVER, } ); assert_eq!( diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index 53c326c4e..1390835b9 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -2,6 +2,9 @@ use std::path::PathBuf; +/// Re-export to save the end crate +pub use tracing_appender::rolling::Rotation; + use super::{Settings, SettingsToggle}; /// Configure specific settings for the File Log subscriber. @@ -18,6 +21,9 @@ pub enum FileLogSettings { /// Path to directory for log files. file_log_dir: PathBuf, + + /// Log rotation frequency. + rotation_period: Rotation, }, } @@ -38,14 +44,22 @@ impl SettingsToggle for FileLogSettings { pub struct FileLogSettingsBuilder { pub(crate) common_settings: Settings, pub(crate) file_log_dir: PathBuf, + pub(crate) rotation_period: Rotation, } impl FileLogSettingsBuilder { + /// Set file rotation period. + pub fn with_rotation_period(mut self, rotation_period: Rotation) -> Self { + self.rotation_period = rotation_period; + self + } + /// Consumes self and returns a valid [`FileLogSettings`] instance. pub fn build(self) -> FileLogSettings { FileLogSettings::Enabled { common_settings: self.common_settings, file_log_dir: self.file_log_dir, + rotation_period: self.rotation_period, } } } @@ -76,11 +90,13 @@ mod test { default_level: LevelFilter::DEBUG, }, file_log_dir: PathBuf::from("/logs"), + rotation_period: Rotation::HOURLY, }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) .file_log_settings_builder(PathBuf::from("/logs")) + .with_rotation_period(Rotation::HOURLY) .build(); assert_eq!(expected, result); diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index cee7a9328..fe6082a53 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -92,6 +92,7 @@ impl SettingsBuilder { FileLogSettingsBuilder { common_settings: self.build(), file_log_dir: path.as_ref().to_path_buf(), + rotation_period: Rotation::NEVER, } } From 560edb3dd66f32cc52b1c3f1a4178d745d49ae19 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 14:19:50 +0200 Subject: [PATCH 2/8] chore(stackable-telemetry): Update changelog --- crates/stackable-telemetry/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/stackable-telemetry/CHANGELOG.md b/crates/stackable-telemetry/CHANGELOG.md index a7935073a..48f5b5237 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Allow customization of the rolling file appender rotation period [#995]. + +[#995]: https://github.com/stackabletech/operator-rs/pull/995 + ## [0.3.0] - 2025-01-30 ### Added From 363f4926a8fb21bf5ca9289280e77bd37ecf50fd Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 15:25:15 +0200 Subject: [PATCH 3/8] feat(stackable-telemetry): Add required file appender filename_suffix field --- crates/stackable-telemetry/CHANGELOG.md | 4 +++- crates/stackable-telemetry/src/tracing/mod.rs | 8 +++++--- .../stackable-telemetry/src/tracing/settings/file_log.rs | 8 +++++++- crates/stackable-telemetry/src/tracing/settings/mod.rs | 7 ++++++- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/stackable-telemetry/CHANGELOG.md b/crates/stackable-telemetry/CHANGELOG.md index 48f5b5237..80e516198 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -6,7 +6,9 @@ All notable changes to this project will be documented in this file. ### Added -- Allow customization of the rolling file appender rotation period [#995]. +- Allow customization of the rolling file appender [#995]. + - rotation period + - filename suffix [#995]: https://github.com/stackabletech/operator-rs/pull/995 diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index 41ac38501..85608fb7d 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -276,6 +276,7 @@ impl Tracing { common_settings, file_log_dir, rotation_period, + filename_suffix, } = &self.file_log_settings { let env_filter_layer = env_filter_builder( @@ -286,7 +287,7 @@ impl Tracing { let file_appender = RollingFileAppender::builder() .rotation(rotation_period.clone()) .filename_prefix(self.service_name.to_string()) - .filename_suffix("tracing-rs.json") + .filename_suffix(filename_suffix) .max_log_files(6) .build(file_log_dir) .context(InitRollingFileAppenderSnafu)?; @@ -694,7 +695,7 @@ mod test { Settings::builder() .with_environment_variable("ABC_FILE") .with_default_level(LevelFilter::INFO) - .file_log_settings_builder(PathBuf::from("/abc_file_dir")) + .file_log_settings_builder(PathBuf::from("/abc_file_dir"), "tracing-rs.json") .build(), ) .with_otlp_log_exporter( @@ -730,6 +731,7 @@ mod test { }, file_log_dir: PathBuf::from("/abc_file_dir"), rotation_period: Rotation::NEVER, + filename_suffix: "tracing-rs.json".to_owned(), } ); assert_eq!( @@ -769,7 +771,7 @@ mod test { .with_file_output(enable_filelog_output.then(|| { Settings::builder() .with_environment_variable("ABC_FILELOG") - .file_log_settings_builder("/dev/null") + .file_log_settings_builder("/dev/null", "tracing-rs.json") .build() })) .with_otlp_trace_exporter(enable_otlp_trace.then(|| { diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index 1390835b9..ab648cd0f 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -24,6 +24,9 @@ pub enum FileLogSettings { /// Log rotation frequency. rotation_period: Rotation, + + /// Suffix for log filenames. + filename_suffix: String, }, } @@ -45,6 +48,7 @@ pub struct FileLogSettingsBuilder { pub(crate) common_settings: Settings, pub(crate) file_log_dir: PathBuf, pub(crate) rotation_period: Rotation, + pub(crate) filename_suffix: String, } impl FileLogSettingsBuilder { @@ -60,6 +64,7 @@ impl FileLogSettingsBuilder { common_settings: self.common_settings, file_log_dir: self.file_log_dir, rotation_period: self.rotation_period, + filename_suffix: self.filename_suffix, } } } @@ -91,11 +96,12 @@ mod test { }, file_log_dir: PathBuf::from("/logs"), rotation_period: Rotation::HOURLY, + filename_suffix: "tracing-rs.log".to_owned(), }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) - .file_log_settings_builder(PathBuf::from("/logs")) + .file_log_settings_builder(PathBuf::from("/logs"), "tracing-rs.json") .with_rotation_period(Rotation::HOURLY) .build(); diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index fe6082a53..d49966e41 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -85,7 +85,11 @@ impl SettingsBuilder { } /// Set specific [`FileLogSettings`]. - pub fn file_log_settings_builder

(self, path: P) -> FileLogSettingsBuilder + pub fn file_log_settings_builder

( + self, + path: P, + filename_suffix: impl Into, + ) -> FileLogSettingsBuilder where P: AsRef, { @@ -93,6 +97,7 @@ impl SettingsBuilder { common_settings: self.build(), file_log_dir: path.as_ref().to_path_buf(), rotation_period: Rotation::NEVER, + filename_suffix: filename_suffix.into(), } } From fc0b0e6f0b061c44639e439d63d561ea142cfcc7 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 15:33:56 +0200 Subject: [PATCH 4/8] feat(stackable-telemetry): Allow customization of the rolling file appender max log files --- crates/stackable-telemetry/CHANGELOG.md | 5 +++-- crates/stackable-telemetry/src/tracing/mod.rs | 13 +++++++++++-- .../src/tracing/settings/file_log.rs | 13 +++++++++++++ .../stackable-telemetry/src/tracing/settings/mod.rs | 1 + 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/crates/stackable-telemetry/CHANGELOG.md b/crates/stackable-telemetry/CHANGELOG.md index 80e516198..8cf6c1418 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -7,8 +7,9 @@ All notable changes to this project will be documented in this file. ### Added - Allow customization of the rolling file appender [#995]. - - rotation period - - filename suffix + - Add required `filename_suffix` field. + - Add `with_rotation_period` method. + - Add `with_max_log_files` method. [#995]: https://github.com/stackabletech/operator-rs/pull/995 diff --git a/crates/stackable-telemetry/src/tracing/mod.rs b/crates/stackable-telemetry/src/tracing/mod.rs index 85608fb7d..0e806b39b 100644 --- a/crates/stackable-telemetry/src/tracing/mod.rs +++ b/crates/stackable-telemetry/src/tracing/mod.rs @@ -277,6 +277,7 @@ impl Tracing { file_log_dir, rotation_period, filename_suffix, + max_log_files, } = &self.file_log_settings { let env_filter_layer = env_filter_builder( @@ -287,8 +288,15 @@ impl Tracing { let file_appender = RollingFileAppender::builder() .rotation(rotation_period.clone()) .filename_prefix(self.service_name.to_string()) - .filename_suffix(filename_suffix) - .max_log_files(6) + .filename_suffix(filename_suffix); + + let file_appender = if let Some(max_log_files) = max_log_files { + file_appender.max_log_files(*max_log_files) + } else { + file_appender + }; + + let file_appender = file_appender .build(file_log_dir) .context(InitRollingFileAppenderSnafu)?; @@ -732,6 +740,7 @@ mod test { file_log_dir: PathBuf::from("/abc_file_dir"), rotation_period: Rotation::NEVER, filename_suffix: "tracing-rs.json".to_owned(), + max_log_files: None, } ); assert_eq!( diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index ab648cd0f..251502a4e 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -27,6 +27,9 @@ pub enum FileLogSettings { /// Suffix for log filenames. filename_suffix: String, + + /// Keep the last `n` files on disk. + max_log_files: Option, }, } @@ -49,6 +52,7 @@ pub struct FileLogSettingsBuilder { pub(crate) file_log_dir: PathBuf, pub(crate) rotation_period: Rotation, pub(crate) filename_suffix: String, + pub(crate) max_log_files: Option, } impl FileLogSettingsBuilder { @@ -58,6 +62,12 @@ impl FileLogSettingsBuilder { self } + /// Set maximum number of log files to keep. + pub fn with_max_log_files(mut self, max_log_files: usize) -> Self { + self.max_log_files = Some(max_log_files); + self + } + /// Consumes self and returns a valid [`FileLogSettings`] instance. pub fn build(self) -> FileLogSettings { FileLogSettings::Enabled { @@ -65,6 +75,7 @@ impl FileLogSettingsBuilder { file_log_dir: self.file_log_dir, rotation_period: self.rotation_period, filename_suffix: self.filename_suffix, + max_log_files: self.max_log_files, } } } @@ -97,12 +108,14 @@ mod test { file_log_dir: PathBuf::from("/logs"), rotation_period: Rotation::HOURLY, filename_suffix: "tracing-rs.log".to_owned(), + max_log_files: Some(6), }; let result = Settings::builder() .with_environment_variable("hello") .with_default_level(LevelFilter::DEBUG) .file_log_settings_builder(PathBuf::from("/logs"), "tracing-rs.json") .with_rotation_period(Rotation::HOURLY) + .with_max_log_files(6) .build(); assert_eq!(expected, result); diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index d49966e41..f83b7bcde 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -98,6 +98,7 @@ impl SettingsBuilder { file_log_dir: path.as_ref().to_path_buf(), rotation_period: Rotation::NEVER, filename_suffix: filename_suffix.into(), + max_log_files: None, } } From d7a85a68bc0f92e5d3293675bfd9af5f01f057ff Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 15:38:22 +0200 Subject: [PATCH 5/8] chore(stackable-telemetry): Mark change as breaking --- 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 8cf6c1418..b23b820b7 100644 --- a/crates/stackable-telemetry/CHANGELOG.md +++ b/crates/stackable-telemetry/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. ### Added -- Allow customization of the rolling file appender [#995]. +- BREAKING: Allow customization of the rolling file appender [#995]. - Add required `filename_suffix` field. - Add `with_rotation_period` method. - Add `with_max_log_files` method. From 8f3383d0a0813d52bf7491da6452a525c3436260 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 15:43:29 +0200 Subject: [PATCH 6/8] feat(stackable-telemetry): Complete the sentence --- crates/stackable-telemetry/src/tracing/settings/file_log.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index 251502a4e..e925a7958 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; -/// Re-export to save the end crate +/// Re-export to save the end crate from an extra import pub use tracing_appender::rolling::Rotation; use super::{Settings, SettingsToggle}; From 5890f654b54f82d101fe5eb417a0bf5006894007 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 15:44:29 +0200 Subject: [PATCH 7/8] feat(stackable-telemetry): Add punctuation --- crates/stackable-telemetry/src/tracing/settings/file_log.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-telemetry/src/tracing/settings/file_log.rs b/crates/stackable-telemetry/src/tracing/settings/file_log.rs index e925a7958..75378cc24 100644 --- a/crates/stackable-telemetry/src/tracing/settings/file_log.rs +++ b/crates/stackable-telemetry/src/tracing/settings/file_log.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; -/// Re-export to save the end crate from an extra import +/// Re-export to save the end crate from an extra import. pub use tracing_appender::rolling::Rotation; use super::{Settings, SettingsToggle}; From 4af7b75224f1f7bdbb915c068170d5ad1f3eccb6 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 1 Apr 2025 16:53:04 +0200 Subject: [PATCH 8/8] chore(stackable-telemetry): Replace generic parameters with impls --- crates/stackable-telemetry/src/tracing/settings/mod.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/stackable-telemetry/src/tracing/settings/mod.rs b/crates/stackable-telemetry/src/tracing/settings/mod.rs index f83b7bcde..6eb19e509 100644 --- a/crates/stackable-telemetry/src/tracing/settings/mod.rs +++ b/crates/stackable-telemetry/src/tracing/settings/mod.rs @@ -85,14 +85,11 @@ impl SettingsBuilder { } /// Set specific [`FileLogSettings`]. - pub fn file_log_settings_builder

( + pub fn file_log_settings_builder( self, - path: P, + path: impl AsRef, filename_suffix: impl Into, - ) -> FileLogSettingsBuilder - where - P: AsRef, - { + ) -> FileLogSettingsBuilder { FileLogSettingsBuilder { common_settings: self.build(), file_log_dir: path.as_ref().to_path_buf(),