From 07d9a2ace23552bff3f452edaa726530c1ed0d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Fri, 3 Mar 2023 21:18:52 -0500 Subject: [PATCH 1/6] feat(appender): size-based rotation This patch adds size-based rotation to tracing-appender. In a constraint environment, we should be able to contain the logging size and period. Related issues: - https://github.com/tokio-rs/tracing/issues/1940 - https://github.com/tokio-rs/tracing/issues/858 --- tracing-appender/src/rolling.rs | 297 ++++++++++++++++++++++++++------ 1 file changed, 247 insertions(+), 50 deletions(-) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index d82646933a..7f274aeb95 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -28,11 +28,15 @@ //! ``` use crate::sync::{RwLock, RwLockReadGuard}; use std::{ + convert::TryFrom, fmt::{self, Debug}, fs::{self, File, OpenOptions}, io::{self, Write}, path::{Path, PathBuf}, - sync::atomic::{AtomicUsize, Ordering}, + sync::{ + atomic::{AtomicU64, AtomicUsize, Ordering}, + Arc, + }, }; use time::{format_description, Date, Duration, OffsetDateTime, Time}; @@ -97,7 +101,10 @@ pub struct RollingFileAppender { /// [writer]: std::io::Write /// [`MakeWriter`]: tracing_subscriber::fmt::writer::MakeWriter #[derive(Debug)] -pub struct RollingWriter<'a>(RwLockReadGuard<'a, File>); +pub struct RollingWriter<'a> { + inner: RwLockReadGuard<'a, File>, + current_size: Arc, +} #[derive(Debug)] struct Inner { @@ -107,6 +114,7 @@ struct Inner { date_format: Vec>, rotation: Rotation, next_date: AtomicUsize, + current_size: Arc, max_files: Option, } @@ -224,11 +232,19 @@ impl io::Write for RollingFileAppender { let now = self.now(); let writer = self.writer.get_mut(); if let Some(current_time) = self.state.should_rollover(now) { - 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..."); + 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); } - writer.write(buf) + + let written = writer.write(buf)?; + + self.state.current_size.fetch_add( + u64::try_from(written).expect("usize to u64 conversion"), + Ordering::SeqCst, + ); + + Ok(written) } fn flush(&mut self) -> io::Result<()> { @@ -238,6 +254,7 @@ impl io::Write for RollingFileAppender { impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for RollingFileAppender { type Writer = RollingWriter<'a>; + fn make_writer(&'a self) -> Self::Writer { let now = self.now(); @@ -249,7 +266,11 @@ impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for RollingFileAppender self.state.refresh_writer(now, &mut self.writer.write()); } } - RollingWriter(self.writer.read()) + + RollingWriter { + inner: self.writer.read(), + current_size: Arc::clone(&self.state.current_size), + } } } @@ -362,7 +383,7 @@ pub fn hourly( /// } /// ``` /// -/// This will result in a log file located at `/some/path/rolling.log.yyyy-MM-dd-HH`. +/// This will result in a log file located at `/some/path/rolling.log.yyyy-MM-dd`. pub fn daily( directory: impl AsRef, file_name_prefix: impl AsRef, @@ -401,6 +422,48 @@ pub fn never(directory: impl AsRef, file_name: impl AsRef) -> Rollin RollingFileAppender::new(Rotation::NEVER, directory, file_name) } +/// Creates a file appender that rotates based on file size. +/// +/// The appender returned by `rolling::max_size` can be used with `non_blocking` to create +/// a non-blocking, size-based file appender. +/// +/// A `RollingFileAppender` has a fixed rotation whose frequency is +/// defined by [`Rotation`]. The `directory` and +/// `file_name_prefix` arguments determine the location and file name's _prefix_ +/// of the log file. `RollingFileAppender` automatically appends the current date in UTC. +/// `max_number_of_bytes` specifies the maximum number of bytes for each file. +/// +/// # Examples +/// +/// ```rust +/// # #[clippy::allow(needless_doctest_main)] +/// fn main () { +/// # fn doc() { +/// let appender = tracing_appender::rolling::max_size("/some/path", "rolling.log", 100_000_000); // 100 MB +/// 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.yyyy-MM-dd-HH-mm-ss`. +pub fn max_size( + directory: impl AsRef, + file_name_prefix: impl AsRef, + max_number_of_bytes: u64, +) -> RollingFileAppender { + RollingFileAppender::new( + Rotation::max_bytes(max_number_of_bytes), + directory, + file_name_prefix, + ) +} + /// Defines a fixed period for rolling of a log file. /// /// To use a `Rotation`, pick one of the following options: @@ -437,10 +500,13 @@ pub fn never(directory: impl AsRef, file_name: impl AsRef) -> Rollin /// # } /// ``` #[derive(Clone, Eq, PartialEq, Debug)] -pub struct Rotation(RotationKind); +pub struct Rotation { + timed: Timed, + max_bytes: Option, +} -#[derive(Clone, Eq, PartialEq, Debug)] -enum RotationKind { +#[derive(Clone, Copy, Eq, PartialEq, Debug)] +enum Timed { Minutely, Hourly, Daily, @@ -448,56 +514,94 @@ enum RotationKind { } impl Rotation { - /// Provides an minutely rotation - pub const MINUTELY: Self = Self(RotationKind::Minutely); - /// Provides an hourly rotation - pub const HOURLY: Self = Self(RotationKind::Hourly); - /// Provides a daily rotation - pub const DAILY: Self = Self(RotationKind::Daily); + /// Provides a minutely rotation. + pub const MINUTELY: Self = Self { + timed: Timed::Minutely, + max_bytes: None, + }; + + /// Provides an hourly rotation. + pub const HOURLY: Self = Self { + timed: Timed::Hourly, + max_bytes: None, + }; + + /// Provides a daily rotation. + pub const DAILY: Self = Self { + timed: Timed::Daily, + max_bytes: None, + }; + /// Provides a rotation that never rotates. - pub const NEVER: Self = Self(RotationKind::Never); + pub const NEVER: Self = Self { + timed: Timed::Never, + max_bytes: None, + }; + + /// Provides a size-based rotation. + pub const fn max_bytes(number_of_bytes: u64) -> Self { + Self { + timed: Timed::Never, + max_bytes: Some(number_of_bytes), + } + } + + /// Provides a daily and size-based rotation. + pub const fn daily_with_max_bytes(number_of_bytes: u64) -> Self { + Self { + timed: Timed::Daily, + max_bytes: Some(number_of_bytes), + } + } pub(crate) fn next_date(&self, current_date: &OffsetDateTime) -> Option { - let unrounded_next_date = match *self { - Rotation::MINUTELY => *current_date + Duration::minutes(1), - Rotation::HOURLY => *current_date + Duration::hours(1), - Rotation::DAILY => *current_date + Duration::days(1), - Rotation::NEVER => return None, + let unrounded_next_date = match self.timed { + Timed::Minutely => *current_date + Duration::minutes(1), + Timed::Hourly => *current_date + Duration::hours(1), + Timed::Daily => *current_date + Duration::days(1), + Timed::Never => return None, }; Some(self.round_date(&unrounded_next_date)) } - // note that this method will panic if passed a `Rotation::NEVER`. + // note that this method will panic if passed a `Timed::Never`. pub(crate) fn round_date(&self, date: &OffsetDateTime) -> OffsetDateTime { - match *self { - Rotation::MINUTELY => { + match self.timed { + Timed::Minutely => { let time = Time::from_hms(date.hour(), date.minute(), 0) .expect("Invalid time; this is a bug in tracing-appender"); date.replace_time(time) } - Rotation::HOURLY => { + Timed::Hourly => { let time = Time::from_hms(date.hour(), 0, 0) .expect("Invalid time; this is a bug in tracing-appender"); date.replace_time(time) } - Rotation::DAILY => { + Timed::Daily => { let time = Time::from_hms(0, 0, 0) .expect("Invalid time; this is a bug in tracing-appender"); date.replace_time(time) } - // Rotation::NEVER is impossible to round. - Rotation::NEVER => { - unreachable!("Rotation::NEVER is impossible to round.") + // Timed::Never is impossible to round. + Timed::Never => { + unreachable!("Timed::Never is impossible to round.") } } } fn date_format(&self) -> Vec> { - match *self { - 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::NEVER => format_description::parse("[year]-[month]-[day]"), + if self.max_bytes.is_some() { + // Very specific format description to avoid conflicts in case logs are filled very quickly + format_description::parse("[year]-[month]-[day]-[hour]-[minute]-[second]") + } else { + match self.timed { + Timed::Minutely => { + format_description::parse("[year]-[month]-[day]-[hour]-[minute]") + } + Timed::Hourly => format_description::parse("[year]-[month]-[day]-[hour]"), + Timed::Daily => format_description::parse("[year]-[month]-[day]"), + Timed::Never => format_description::parse("[year]-[month]-[day]"), + } } .expect("Unable to create a formatter; this is a bug in tracing-appender") } @@ -507,11 +611,16 @@ impl Rotation { impl io::Write for RollingWriter<'_> { fn write(&mut self, buf: &[u8]) -> io::Result { - (&*self.0).write(buf) + let written = (&*self.inner).write(buf)?; + self.current_size.fetch_add( + u64::try_from(written).expect("usize to u64 conversion"), + Ordering::SeqCst, + ); + Ok(written) } fn flush(&mut self) -> io::Result<()> { - (&*self.0).flush() + (&*self.inner).flush() } } @@ -530,7 +639,7 @@ impl Inner { let date_format = rotation.date_format(); let next_date = rotation.next_date(&now); - let inner = Inner { + let mut inner = Inner { log_directory, log_filename_prefix, log_filename_suffix, @@ -542,9 +651,20 @@ impl Inner { ), rotation, max_files, + current_size: Arc::new(AtomicU64::new(0)), }; + let filename = inner.join_date(&now); - let writer = RwLock::new(create_writer(inner.log_directory.as_ref(), &filename)?); + let file = create_writer(inner.log_directory.as_ref(), &filename)?; + + let current_file_size = file + .metadata() + .map_err(builder::InitError::ctx("Failed to read file metadata"))? + .len(); + inner.current_size = Arc::new(AtomicU64::new(current_file_size)); + + let writer = RwLock::new(file); + Ok((inner, writer)) } @@ -554,17 +674,20 @@ impl Inner { .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender"); match ( - &self.rotation, + self.rotation.timed, + self.rotation.max_bytes, &self.log_filename_prefix, &self.log_filename_suffix, ) { - (&Rotation::NEVER, Some(filename), None) => filename.to_string(), - (&Rotation::NEVER, Some(filename), Some(suffix)) => format!("{}.{}", filename, suffix), - (&Rotation::NEVER, None, Some(suffix)) => suffix.to_string(), - (_, Some(filename), Some(suffix)) => format!("{}.{}.{}", filename, date, suffix), - (_, Some(filename), None) => format!("{}.{}", filename, date), - (_, None, Some(suffix)) => format!("{}.{}", date, suffix), - (_, None, None) => date, + (Timed::Never, None, Some(filename), None) => filename.to_string(), + (Timed::Never, None, Some(filename), Some(suffix)) => { + format!("{}.{}", filename, suffix) + } + (Timed::Never, None, None, Some(suffix)) => suffix.to_string(), + (_, _, Some(filename), Some(suffix)) => format!("{}.{}.{}", filename, date, suffix), + (_, _, Some(filename), None) => format!("{}.{}", filename, date), + (_, _, None, Some(suffix)) => format!("{}.{}", date, suffix), + (_, _, None, None) => date, } } @@ -647,6 +770,7 @@ impl Inner { eprintln!("Couldn't flush previous writer: {}", err); } *file = new_file; + self.current_size.store(0, Ordering::Release); } Err(err) => eprintln!("Couldn't create writer for logs: {}", err), } @@ -662,6 +786,14 @@ impl Inner { /// Otherwise, if this returns we should not rotate the log file. fn should_rollover(&self, date: OffsetDateTime) -> Option { let next_date = self.next_date.load(Ordering::Acquire); + + match self.rotation.max_bytes { + Some(max_bytes) if self.current_size.load(Ordering::Acquire) > max_bytes => { + return Some(next_date); + } + _ => {} + } + // if the next date is 0, this appender *never* rotates log files. if next_date == 0 { return None; @@ -791,7 +923,7 @@ mod test { #[test] #[should_panic( - expected = "internal error: entered unreachable code: Rotation::NEVER is impossible to round." + expected = "internal error: entered unreachable code: Timed::Never is impossible to round." )] fn test_never_date_rounding() { let now = OffsetDateTime::now_utc(); @@ -841,6 +973,18 @@ mod test { let test_cases = vec![ // prefix only + TestCase { + expected: "app.log.2020-02-01-10-01-00", + rotation: Rotation::daily_with_max_bytes(1024), + prefix: Some("app.log"), + suffix: None, + }, + TestCase { + expected: "app.log.2020-02-01-10-01-00", + rotation: Rotation::max_bytes(1024), + prefix: Some("app.log"), + suffix: None, + }, TestCase { expected: "app.log.2020-02-01-10-01", rotation: Rotation::MINUTELY, @@ -866,6 +1010,18 @@ mod test { suffix: None, }, // prefix and suffix + TestCase { + expected: "app.2020-02-01-10-01-00.log", + rotation: Rotation::daily_with_max_bytes(1024), + prefix: Some("app"), + suffix: Some("log"), + }, + TestCase { + expected: "app.2020-02-01-10-01-00.log", + rotation: Rotation::max_bytes(1024), + prefix: Some("app"), + suffix: Some("log"), + }, TestCase { expected: "app.2020-02-01-10-01.log", rotation: Rotation::MINUTELY, @@ -891,6 +1047,18 @@ mod test { suffix: Some("log"), }, // suffix only + TestCase { + expected: "2020-02-01-10-01-00.log", + rotation: Rotation::daily_with_max_bytes(1024), + prefix: None, + suffix: Some("log"), + }, + TestCase { + expected: "2020-02-01-10-01-00.log", + rotation: Rotation::max_bytes(1024), + prefix: None, + suffix: Some("log"), + }, TestCase { expected: "2020-02-01-10-01.log", rotation: Rotation::MINUTELY, @@ -916,6 +1084,7 @@ mod test { suffix: Some("log"), }, ]; + for test_case in test_cases { test(test_case) } @@ -1019,7 +1188,7 @@ mod test { let (state, writer) = Inner::new( now, Rotation::HOURLY, - directory.path(), + &directory, Some("test_max_log_files".to_string()), None, Some(2), @@ -1078,7 +1247,7 @@ mod test { drop(default); - let dir_contents = fs::read_dir(directory.path()).expect("Failed to read directory"); + let dir_contents = fs::read_dir(&directory).expect("Failed to read directory"); println!("dir={:?}", dir_contents); for entry in dir_contents { @@ -1106,4 +1275,32 @@ mod test { } } } + + #[test] + fn size_based_rotation() { + let directory = tempfile::tempdir().expect("failed to create tempdir"); + + let mut appender = max_size(&directory, "size_based_rotation", 8); + + writeln!(appender, "more than 8 bytes").unwrap(); + std::thread::sleep(std::time::Duration::from_secs(1)); + writeln!(appender, "more than 8 bytes again").unwrap(); + std::thread::sleep(std::time::Duration::from_secs(1)); + writeln!(appender, "and here is a third file").unwrap(); + std::thread::sleep(std::time::Duration::from_secs(1)); + + let dir_contents = fs::read_dir(&directory).expect("Failed to read directory"); + println!("dir={:?}", dir_contents); + + let mut count = 0; + for entry in dir_contents { + println!("entry={:?}", entry); + let path = entry.expect("Expected dir entry").path(); + let file = fs::read_to_string(&path).expect("Failed to read file"); + println!("path={}\nfile={:?}", path.display(), file); + count += 1; + } + + assert_eq!(count, 3); + } } From 8704565b79ea9b2d492960f2dc775af754988f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 5 Sep 2023 11:31:53 -0400 Subject: [PATCH 2/6] Code review --- tracing-appender/src/rolling.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index 7f274aeb95..5ad25bef2b 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -103,7 +103,7 @@ pub struct RollingFileAppender { #[derive(Debug)] pub struct RollingWriter<'a> { inner: RwLockReadGuard<'a, File>, - current_size: Arc, + current_size: &'a AtomicU64, } #[derive(Debug)] @@ -269,7 +269,7 @@ impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for RollingFileAppender RollingWriter { inner: self.writer.read(), - current_size: Arc::clone(&self.state.current_size), + current_size: &self.state.current_size, } } } @@ -546,6 +546,22 @@ impl Rotation { } } + /// Provides a minutely and size-based rotation. + pub const fn minutely_with_max_bytes(number_of_bytes: u64) -> Self { + Self { + timed: Timed::Minutely, + max_bytes: Some(number_of_bytes), + } + } + + /// Provides a hourly and size-based rotation. + pub const fn hourly_with_max_bytes(number_of_bytes: u64) -> Self { + Self { + timed: Timed::Hourly, + max_bytes: Some(number_of_bytes), + } + } + /// Provides a daily and size-based rotation. pub const fn daily_with_max_bytes(number_of_bytes: u64) -> Self { Self { From 81d2d706825a4e99c3653ef450febc7593de932b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 5 Sep 2023 19:12:22 -0400 Subject: [PATCH 3/6] Format date according to `Timed` and do not create too many files Follow the date format induced by the selected `Timed`. An index is inserted into the file name so we can rotate when the max size is reached. Also, when the most recent file does not exceed the max size, we simply reuse it instead of creating yet another file. --- tracing-appender/src/rolling.rs | 432 +++++++++++++++++++++++++------- 1 file changed, 348 insertions(+), 84 deletions(-) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index 5ad25bef2b..77a76e81d0 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -111,6 +111,7 @@ struct Inner { log_directory: PathBuf, log_filename_prefix: Option, log_filename_suffix: Option, + log_filename_index: Option, date_format: Vec>, rotation: Rotation, next_date: AtomicUsize, @@ -232,7 +233,7 @@ impl io::Write for RollingFileAppender { let now = self.now(); let writer = self.writer.get_mut(); if let Some(current_time) = self.state.should_rollover(now) { - let did_cas = self.state.advance_date(now, current_time); + let did_cas = self.state.advance_date_and_index(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); } @@ -262,7 +263,7 @@ impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for RollingFileAppender if let Some(current_time) = self.state.should_rollover(now) { // Did we get the right to lock the file? If not, another thread // did it and we can just make a writer. - if self.state.advance_date(now, current_time) { + if self.state.advance_date_and_index(now, current_time) { self.state.refresh_writer(now, &mut self.writer.write()); } } @@ -606,18 +607,11 @@ impl Rotation { } fn date_format(&self) -> Vec> { - if self.max_bytes.is_some() { - // Very specific format description to avoid conflicts in case logs are filled very quickly - format_description::parse("[year]-[month]-[day]-[hour]-[minute]-[second]") - } else { - match self.timed { - Timed::Minutely => { - format_description::parse("[year]-[month]-[day]-[hour]-[minute]") - } - Timed::Hourly => format_description::parse("[year]-[month]-[day]-[hour]"), - Timed::Daily => format_description::parse("[year]-[month]-[day]"), - Timed::Never => format_description::parse("[year]-[month]-[day]"), - } + match self.timed { + Timed::Minutely => format_description::parse("[year]-[month]-[day]-[hour]-[minute]"), + Timed::Hourly => format_description::parse("[year]-[month]-[day]-[hour]"), + Timed::Daily => format_description::parse("[year]-[month]-[day]"), + Timed::Never => format_description::parse("[year]-[month]-[day]"), } .expect("Unable to create a formatter; this is a bug in tracing-appender") } @@ -659,6 +653,7 @@ impl Inner { log_directory, log_filename_prefix, log_filename_suffix, + log_filename_index: None, date_format, next_date: AtomicUsize::new( next_date @@ -670,6 +665,11 @@ impl Inner { current_size: Arc::new(AtomicU64::new(0)), }; + if let Some(max_bytes) = inner.rotation.max_bytes { + let next_index = inner.find_filename_index(now, max_bytes); + inner.log_filename_index = Some(AtomicU64::new(next_index)); + } + let filename = inner.join_date(&now); let file = create_writer(inner.log_directory.as_ref(), &filename)?; @@ -685,64 +685,199 @@ impl Inner { } pub(crate) fn join_date(&self, date: &OffsetDateTime) -> String { + macro_rules! insert_dot { + ($name:ident) => { + if !$name.is_empty() { + $name.push_str("."); + } + }; + } + let date = date .format(&self.date_format) .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender"); - match ( - self.rotation.timed, - self.rotation.max_bytes, - &self.log_filename_prefix, - &self.log_filename_suffix, - ) { - (Timed::Never, None, Some(filename), None) => filename.to_string(), - (Timed::Never, None, Some(filename), Some(suffix)) => { - format!("{}.{}", filename, suffix) + let mut filename = String::new(); + + if let Some(prefix) = &self.log_filename_prefix { + filename.push_str(prefix); + }; + + match self.rotation.timed { + // Insert the date when there is time-based rotations + Timed::Minutely | Timed::Hourly | Timed::Daily => { + insert_dot!(filename); + filename.push_str(&date); } - (Timed::Never, None, None, Some(suffix)) => suffix.to_string(), - (_, _, Some(filename), Some(suffix)) => format!("{}.{}.{}", filename, date, suffix), - (_, _, Some(filename), None) => format!("{}.{}", filename, date), - (_, _, None, Some(suffix)) => format!("{}.{}", date, suffix), - (_, _, None, None) => date, + // "Never" but no prefix and no suffix means we should use the date anyway + // The date is always included when there is a filename index (size-based rotation) + Timed::Never + if (self.log_filename_prefix.is_none() && self.log_filename_suffix.is_none()) + || self.log_filename_index.is_some() => + { + insert_dot!(filename); + filename.push_str(&date); + } + // Otherwise, the date must not be inserted + Timed::Never => {} + } + + if let Some(index) = &self.log_filename_index { + insert_dot!(filename); + filename.push_str(&index.load(Ordering::Acquire).to_string()); + } + + if let Some(suffix) = &self.log_filename_suffix { + insert_dot!(filename); + filename.push_str(suffix); } + + // Sanity check: we should never end up with an empty filename + assert!( + !filename.is_empty(), + "log file name should never be empty; this is a bug in tracing-appender" + ); + + filename } - fn prune_old_logs(&self, max_files: usize) { - let files = fs::read_dir(&self.log_directory).map(|dir| { - dir.filter_map(|entry| { - let entry = entry.ok()?; - let metadata = entry.metadata().ok()?; + fn filter_log_file(&self, entry: &fs::DirEntry) -> Option { + let metadata = entry.metadata().ok()?; - // the appender only creates files, not directories or symlinks, - // so we should never delete a dir or symlink. - if !metadata.is_file() { - return None; - } + // the appender only creates files, not directories or symlinks, + // so we should never delete a dir or symlink. + if !metadata.is_file() { + return None; + } - let filename = entry.file_name(); - // if the filename is not a UTF-8 string, skip it. - let filename = filename.to_str()?; - if let Some(prefix) = &self.log_filename_prefix { - if !filename.starts_with(prefix) { - return None; - } - } + let filename = entry.file_name(); + + // if the filename is not a UTF-8 string, skip it. + let mut filename = filename.to_str()?; + + if let Some(prefix) = &self.log_filename_prefix { + let striped = filename.strip_prefix(prefix)?; + filename = striped.trim_start_matches('.'); + } + + if let Some(suffix) = &self.log_filename_suffix { + let striped = filename.strip_suffix(suffix)?; + filename = striped.trim_end_matches('.'); + } + + let mut found_index = None; + + let formatted_date = match (self.rotation.timed, filename.find('.')) { + (Timed::Never, None) + if self.log_filename_prefix.is_none() && self.log_filename_suffix.is_none() => + { + let _date = Date::parse(filename, &self.date_format).ok()?; + Some(filename.to_owned()) + } + (_, Some(dot_idx)) => { + // Check for . pattern + + let date_segment = &filename[..dot_idx]; + let index_segment = &filename[dot_idx + 1..]; + + let _date = Date::parse(date_segment, &self.date_format).ok()?; + let index = index_segment.parse::().ok()?; - if let Some(suffix) = &self.log_filename_suffix { - if !filename.ends_with(suffix) { - return None; - } + found_index = Some(index); + Some(date_segment.to_owned()) + } + (_, None) => { + if Date::parse(filename, &self.date_format).is_ok() { + Some(filename.to_owned()) + } else { + None } + } + }; + + if self.log_filename_prefix.is_none() + && self.log_filename_suffix.is_none() + && found_index.is_none() + && formatted_date.is_none() + { + return None; + } + + let created_at = metadata.created().ok()?; + + Some(FilteredLogFile { + path: entry.path(), + metadata, + created_at, + formatted_date, + index: found_index, + }) + } - if self.log_filename_prefix.is_none() - && self.log_filename_suffix.is_none() - && Date::parse(filename, &self.date_format).is_err() - { - return None; + fn find_filename_index(&self, now: OffsetDateTime, max_bytes: u64) -> u64 { + macro_rules! unwrap_or_zero { + ($value:expr) => { + if let Some(value) = $value { + value + } else { + return 0; } + }; + } + + let read_dir = unwrap_or_zero!(fs::read_dir(&self.log_directory).ok()); - let created = metadata.created().ok()?; - Some((entry, created)) + let mut files: Vec = read_dir + .filter_map(|entry| { + let entry = entry.ok()?; + self.filter_log_file(&entry) + }) + .collect(); + + files.sort_unstable_by_key(|file| file.created_at); + + let most_recent_file = unwrap_or_zero!(files.last()); + + let most_recent_formatted_date = unwrap_or_zero!(&most_recent_file.formatted_date).clone(); + + let formatted_date_now = now + .format(&self.date_format) + .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender"); + + if most_recent_formatted_date != formatted_date_now { + return 0; + } + + let latest_file = files + .into_iter() + .filter(|log_file| { + log_file + .formatted_date + .as_ref() + .and_then(|formatted_date| { + most_recent_formatted_date.eq(formatted_date).then_some(()) + }) + .is_some() + }) + .max_by_key(|log_file| log_file.index); + + let latest_file = unwrap_or_zero!(latest_file); + + let index = unwrap_or_zero!(latest_file.index); + + // Increase the index if the latest file was too big + if latest_file.metadata.len() > max_bytes { + index + 1 + } else { + index + } + } + + fn prune_old_logs(&self, max_files: usize) { + let files = fs::read_dir(&self.log_directory).map(|dir| { + dir.filter_map(|entry| { + let entry = entry.ok()?; + self.filter_log_file(&entry) }) .collect::>() }); @@ -759,14 +894,14 @@ impl Inner { } // sort the files by their creation timestamps. - files.sort_by_key(|(_, created_at)| *created_at); + files.sort_by_key(|log_file| log_file.created_at); // delete files, so that (n-1) files remain, because we will create another log file - for (file, _) in files.iter().take(files.len() - (max_files - 1)) { - if let Err(error) = fs::remove_file(file.path()) { + for file in files.iter().take(files.len() - (max_files - 1)) { + if let Err(error) = fs::remove_file(&file.path) { eprintln!( "Failed to remove old log file {}: {}", - file.path().display(), + file.path.display(), error ); } @@ -803,14 +938,16 @@ impl Inner { fn should_rollover(&self, date: OffsetDateTime) -> Option { let next_date = self.next_date.load(Ordering::Acquire); + // if there is a maximum size for the file, check that it is not exceeded match self.rotation.max_bytes { Some(max_bytes) if self.current_size.load(Ordering::Acquire) > max_bytes => { + // maximum size is exceeded, the appender should rotate immediately return Some(next_date); } _ => {} } - // if the next date is 0, this appender *never* rotates log files. + // otherwise, if the next date is 0, this appender *never* rotates log files. if next_date == 0 { return None; } @@ -822,15 +959,30 @@ impl Inner { None } - fn advance_date(&self, now: OffsetDateTime, current: usize) -> bool { + fn advance_date_and_index(&self, now: OffsetDateTime, current: usize) -> bool { let next_date = self .rotation .next_date(&now) .map(|date| date.unix_timestamp() as usize) .unwrap_or(0); - self.next_date + let next_date_updated = self + .next_date .compare_exchange(current, next_date, Ordering::AcqRel, Ordering::Acquire) - .is_ok() + .is_ok(); + + if next_date_updated { + if let Some(index) = &self.log_filename_index { + if current == next_date { + index.fetch_add(1, Ordering::SeqCst); + } else { + index.store(0, Ordering::Release); + } + } + + true + } else { + false + } } } @@ -852,11 +1004,21 @@ fn create_writer(directory: &Path, filename: &str) -> Result { new_file.map_err(InitError::ctx("failed to create initial log file")) } +struct FilteredLogFile { + path: PathBuf, + metadata: fs::Metadata, + created_at: std::time::SystemTime, + /// This is the date found in the filename, rounded (as opposed to `created_at`) + formatted_date: Option, + index: Option, +} + #[cfg(test)] mod test { use super::*; use std::fs; use std::io::Write; + use std::ops::Deref; fn find_str_in_log(dir_path: &Path, expected_value: &str) -> bool { let dir_contents = fs::read_dir(dir_path).expect("Failed to read directory"); @@ -990,13 +1152,25 @@ mod test { let test_cases = vec![ // prefix only TestCase { - expected: "app.log.2020-02-01-10-01-00", + expected: "app.log.2020-02-01-10-01.0", + rotation: Rotation::minutely_with_max_bytes(1024), + prefix: Some("app.log"), + suffix: None, + }, + TestCase { + expected: "app.log.2020-02-01-10.0", + rotation: Rotation::hourly_with_max_bytes(1024), + prefix: Some("app.log"), + suffix: None, + }, + TestCase { + expected: "app.log.2020-02-01.0", rotation: Rotation::daily_with_max_bytes(1024), prefix: Some("app.log"), suffix: None, }, TestCase { - expected: "app.log.2020-02-01-10-01-00", + expected: "app.log.2020-02-01.0", rotation: Rotation::max_bytes(1024), prefix: Some("app.log"), suffix: None, @@ -1027,13 +1201,25 @@ mod test { }, // prefix and suffix TestCase { - expected: "app.2020-02-01-10-01-00.log", + expected: "app.2020-02-01-10-01.0.log", + rotation: Rotation::minutely_with_max_bytes(1024), + prefix: Some("app"), + suffix: Some("log"), + }, + TestCase { + expected: "app.2020-02-01-10.0.log", + rotation: Rotation::hourly_with_max_bytes(1024), + prefix: Some("app"), + suffix: Some("log"), + }, + TestCase { + expected: "app.2020-02-01.0.log", rotation: Rotation::daily_with_max_bytes(1024), prefix: Some("app"), suffix: Some("log"), }, TestCase { - expected: "app.2020-02-01-10-01-00.log", + expected: "app.2020-02-01.0.log", rotation: Rotation::max_bytes(1024), prefix: Some("app"), suffix: Some("log"), @@ -1064,13 +1250,25 @@ mod test { }, // suffix only TestCase { - expected: "2020-02-01-10-01-00.log", + expected: "2020-02-01-10-01.0.log", + rotation: Rotation::minutely_with_max_bytes(1024), + prefix: None, + suffix: Some("log"), + }, + TestCase { + expected: "2020-02-01-10.0.log", + rotation: Rotation::hourly_with_max_bytes(1024), + prefix: None, + suffix: Some("log"), + }, + TestCase { + expected: "2020-02-01.0.log", rotation: Rotation::daily_with_max_bytes(1024), prefix: None, suffix: Some("log"), }, TestCase { - expected: "2020-02-01-10-01-00.log", + expected: "2020-02-01.0.log", rotation: Rotation::max_bytes(1024), prefix: None, suffix: Some("log"), @@ -1099,6 +1297,55 @@ mod test { prefix: None, suffix: Some("log"), }, + // no suffix nor prefix + TestCase { + expected: "2020-02-01-10-01.0", + rotation: Rotation::minutely_with_max_bytes(1024), + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01-10.0", + rotation: Rotation::hourly_with_max_bytes(1024), + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01.0", + rotation: Rotation::daily_with_max_bytes(1024), + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01.0", + rotation: Rotation::max_bytes(1024), + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01-10-01", + rotation: Rotation::MINUTELY, + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01-10", + rotation: Rotation::HOURLY, + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01", + rotation: Rotation::DAILY, + prefix: None, + suffix: None, + }, + TestCase { + expected: "2020-02-01", + rotation: Rotation::NEVER, + prefix: None, + suffix: None, + }, ]; for test_case in test_cases { @@ -1294,29 +1541,46 @@ mod test { #[test] fn size_based_rotation() { + let format = format_description::parse("size_based_rotation.[year]-[month]-[day]").unwrap(); + let now = OffsetDateTime::now_utc(); + let directory = tempfile::tempdir().expect("failed to create tempdir"); let mut appender = max_size(&directory, "size_based_rotation", 8); - writeln!(appender, "more than 8 bytes").unwrap(); - std::thread::sleep(std::time::Duration::from_secs(1)); - writeln!(appender, "more than 8 bytes again").unwrap(); - std::thread::sleep(std::time::Duration::from_secs(1)); - writeln!(appender, "and here is a third file").unwrap(); - std::thread::sleep(std::time::Duration::from_secs(1)); + writeln!(appender, "(file1) more than 8 bytes").unwrap(); + writeln!(appender, "(file2) more than 8 bytes again").unwrap(); + writeln!(appender, "(file3) and here is a third file").unwrap(); - let dir_contents = fs::read_dir(&directory).expect("Failed to read directory"); + let dir_contents = fs::read_dir(&directory).expect("read directory"); println!("dir={:?}", dir_contents); - let mut count = 0; + let expected_files = [ + format!("{}.0", now.format(&format).unwrap()), + format!("{}.1", now.format(&format).unwrap()), + format!("{}.2", now.format(&format).unwrap()), + ]; + let mut expected_files: std::collections::HashSet<&str> = + expected_files.iter().map(|s| s.deref()).collect(); + for entry in dir_contents { println!("entry={:?}", entry); - let path = entry.expect("Expected dir entry").path(); - let file = fs::read_to_string(&path).expect("Failed to read file"); - println!("path={}\nfile={:?}", path.display(), file); - count += 1; + + let path = entry.expect("dir entry").path(); + let contents = fs::read_to_string(&path).expect("read file"); + println!("path={}\ncontents={:?}", path.display(), contents); + + let filename = path.file_name().unwrap().to_str().unwrap(); + expected_files + .remove(filename) + .then_some(()) + .expect("filename"); } - assert_eq!(count, 3); + assert!( + expected_files.is_empty(), + "missing file(s): {:?}", + expected_files + ); } } From 08eae52027771cbffa1655b5795d9f55506a00ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 5 Sep 2023 20:57:50 -0400 Subject: [PATCH 4/6] Code review 2 --- tracing-appender/src/rolling.rs | 70 +++++++++++---------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index 77a76e81d0..6ff4a00fba 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -541,34 +541,17 @@ impl Rotation { /// Provides a size-based rotation. pub const fn max_bytes(number_of_bytes: u64) -> Self { - Self { - timed: Timed::Never, - max_bytes: Some(number_of_bytes), - } + Self::NEVER.with_max_bytes(number_of_bytes) } - /// Provides a minutely and size-based rotation. - pub const fn minutely_with_max_bytes(number_of_bytes: u64) -> Self { - Self { - timed: Timed::Minutely, - max_bytes: Some(number_of_bytes), - } - } - - /// Provides a hourly and size-based rotation. - pub const fn hourly_with_max_bytes(number_of_bytes: u64) -> Self { - Self { - timed: Timed::Hourly, - max_bytes: Some(number_of_bytes), - } - } - - /// Provides a daily and size-based rotation. - pub const fn daily_with_max_bytes(number_of_bytes: u64) -> Self { - Self { - timed: Timed::Daily, - max_bytes: Some(number_of_bytes), - } + /// Adds a maximum size limit to an existing `Rotation`. + /// + /// The new `Rotation` will rotate the log file when the log file reaches + /// the specified size limit, or when the rotation period elapses, whichever + /// occurs first. + pub const fn with_max_bytes(mut self, number_of_bytes: u64) -> Self { + self.max_bytes = Some(number_of_bytes); + self } pub(crate) fn next_date(&self, current_date: &OffsetDateTime) -> Option { @@ -850,14 +833,9 @@ impl Inner { let latest_file = files .into_iter() - .filter(|log_file| { - log_file - .formatted_date - .as_ref() - .and_then(|formatted_date| { - most_recent_formatted_date.eq(formatted_date).then_some(()) - }) - .is_some() + .filter(|log_file| match &log_file.formatted_date { + Some(formatted_date) => most_recent_formatted_date.eq(formatted_date), + None => false, }) .max_by_key(|log_file| log_file.index); @@ -1153,19 +1131,19 @@ mod test { // prefix only TestCase { expected: "app.log.2020-02-01-10-01.0", - rotation: Rotation::minutely_with_max_bytes(1024), + rotation: Rotation::MINUTELY.with_max_bytes(1024), prefix: Some("app.log"), suffix: None, }, TestCase { expected: "app.log.2020-02-01-10.0", - rotation: Rotation::hourly_with_max_bytes(1024), + rotation: Rotation::HOURLY.with_max_bytes(1024), prefix: Some("app.log"), suffix: None, }, TestCase { expected: "app.log.2020-02-01.0", - rotation: Rotation::daily_with_max_bytes(1024), + rotation: Rotation::DAILY.with_max_bytes(1024), prefix: Some("app.log"), suffix: None, }, @@ -1202,19 +1180,19 @@ mod test { // prefix and suffix TestCase { expected: "app.2020-02-01-10-01.0.log", - rotation: Rotation::minutely_with_max_bytes(1024), + rotation: Rotation::MINUTELY.with_max_bytes(1024), prefix: Some("app"), suffix: Some("log"), }, TestCase { expected: "app.2020-02-01-10.0.log", - rotation: Rotation::hourly_with_max_bytes(1024), + rotation: Rotation::HOURLY.with_max_bytes(1024), prefix: Some("app"), suffix: Some("log"), }, TestCase { expected: "app.2020-02-01.0.log", - rotation: Rotation::daily_with_max_bytes(1024), + rotation: Rotation::DAILY.with_max_bytes(1024), prefix: Some("app"), suffix: Some("log"), }, @@ -1251,19 +1229,19 @@ mod test { // suffix only TestCase { expected: "2020-02-01-10-01.0.log", - rotation: Rotation::minutely_with_max_bytes(1024), + rotation: Rotation::MINUTELY.with_max_bytes(1024), prefix: None, suffix: Some("log"), }, TestCase { expected: "2020-02-01-10.0.log", - rotation: Rotation::hourly_with_max_bytes(1024), + rotation: Rotation::HOURLY.with_max_bytes(1024), prefix: None, suffix: Some("log"), }, TestCase { expected: "2020-02-01.0.log", - rotation: Rotation::daily_with_max_bytes(1024), + rotation: Rotation::DAILY.with_max_bytes(1024), prefix: None, suffix: Some("log"), }, @@ -1300,19 +1278,19 @@ mod test { // no suffix nor prefix TestCase { expected: "2020-02-01-10-01.0", - rotation: Rotation::minutely_with_max_bytes(1024), + rotation: Rotation::MINUTELY.with_max_bytes(1024), prefix: None, suffix: None, }, TestCase { expected: "2020-02-01-10.0", - rotation: Rotation::hourly_with_max_bytes(1024), + rotation: Rotation::HOURLY.with_max_bytes(1024), prefix: None, suffix: None, }, TestCase { expected: "2020-02-01.0", - rotation: Rotation::daily_with_max_bytes(1024), + rotation: Rotation::DAILY.with_max_bytes(1024), prefix: None, suffix: None, }, From 617c289c4ceae60b84c18bc90e7b4f3fe1195cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 6 Sep 2023 11:57:05 -0400 Subject: [PATCH 5/6] Harden size-based rotation test (use a constant OffsetDateTime) --- tracing-appender/src/rolling.rs | 72 ++++++++++++++++--------- tracing-appender/src/rolling/builder.rs | 9 ++++ 2 files changed, 57 insertions(+), 24 deletions(-) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index 6ff4a00fba..42560667ae 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -33,10 +33,7 @@ use std::{ fs::{self, File, OpenOptions}, io::{self, Write}, path::{Path, PathBuf}, - sync::{ - atomic::{AtomicU64, AtomicUsize, Ordering}, - Arc, - }, + sync::atomic::{AtomicU64, AtomicUsize, Ordering}, }; use time::{format_description, Date, Duration, OffsetDateTime, Time}; @@ -115,7 +112,7 @@ struct Inner { date_format: Vec>, rotation: Rotation, next_date: AtomicUsize, - current_size: Arc, + current_size: AtomicU64, max_files: Option, } @@ -194,27 +191,53 @@ impl RollingFileAppender { } fn from_builder(builder: &Builder, directory: impl AsRef) -> Result { + Self::impl_from_builder( + builder, + directory, + #[cfg(test)] + Box::new(OffsetDateTime::now_utc), + ) + } + + #[cfg(test)] + fn from_builder_with_custom_now( + builder: &Builder, + directory: impl AsRef, + now: Box OffsetDateTime + Send + Sync>, + ) -> Result { + Self::impl_from_builder(builder, directory, now) + } + + fn impl_from_builder( + builder: &Builder, + directory: impl AsRef, + #[cfg(test)] now: Box OffsetDateTime + Send + Sync>, + ) -> Result { let Builder { - ref rotation, - ref prefix, - ref suffix, - ref max_files, - } = builder; + rotation, + prefix, + suffix, + max_files, + } = &builder; + let directory = directory.as_ref().to_path_buf(); - let now = OffsetDateTime::now_utc(); let (state, writer) = Inner::new( - now, + #[cfg(not(test))] + OffsetDateTime::now_utc(), + #[cfg(test)] + now(), rotation.clone(), directory, prefix.clone(), suffix.clone(), *max_files, )?; + Ok(Self { state, writer, #[cfg(test)] - now: Box::new(OffsetDateTime::now_utc), + now, }) } @@ -645,7 +668,7 @@ impl Inner { ), rotation, max_files, - current_size: Arc::new(AtomicU64::new(0)), + current_size: AtomicU64::new(0), }; if let Some(max_bytes) = inner.rotation.max_bytes { @@ -660,7 +683,7 @@ impl Inner { .metadata() .map_err(builder::InitError::ctx("Failed to read file metadata"))? .len(); - inner.current_size = Arc::new(AtomicU64::new(current_file_size)); + inner.current_size = AtomicU64::new(current_file_size); let writer = RwLock::new(file); @@ -996,7 +1019,6 @@ mod test { use super::*; use std::fs; use std::io::Write; - use std::ops::Deref; fn find_str_in_log(dir_path: &Path, expected_value: &str) -> bool { let dir_contents = fs::read_dir(dir_path).expect("Failed to read directory"); @@ -1519,12 +1541,14 @@ mod test { #[test] fn size_based_rotation() { - let format = format_description::parse("size_based_rotation.[year]-[month]-[day]").unwrap(); - let now = OffsetDateTime::now_utc(); - let directory = tempfile::tempdir().expect("failed to create tempdir"); - let mut appender = max_size(&directory, "size_based_rotation", 8); + let mut appender = RollingFileAppender::builder() + .rotation(Rotation::max_bytes(8)) + .filename_prefix("size_based_rotation") + .filename_suffix("log") + .build_with_now(&directory, Box::new(|| OffsetDateTime::UNIX_EPOCH)) + .expect("initializing rolling file appender failed"); writeln!(appender, "(file1) more than 8 bytes").unwrap(); writeln!(appender, "(file2) more than 8 bytes again").unwrap(); @@ -1534,12 +1558,12 @@ mod test { println!("dir={:?}", dir_contents); let expected_files = [ - format!("{}.0", now.format(&format).unwrap()), - format!("{}.1", now.format(&format).unwrap()), - format!("{}.2", now.format(&format).unwrap()), + "size_based_rotation.1970-01-01.0.log", + "size_based_rotation.1970-01-01.1.log", + "size_based_rotation.1970-01-01.2.log", ]; let mut expected_files: std::collections::HashSet<&str> = - expected_files.iter().map(|s| s.deref()).collect(); + expected_files.iter().copied().collect(); for entry in dir_contents { println!("entry={:?}", entry); diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs index 8c92ca1238..b8c05a6670 100644 --- a/tracing-appender/src/rolling/builder.rs +++ b/tracing-appender/src/rolling/builder.rs @@ -264,6 +264,15 @@ impl Builder { pub fn build(&self, directory: impl AsRef) -> Result { RollingFileAppender::from_builder(self, directory) } + + #[cfg(test)] + pub(super) fn build_with_now( + &self, + directory: impl AsRef, + now: Box time::OffsetDateTime + Send + Sync>, + ) -> Result { + RollingFileAppender::from_builder_with_custom_now(self, directory, now) + } } impl Default for Builder { From 717ced15217178e19180eba9d7b58a6e285e8778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 6 Sep 2023 16:32:22 -0400 Subject: [PATCH 6/6] Refactor advance_date_and_index a bit --- tracing-appender/src/rolling.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index 42560667ae..bba1088c4a 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -971,19 +971,18 @@ impl Inner { .compare_exchange(current, next_date, Ordering::AcqRel, Ordering::Acquire) .is_ok(); - if next_date_updated { - if let Some(index) = &self.log_filename_index { + match &self.log_filename_index { + Some(index) if next_date_updated => { if current == next_date { index.fetch_add(1, Ordering::SeqCst); } else { index.store(0, Ordering::Release); } } - - true - } else { - false + _ => {} } + + next_date_updated } }