From 941605cb5cce0e6e15e225904399cc00a770ecee Mon Sep 17 00:00:00 2001 From: Cristian Filipescu Date: Thu, 7 Mar 2024 12:41:40 -0500 Subject: [PATCH 1/3] appender: add size based rolling --- tracing-appender/src/rolling.rs | 175 ++++++++++++++++++++++++ tracing-appender/src/rolling/builder.rs | 30 ++++ 2 files changed, 205 insertions(+) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index d82646933a..235baa9869 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -108,6 +108,7 @@ struct Inner { rotation: Rotation, next_date: AtomicUsize, max_files: Option, + max_file_size: Option, } // === impl RollingFileAppender === @@ -190,6 +191,7 @@ impl RollingFileAppender { ref prefix, ref suffix, ref max_files, + ref max_file_size, } = builder; let directory = directory.as_ref().to_path_buf(); let now = OffsetDateTime::now_utc(); @@ -200,6 +202,7 @@ impl RollingFileAppender { prefix.clone(), suffix.clone(), *max_files, + *max_file_size, )?; Ok(Self { state, @@ -227,6 +230,8 @@ impl io::Write for RollingFileAppender { let _did_cas = self.state.advance_date(now, current_time); debug_assert!(_did_cas, "if we have &mut access to the appender, no other thread can have advanced the timestamp..."); self.state.refresh_writer(now, writer); + } else if self.state.should_rollover_due_to_size(writer) { + self.state.refresh_writer(now, writer); } writer.write(buf) } @@ -248,6 +253,8 @@ impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for RollingFileAppender if self.state.advance_date(now, current_time) { self.state.refresh_writer(now, &mut self.writer.write()); } + } else if self.state.should_rollover_due_to_size(&self.writer.write()) { + self.state.refresh_writer(now, &mut self.writer.write()); } RollingWriter(self.writer.read()) } @@ -370,6 +377,38 @@ pub fn daily( RollingFileAppender::new(Rotation::DAILY, directory, file_name_prefix) } +/// Creates a size based rolling file appender. +/// +/// The appender returned by `rolling::size` can be used with `non_blocking` to create +/// a non-blocking, size based rotating appender. +/// +/// The location of the log file will be specified by the `directory` passed in. +/// `file_name` specifies the complete name of the log file. +/// `RollingFileAppender` automatically appends the current date in UTC. +/// +/// # Examples +/// +/// ``` rust +/// # #[clippy::allow(needless_doctest_main)] +/// fn main () { +/// # fn doc() { +/// let appender = tracing_appender::rolling::size("/some/path", "rolling.log"); +/// let (non_blocking_appender, _guard) = tracing_appender::non_blocking(appender); +/// +/// let collector = tracing_subscriber::fmt().with_writer(non_blocking_appender); +/// +/// tracing::collect::with_default(collector.finish(), || { +/// tracing::event!(tracing::Level::INFO, "Hello"); +/// }); +/// # } +/// } +/// ``` +/// +/// This will result in a log file located at `/some/path/rolling.log`. +pub fn size(directory: impl AsRef, file_name: impl AsRef) -> RollingFileAppender { + RollingFileAppender::new(Rotation::SIZE, directory, file_name) +} + /// Creates a non-rolling file appender. /// /// The appender returned by `rolling::never` can be used with `non_blocking` to create @@ -444,6 +483,7 @@ enum RotationKind { Minutely, Hourly, Daily, + Size, Never, } @@ -454,6 +494,8 @@ impl Rotation { pub const HOURLY: Self = Self(RotationKind::Hourly); /// Provides a daily rotation pub const DAILY: Self = Self(RotationKind::Daily); + /// Provides a size based rotation + pub const SIZE: Self = Self(RotationKind::Size); /// Provides a rotation that never rotates. pub const NEVER: Self = Self(RotationKind::Never); @@ -462,6 +504,7 @@ impl Rotation { Rotation::MINUTELY => *current_date + Duration::minutes(1), Rotation::HOURLY => *current_date + Duration::hours(1), Rotation::DAILY => *current_date + Duration::days(1), + Rotation::SIZE => return None, Rotation::NEVER => return None, }; Some(self.round_date(&unrounded_next_date)) @@ -485,6 +528,10 @@ impl Rotation { .expect("Invalid time; this is a bug in tracing-appender"); date.replace_time(time) } + // Rotation::SIZE is impossible to round. + Rotation::SIZE => { + unreachable!("Rotation::SIZE is impossible to round.") + } // Rotation::NEVER is impossible to round. Rotation::NEVER => { unreachable!("Rotation::NEVER is impossible to round.") @@ -497,6 +544,9 @@ impl Rotation { Rotation::MINUTELY => format_description::parse("[year]-[month]-[day]-[hour]-[minute]"), Rotation::HOURLY => format_description::parse("[year]-[month]-[day]-[hour]"), Rotation::DAILY => format_description::parse("[year]-[month]-[day]"), + Rotation::SIZE => format_description::parse( + "[year]-[month]-[day]-[hour]-[minute]-[second]-[subsecond]", + ), Rotation::NEVER => format_description::parse("[year]-[month]-[day]"), } .expect("Unable to create a formatter; this is a bug in tracing-appender") @@ -525,6 +575,7 @@ impl Inner { log_filename_prefix: Option, log_filename_suffix: Option, max_files: Option, + max_file_size: Option, ) -> Result<(Self, RwLock), builder::InitError> { let log_directory = directory.as_ref().to_path_buf(); let date_format = rotation.date_format(); @@ -542,6 +593,7 @@ impl Inner { ), rotation, max_files, + max_file_size, }; let filename = inner.join_date(&now); let writer = RwLock::new(create_writer(inner.log_directory.as_ref(), &filename)?); @@ -674,6 +726,23 @@ impl Inner { None } + /// Checks whether or not the file needs to rollover because it reached the size limit. + /// + /// If this method returns `true`, we should roll to a new log file. + /// Otherwise, if this returns `false` we should not rotate the log file. + fn should_rollover_due_to_size(&self, current_file: &File) -> bool { + current_file.sync_all().ok(); + if let (Ok(file_metadata), Some(max_file_size), &Rotation::SIZE) = + (current_file.metadata(), self.max_file_size, &self.rotation) + { + if file_metadata.len() >= max_file_size { + return true; + } + } + + false + } + fn advance_date(&self, now: OffsetDateTime, current: usize) -> bool { let next_date = self .rotation @@ -761,6 +830,11 @@ mod test { test_appender(Rotation::DAILY, "daily.log"); } + #[test] + fn write_size_log() { + test_appender(Rotation::SIZE, "size.log"); + } + #[test] fn write_never_log() { test_appender(Rotation::NEVER, "never.log"); @@ -783,6 +857,11 @@ mod test { let next = Rotation::DAILY.next_date(&now).unwrap(); assert_eq!((now + Duration::DAY).day(), next.day()); + // size + let now = OffsetDateTime::now_utc(); + let next = Rotation::SIZE.next_date(&now); + assert!(next.is_none()); + // never let now = OffsetDateTime::now_utc(); let next = Rotation::NEVER.next_date(&now); @@ -829,6 +908,7 @@ mod test { prefix.map(ToString::to_string), suffix.map(ToString::to_string), None, + None, ) .unwrap(); let path = inner.join_date(&now); @@ -859,6 +939,12 @@ mod test { prefix: Some("app.log"), suffix: None, }, + TestCase { + expected: "app.log.2020-02-01-10-01-00-0", + rotation: Rotation::SIZE, + prefix: Some("app.log"), + suffix: None, + }, TestCase { expected: "app.log", rotation: Rotation::NEVER, @@ -884,6 +970,12 @@ mod test { prefix: Some("app"), suffix: Some("log"), }, + TestCase { + expected: "app.2020-02-01-10-01-00-0.log", + rotation: Rotation::SIZE, + prefix: Some("app"), + suffix: Some("log"), + }, TestCase { expected: "app.log", rotation: Rotation::NEVER, @@ -909,6 +1001,12 @@ mod test { prefix: None, suffix: Some("log"), }, + TestCase { + expected: "2020-02-01-10-01-00-0.log", + rotation: Rotation::SIZE, + prefix: None, + suffix: Some("log"), + }, TestCase { expected: "log", rotation: Rotation::NEVER, @@ -941,6 +1039,7 @@ mod test { Some("test_make_writer".to_string()), None, None, + None, ) .unwrap(); @@ -1023,6 +1122,7 @@ mod test { Some("test_max_log_files".to_string()), None, Some(2), + None, ) .unwrap(); @@ -1106,4 +1206,79 @@ mod test { } } } + + #[test] + fn test_size_based_rolling() { + use std::sync::{Arc, Mutex}; + use tracing_subscriber::prelude::*; + + let format = format_description::parse( + "[year]-[month]-[day] [hour]:[minute]:[second] [offset_hour \ + sign:mandatory]:[offset_minute]:[offset_second]", + ) + .unwrap(); + + const MAX_FILE_SIZE: u64 = 1024; + let now = OffsetDateTime::parse("2020-02-01 10:01:00 +00:00:00", &format).unwrap(); + let directory = tempfile::tempdir().expect("failed to create tempdir"); + let (state, writer) = Inner::new( + now, + Rotation::SIZE, + directory.path(), + Some("test_max_file_size".to_string()), + None, + Some(5), + Some(MAX_FILE_SIZE), + ) + .unwrap(); + + let clock = Arc::new(Mutex::new(now)); + let now = { + let clock = clock.clone(); + Box::new(move || *clock.lock().unwrap()) + }; + let appender = RollingFileAppender { state, writer, now }; + let default = tracing_subscriber::fmt() + .without_time() + .with_level(false) + .with_target(false) + .with_max_level(tracing_subscriber::filter::LevelFilter::TRACE) + .with_writer(appender) + .finish() + .set_default(); + + for file_num in 0..5 { + for i in 0..58 { + tracing::info!("file {} content {}", file_num, i); + (*clock.lock().unwrap()) += Duration::milliseconds(1); + } + } + + drop(default); + + let dir_contents = fs::read_dir(directory.path()).expect("Failed to read directory"); + println!("dir={:?}", dir_contents); + + for entry in dir_contents { + println!("entry={:?}", entry); + let path = entry.expect("Expected dir entry").path(); + let file_fd = fs::File::open(&path).expect("Failed to open file"); + let file_metadata = file_fd.metadata().expect("Failed to get file metadata"); + println!( + "path={}\nfile_len={:?}", + path.display(), + file_metadata.len() + ); + let file = fs::read_to_string(&path).expect("Failed to read file"); + println!("path={}\nfile={:?}", path.display(), file); + + assert_eq!( + MAX_FILE_SIZE + 10, + file_metadata.len(), + "expected size = {:?}, file size = {:?}", + MAX_FILE_SIZE, + file_metadata.len(), + ); + } + } } diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs index 8c92ca1238..80ebddac6e 100644 --- a/tracing-appender/src/rolling/builder.rs +++ b/tracing-appender/src/rolling/builder.rs @@ -11,6 +11,7 @@ pub struct Builder { pub(super) prefix: Option, pub(super) suffix: Option, pub(super) max_files: Option, + pub(super) max_file_size: Option, } /// Errors returned by [`Builder::build`]. @@ -42,11 +43,13 @@ impl Builder { /// | [`filename_prefix`] | `""` | By default, log file names will not have a prefix. | /// | [`filename_suffix`] | `""` | By default, log file names will not have a suffix. | /// | [`max_log_files`] | `None` | By default, there is no limit for maximum log file count. | + /// | [`max_file_size`] | `None` | By default, there is no limit for maximum log file size. | /// /// [`rotation`]: Self::rotation /// [`filename_prefix`]: Self::filename_prefix /// [`filename_suffix`]: Self::filename_suffix /// [`max_log_files`]: Self::max_log_files + /// ['max_file_size`]: Self::max_file_size #[must_use] pub const fn new() -> Self { Self { @@ -54,6 +57,7 @@ impl Builder { prefix: None, suffix: None, max_files: None, + max_file_size: None, } } @@ -233,6 +237,32 @@ impl Builder { } } + /// Limits the file size to `n` bytes on disk, when using SIZE rotation. + /// + /// # Examples + /// + /// ``` + /// use tracing_appender::rolling::RollingFileAppender; + /// use tracing_appender::rolling::Rotation; + /// + /// # fn docs() { + /// let appender = RollingFileAppender::builder() + /// .rotation(Rotation::SIZE) // rotate log files when they reach a certain size + /// .max_file_size(1024) // only the most recent 5 log files will be kept + /// // ... + /// .build("/var/log") + /// .expect("failed to initialize rolling file appender"); + /// # drop(appender) + /// # } + /// ``` + #[must_use] + pub fn max_file_size(self, n: u64) -> Self { + Self { + max_file_size: Some(n), + ..self + } + } + /// Builds a new [`RollingFileAppender`] with the configured parameters, /// emitting log files to the provided directory. /// From 47b50896fecd761263e4dab123e0ee5a351f6252 Mon Sep 17 00:00:00 2001 From: Cristian Filipescu Date: Thu, 7 Mar 2024 22:43:19 -0500 Subject: [PATCH 2/3] appender: fix typo --- tracing-appender/src/rolling/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs index 80ebddac6e..b84a6b4b89 100644 --- a/tracing-appender/src/rolling/builder.rs +++ b/tracing-appender/src/rolling/builder.rs @@ -49,7 +49,7 @@ impl Builder { /// [`filename_prefix`]: Self::filename_prefix /// [`filename_suffix`]: Self::filename_suffix /// [`max_log_files`]: Self::max_log_files - /// ['max_file_size`]: Self::max_file_size + /// [`max_file_size`]: Self::max_file_size #[must_use] pub const fn new() -> Self { Self { From bf707aa09f22096e8a74d83698b0c059ccee0258 Mon Sep 17 00:00:00 2001 From: Cristian Filipescu Date: Thu, 4 Apr 2024 00:50:25 -0400 Subject: [PATCH 3/3] appender: prune old logs when building the appender for the first time --- tracing-appender/src/rolling.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index 235baa9869..1a3ceaa826 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -596,6 +596,9 @@ impl Inner { max_file_size, }; let filename = inner.join_date(&now); + if let Some(max_files) = inner.max_files { + inner.prune_old_logs(max_files); + } let writer = RwLock::new(create_writer(inner.log_directory.as_ref(), &filename)?); Ok((inner, writer)) }