From 06e280373923410c7e65ee27c82c836f913182c8 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Mon, 22 Aug 2022 15:32:10 +0300
Subject: [PATCH 01/28] keep_last_n

---
 tracing-appender/Cargo.toml     |  2 +-
 tracing-appender/src/rolling.rs | 83 ++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/tracing-appender/Cargo.toml b/tracing-appender/Cargo.toml
index c7716a2564..dffb6253d7 100644
--- a/tracing-appender/Cargo.toml
+++ b/tracing-appender/Cargo.toml
@@ -22,7 +22,7 @@ rust-version = "1.53.0"
 
 [dependencies]
 crossbeam-channel = "0.5.6"
-time = { version = "0.3.2", default-features = false, features = ["formatting"] }
+time = { version = "0.3.2", default-features = false, features = ["formatting", "parsing"] }
 parking_lot = { optional = true, version = "0.12.1" }
 thiserror = "1"
 
diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 8e9597c29a..5977544141 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -34,7 +34,7 @@ use std::{
     path::{Path, PathBuf},
     sync::atomic::{AtomicUsize, Ordering},
 };
-use time::{format_description, Duration, OffsetDateTime, Time};
+use time::{format_description, Date, Duration, OffsetDateTime, Time};
 
 mod builder;
 pub use builder::{Builder, InitError};
@@ -106,6 +106,7 @@ struct Inner {
     log_filename_suffix: Option<String>,
     rotation: Rotation,
     next_date: AtomicUsize,
+    keep_last: Option<usize>,
 }
 
 // === impl RollingFileAppender ===
@@ -152,6 +153,13 @@ impl RollingFileAppender {
             .expect("initializing rolling file appender failed")
     }
 
+    /// Keep the last `n` log entries on disk.
+    ///
+    /// If no value is supplied, `RollingAppender` will not remove any files.
+    pub fn keep_last_n_logs(&mut self, n: usize) {
+        self.state.keep_last = Some(n);
+    }
+
     /// Returns a new [`Builder`] for configuring a `RollingFileAppender`.
     ///
     /// The builder interface can be used to set additional configuration
@@ -558,10 +566,79 @@ impl Inner {
                     .unwrap_or(0),
             ),
             rotation,
+            keep_last: None,
         };
         Ok((inner, writer))
     }
 
+    fn prune_old_logs(&self, keep_last: usize) {
+        let format;
+        match self.rotation {
+            Rotation::MINUTELY => {
+                format = format_description::parse("[year]-[month]-[day]-[hour]-[minute]")
+                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
+            }
+            Rotation::HOURLY => {
+                format = format_description::parse("[year]-[month]-[day]-[hour]")
+                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
+            }
+            Rotation::DAILY => {
+                format = format_description::parse("[year]-[month]-[day]")
+                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
+            }
+            Rotation::NEVER => {
+                unreachable!("keep last N files feature is not available for `Rotation::NEVER`!")
+            }
+        }
+
+        let files = fs::read_dir(&self.log_directory).map(|dir| {
+            dir.filter_map(Result::ok)
+                .filter(|entry| {
+                    entry.file_name().to_string_lossy().contains(
+                        &self
+                            .log_filename_prefix
+                            .clone()
+                            .expect("prefix is always present"),
+                    )
+                })
+                .collect::<Vec<_>>()
+        });
+
+        match files {
+            Ok(files) => {
+                if files.len() >= keep_last {
+                    let mut sorted_files = vec![];
+                    for file in files {
+                        let filename = file.file_name().to_string_lossy().to_string();
+                        match filename.rfind('.') {
+                            Some(date_index) => {
+                                let date_str = filename[(date_index + 1)..].to_string();
+                                match Date::parse(&date_str, &format) {
+                                    Ok(date) => sorted_files.push((file.path(), date)),
+                                    Err(error) => eprintln!("error parsing the date: {error}"),
+                                }
+                            }
+                            None => {
+                                eprintln!("Filename do not meet the expected format!");
+                                continue;
+                            }
+                        }
+                    }
+                    sorted_files.sort_by_key(|item| item.1);
+                    // delete files, so that (n-1) files remain, because we will create another log file
+                    for (file, _) in &sorted_files[..sorted_files.len() - (keep_last - 1)] {
+                        if let Err(error) = fs::remove_file(file) {
+                            eprintln!("Failed to remove old log file: {error}");
+                        }
+                    }
+                }
+            }
+            Err(error) => {
+                eprintln!("Error reading the log directory/files: {error}")
+            }
+        }
+    }
+
     fn refresh_writer(&self, now: OffsetDateTime, file: &mut File) {
         let filename = self.rotation.join_date(
             self.log_filename_prefix.as_deref(),
@@ -569,6 +646,10 @@ impl Inner {
             self.log_filename_suffix.as_deref(),
         );
 
+        if let Some(keep_last) = self.keep_last {
+            self.prune_old_logs(keep_last);
+        }
+
         match create_writer(&self.log_directory, &filename) {
             Ok(new_file) => {
                 if let Err(err) = file.flush() {

From a98a62751d8e068026ada53b276eb2eaa0ba6233 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 11:07:28 +0300
Subject: [PATCH 02/28] Update tracing-appender/src/rolling.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling.rs | 40 ++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 5977544141..662c5174f2 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -605,28 +605,28 @@ impl Inner {
         });
 
         match files {
-            Ok(files) => {
+            Ok(mut files) => {
                 if files.len() >= keep_last {
-                    let mut sorted_files = vec![];
-                    for file in files {
-                        let filename = file.file_name().to_string_lossy().to_string();
-                        match filename.rfind('.') {
-                            Some(date_index) => {
-                                let date_str = filename[(date_index + 1)..].to_string();
-                                match Date::parse(&date_str, &format) {
-                                    Ok(date) => sorted_files.push((file.path(), date)),
-                                    Err(error) => eprintln!("error parsing the date: {error}"),
-                                }
-                            }
-                            None => {
-                                eprintln!("Filename do not meet the expected format!");
-                                continue;
-                            }
-                        }
-                    }
-                    sorted_files.sort_by_key(|item| item.1);
+                    // sort the files by their creation timestamps.
+                    files.sort_by_key(|file| {
+                        file.metadata()
+                            .and_then(|metadata| metadata.created().map(Some))
+                            .unwrap_or_else(|error| {
+                                eprintln!(
+                                    "Unable to get file creation time for {}: {}",
+                                    file.path().display(),
+                                    error
+                                );
+                                // if we couldn't determine the file's creation
+                                // timestamp for whatever reason, it should
+                                // compare as "less than" other files, so that
+                                // it's not deleted.
+                                None
+                            })
+                    });
+
                     // delete files, so that (n-1) files remain, because we will create another log file
-                    for (file, _) in &sorted_files[..sorted_files.len() - (keep_last - 1)] {
+                    for file in &files[..files.len() - (keep_last - 1)] {
                         if let Err(error) = fs::remove_file(file) {
                             eprintln!("Failed to remove old log file: {error}");
                         }

From 30735ce547850bae6c64e03ea8f3c20e8726cb2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 11:07:49 +0300
Subject: [PATCH 03/28] Update tracing-appender/src/rolling.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 662c5174f2..e4c1c8c875 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -628,7 +628,7 @@ impl Inner {
                     // delete files, so that (n-1) files remain, because we will create another log file
                     for file in &files[..files.len() - (keep_last - 1)] {
                         if let Err(error) = fs::remove_file(file) {
-                            eprintln!("Failed to remove old log file: {error}");
+                            eprintln!("Failed to remove old log file: {}", error);
                         }
                     }
                 }

From 15f695b6984759b3ea76054d47dad0b7f220c51c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 11:08:01 +0300
Subject: [PATCH 04/28] Update tracing-appender/src/rolling.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index e4c1c8c875..aec5f6b80d 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -634,7 +634,7 @@ impl Inner {
                 }
             }
             Err(error) => {
-                eprintln!("Error reading the log directory/files: {error}")
+                eprintln!("Error reading the log directory/files: {}", error)
             }
         }
     }

From e089385abec43c8c0a998a35178c09e45ba1f202 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 11:17:25 +0300
Subject: [PATCH 05/28] Update tracing-appender/src/rolling.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index aec5f6b80d..1019cbec56 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -34,7 +34,7 @@ use std::{
     path::{Path, PathBuf},
     sync::atomic::{AtomicUsize, Ordering},
 };
-use time::{format_description, Date, Duration, OffsetDateTime, Time};
+use time::{format_description, Duration, OffsetDateTime, Time};
 
 mod builder;
 pub use builder::{Builder, InitError};

From d544f8852524b0b4129830ab2dc1cc8860160dbf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 11:17:45 +0300
Subject: [PATCH 06/28] Update tracing-appender/Cargo.toml

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/Cargo.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracing-appender/Cargo.toml b/tracing-appender/Cargo.toml
index dffb6253d7..c7716a2564 100644
--- a/tracing-appender/Cargo.toml
+++ b/tracing-appender/Cargo.toml
@@ -22,7 +22,7 @@ rust-version = "1.53.0"
 
 [dependencies]
 crossbeam-channel = "0.5.6"
-time = { version = "0.3.2", default-features = false, features = ["formatting", "parsing"] }
+time = { version = "0.3.2", default-features = false, features = ["formatting"] }
 parking_lot = { optional = true, version = "0.12.1" }
 thiserror = "1"
 

From 7d9a275d6fe2f7fe1ee21056bed5ba8980345730 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 12:31:38 +0300
Subject: [PATCH 07/28] keep_last moved to builder

---
 tracing-appender/src/rolling.rs         | 89 +++++++++++--------------
 tracing-appender/src/rolling/builder.rs | 26 ++++++++
 2 files changed, 64 insertions(+), 51 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 1019cbec56..f692d897d8 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -195,6 +195,7 @@ impl RollingFileAppender {
             ref rotation,
             ref prefix,
             ref suffix,
+            ref keep_last,
         } = builder;
         let directory = directory.as_ref().to_path_buf();
         let now = OffsetDateTime::now_utc();
@@ -204,6 +205,7 @@ impl RollingFileAppender {
             directory,
             prefix.clone(),
             suffix.clone(),
+            keep_last.clone(),
         )?;
         Ok(Self {
             state,
@@ -546,6 +548,7 @@ impl Inner {
         directory: impl AsRef<Path>,
         log_filename_prefix: Option<String>,
         log_filename_suffix: Option<String>,
+        keep_last: Option<usize>,
     ) -> Result<(Self, RwLock<File>), builder::InitError> {
         let log_directory = directory.as_ref().to_path_buf();
         let filename = rotation.join_date(
@@ -566,31 +569,12 @@ impl Inner {
                     .unwrap_or(0),
             ),
             rotation,
-            keep_last: None,
+            keep_last,
         };
         Ok((inner, writer))
     }
 
     fn prune_old_logs(&self, keep_last: usize) {
-        let format;
-        match self.rotation {
-            Rotation::MINUTELY => {
-                format = format_description::parse("[year]-[month]-[day]-[hour]-[minute]")
-                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
-            }
-            Rotation::HOURLY => {
-                format = format_description::parse("[year]-[month]-[day]-[hour]")
-                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
-            }
-            Rotation::DAILY => {
-                format = format_description::parse("[year]-[month]-[day]")
-                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
-            }
-            Rotation::NEVER => {
-                unreachable!("keep last N files feature is not available for `Rotation::NEVER`!")
-            }
-        }
-
         let files = fs::read_dir(&self.log_directory).map(|dir| {
             dir.filter_map(Result::ok)
                 .filter(|entry| {
@@ -603,38 +587,40 @@ impl Inner {
                 })
                 .collect::<Vec<_>>()
         });
-
-        match files {
-            Ok(mut files) => {
-                if files.len() >= keep_last {
-                    // sort the files by their creation timestamps.
-                    files.sort_by_key(|file| {
-                        file.metadata()
-                            .and_then(|metadata| metadata.created().map(Some))
-                            .unwrap_or_else(|error| {
-                                eprintln!(
-                                    "Unable to get file creation time for {}: {}",
-                                    file.path().display(),
-                                    error
-                                );
-                                // if we couldn't determine the file's creation
-                                // timestamp for whatever reason, it should
-                                // compare as "less than" other files, so that
-                                // it's not deleted.
-                                None
-                            })
-                    });
-
-                    // delete files, so that (n-1) files remain, because we will create another log file
-                    for file in &files[..files.len() - (keep_last - 1)] {
-                        if let Err(error) = fs::remove_file(file) {
-                            eprintln!("Failed to remove old log file: {}", error);
-                        }
-                    }
-                }
-            }
+        let mut files = match files {
+            Ok(files) => files,
             Err(error) => {
-                eprintln!("Error reading the log directory/files: {}", error)
+                eprintln!("Error reading the log directory/files: {}", error);
+                return;
+            }
+        };
+        if files.len() < keep_last {
+            return;
+        }
+        // sort the files by their creation timestamps.
+        files.sort_by_key(|file| {
+            file.metadata()
+                .and_then(|metadata| metadata.created().map(Some))
+                .unwrap_or_else(|error| {
+                    eprintln!(
+                        "Unable to get file creation time for {}: {}",
+                        file.path().display(),
+                        error
+                    );
+                    // if we couldn't determine the file's creation timestamp
+                    // for whatever reason, it should compare as "less than"
+                    // other files, so that it's not deleted.
+                    None
+                })
+        });
+        // delete files, so that (n-1) files remain, because we will create another log file
+        for file in &files[..files.len() - (keep_last - 1)] {
+            if let Err(error) = fs::remove_file(file.path()) {
+                eprintln!(
+                    "Failed to remove old log file {}: {}",
+                    file.path().display(),
+                    error
+                );
             }
         }
     }
@@ -885,6 +871,7 @@ mod test {
             directory.path(),
             Some("test_make_writer".to_string()),
             None,
+            None,
         )
         .unwrap();
 
diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index ea5d39cd5f..2a540d32f1 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -10,6 +10,7 @@ pub struct Builder {
     pub(super) rotation: Rotation,
     pub(super) prefix: Option<String>,
     pub(super) suffix: Option<String>,
+    pub(super) keep_last: Option<usize>,
 }
 
 /// Errors returned by [`Builder::build`].
@@ -48,6 +49,7 @@ impl Builder {
             rotation: Rotation::NEVER,
             prefix: None,
             suffix: None,
+            keep_last: None,
         }
     }
 
@@ -181,6 +183,30 @@ impl Builder {
         Self { suffix, ..self }
     }
 
+    /// Keep the last `n` log entries on disk.
+    ///
+    /// # Examples
+    ///
+    /// Setting a suffix:
+    ///
+    /// ```
+    /// use tracing_appender::rolling::RollingFileAppender;
+    ///
+    /// # fn docs() {
+    /// let appender = RollingFileAppender::builder()
+    ///     .keep_last_n_logs(5) // only the most recent 5 logs files will be kept
+    ///     // ...
+    ///     .build("/var/log")
+    ///     .expect("failed to initialize rolling file appender");
+    /// # drop(appender)
+    /// # }
+    /// ```
+    ///
+    /// If no value is supplied, `RollingAppender` will not remove any files.
+    pub fn keep_last_n_logs(&mut self, n: usize) {
+        self.keep_last = Some(n);
+    }
+
     /// Builds a new [`RollingFileAppender`] with the configured parameters,
     /// emitting log files to the provided directory.
     ///

From 1d0d5eaecbf17b4f143c727e53d0fd4bef0d512f Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 12:43:28 +0300
Subject: [PATCH 08/28] builder method fix, and clippy fixes

---
 tracing-appender/src/rolling.rs         | 6 +++---
 tracing-appender/src/rolling/builder.rs | 7 +++++--
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index f692d897d8..0db1a35b58 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -190,7 +190,7 @@ impl RollingFileAppender {
         Builder::new()
     }
 
-    fn from_builder(builder: &Builder, directory: impl AsRef<Path>) -> Result<Self, InitError> {
+    fn from_builder(builder: Builder, directory: impl AsRef<Path>) -> Result<Self, InitError> {
         let Builder {
             ref rotation,
             ref prefix,
@@ -205,7 +205,7 @@ impl RollingFileAppender {
             directory,
             prefix.clone(),
             suffix.clone(),
-            keep_last.clone(),
+            *keep_last,
         )?;
         Ok(Self {
             state,
@@ -252,7 +252,7 @@ impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for RollingFileAppender
             // 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) {
-                self.state.refresh_writer(now, &mut *self.writer.write());
+                self.state.refresh_writer(now, &mut self.writer.write());
             }
         }
         RollingWriter(self.writer.read())
diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index 2a540d32f1..511d9725c6 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -203,8 +203,11 @@ impl Builder {
     /// ```
     ///
     /// If no value is supplied, `RollingAppender` will not remove any files.
-    pub fn keep_last_n_logs(&mut self, n: usize) {
-        self.keep_last = Some(n);
+    pub fn keep_last_n_logs(self, n: usize) -> Self {
+        Self {
+            keep_last: Some(n),
+            ..self
+        }
     }
 
     /// Builds a new [`RollingFileAppender`] with the configured parameters,

From 2881d40d8bfc4b6a5ce6fa1e9209e19e918d89a1 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Thu, 22 Sep 2022 12:48:22 +0300
Subject: [PATCH 09/28] typo

---
 tracing-appender/src/rolling.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 0db1a35b58..54c224a4d9 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -190,7 +190,7 @@ impl RollingFileAppender {
         Builder::new()
     }
 
-    fn from_builder(builder: Builder, directory: impl AsRef<Path>) -> Result<Self, InitError> {
+    fn from_builder(builder: &Builder, directory: impl AsRef<Path>) -> Result<Self, InitError> {
         let Builder {
             ref rotation,
             ref prefix,

From 6a82706ffa7464902b66180495aa1321a1594f5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 15:11:08 +0300
Subject: [PATCH 10/28] Update tracing-appender/src/rolling/builder.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 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 511d9725c6..baf02f33e3 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -183,7 +183,7 @@ impl Builder {
         Self { suffix, ..self }
     }
 
-    /// Keep the last `n` log entries on disk.
+    /// Keep the last `n` log files on disk.
     ///
     /// # Examples
     ///

From 50034c2a8bebe31b03d4ac0bdf0f1fafa0dc09da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 15:11:20 +0300
Subject: [PATCH 11/28] Update tracing-appender/src/rolling/builder.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling/builder.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index baf02f33e3..030a048497 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -185,6 +185,8 @@ impl Builder {
 
     /// Keep the last `n` log files on disk.
     ///
+    /// When a new log file is created, if there are `n` or more
+    /// existing log files in the directory, the oldest will be deleted.
     /// # Examples
     ///
     /// Setting a suffix:

From cc1091cf62cd46002c6429cca746eedee8415853 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 15:11:38 +0300
Subject: [PATCH 12/28] Update tracing-appender/src/rolling/builder.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 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 030a048497..25995b02e4 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -196,7 +196,7 @@ impl Builder {
     ///
     /// # fn docs() {
     /// let appender = RollingFileAppender::builder()
-    ///     .keep_last_n_logs(5) // only the most recent 5 logs files will be kept
+    ///     .keep_last_n_logs(5) // only the most recent 5 log files will be kept
     ///     // ...
     ///     .build("/var/log")
     ///     .expect("failed to initialize rolling file appender");

From 803f48d3f5d348c960a7cad13f4c11a17d1a406f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 15:11:46 +0300
Subject: [PATCH 13/28] Update tracing-appender/src/rolling/builder.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling/builder.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index 25995b02e4..02de6d4b59 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -189,8 +189,6 @@ impl Builder {
     /// existing log files in the directory, the oldest will be deleted.
     /// # Examples
     ///
-    /// Setting a suffix:
-    ///
     /// ```
     /// use tracing_appender::rolling::RollingFileAppender;
     ///

From 13a8c5a7972cc0e8a28b479d0c980203b2f4225c Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 16:23:57 +0300
Subject: [PATCH 14/28] suggestions

---
 tracing-appender/src/rolling.rs         | 49 ++++++++++---------------
 tracing-appender/src/rolling/builder.rs | 15 ++++----
 2 files changed, 28 insertions(+), 36 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 54c224a4d9..684594d531 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -106,7 +106,7 @@ struct Inner {
     log_filename_suffix: Option<String>,
     rotation: Rotation,
     next_date: AtomicUsize,
-    keep_last: Option<usize>,
+    max_files: Option<usize>,
 }
 
 // === impl RollingFileAppender ===
@@ -153,13 +153,6 @@ impl RollingFileAppender {
             .expect("initializing rolling file appender failed")
     }
 
-    /// Keep the last `n` log entries on disk.
-    ///
-    /// If no value is supplied, `RollingAppender` will not remove any files.
-    pub fn keep_last_n_logs(&mut self, n: usize) {
-        self.state.keep_last = Some(n);
-    }
-
     /// Returns a new [`Builder`] for configuring a `RollingFileAppender`.
     ///
     /// The builder interface can be used to set additional configuration
@@ -195,7 +188,7 @@ impl RollingFileAppender {
             ref rotation,
             ref prefix,
             ref suffix,
-            ref keep_last,
+            ref max_files,
         } = builder;
         let directory = directory.as_ref().to_path_buf();
         let now = OffsetDateTime::now_utc();
@@ -205,7 +198,7 @@ impl RollingFileAppender {
             directory,
             prefix.clone(),
             suffix.clone(),
-            *keep_last,
+            *max_files,
         )?;
         Ok(Self {
             state,
@@ -548,7 +541,7 @@ impl Inner {
         directory: impl AsRef<Path>,
         log_filename_prefix: Option<String>,
         log_filename_suffix: Option<String>,
-        keep_last: Option<usize>,
+        max_files: Option<usize>,
     ) -> Result<(Self, RwLock<File>), builder::InitError> {
         let log_directory = directory.as_ref().to_path_buf();
         let filename = rotation.join_date(
@@ -569,12 +562,12 @@ impl Inner {
                     .unwrap_or(0),
             ),
             rotation,
-            keep_last,
+            max_files,
         };
         Ok((inner, writer))
     }
 
-    fn prune_old_logs(&self, keep_last: usize) {
+    fn prune_old_logs(&self, max_files: usize) {
         let files = fs::read_dir(&self.log_directory).map(|dir| {
             dir.filter_map(Result::ok)
                 .filter(|entry| {
@@ -594,27 +587,25 @@ impl Inner {
                 return;
             }
         };
-        if files.len() < keep_last {
+        if files.len() < max_files {
             return;
         }
+
         // sort the files by their creation timestamps.
         files.sort_by_key(|file| {
-            file.metadata()
-                .and_then(|metadata| metadata.created().map(Some))
-                .unwrap_or_else(|error| {
-                    eprintln!(
-                        "Unable to get file creation time for {}: {}",
-                        file.path().display(),
-                        error
-                    );
+            match file.metadata().and_then(|metadata| metadata.created()) {
+                Ok(val) => val,
+                Err(why) => {
+                    println!("failed reading the timestamp because: {}", why);
                     // if we couldn't determine the file's creation timestamp
-                    // for whatever reason, it should compare as "less than"
-                    // other files, so that it's not deleted.
-                    None
-                })
+                    // for whatever reason, we should return early
+                    return;
+                }
+            };
         });
+
         // delete files, so that (n-1) files remain, because we will create another log file
-        for file in &files[..files.len() - (keep_last - 1)] {
+        for file in &files[..files.len() - (max_files - 1)] {
             if let Err(error) = fs::remove_file(file.path()) {
                 eprintln!(
                     "Failed to remove old log file {}: {}",
@@ -632,8 +623,8 @@ impl Inner {
             self.log_filename_suffix.as_deref(),
         );
 
-        if let Some(keep_last) = self.keep_last {
-            self.prune_old_logs(keep_last);
+        if let Some(max_files) = self.max_files {
+            self.prune_old_logs(max_files);
         }
 
         match create_writer(&self.log_directory, &filename) {
diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index 02de6d4b59..b4dadab7fc 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -10,7 +10,7 @@ pub struct Builder {
     pub(super) rotation: Rotation,
     pub(super) prefix: Option<String>,
     pub(super) suffix: Option<String>,
-    pub(super) keep_last: Option<usize>,
+    pub(super) max_files: Option<usize>,
 }
 
 /// Errors returned by [`Builder::build`].
@@ -49,7 +49,7 @@ impl Builder {
             rotation: Rotation::NEVER,
             prefix: None,
             suffix: None,
-            keep_last: None,
+            max_files: None,
         }
     }
 
@@ -187,6 +187,8 @@ impl Builder {
     ///
     /// When a new log file is created, if there are `n` or more
     /// existing log files in the directory, the oldest will be deleted.
+    /// If no value is supplied, `RollingAppender` will not remove any files.
+    ///
     /// # Examples
     ///
     /// ```
@@ -194,18 +196,17 @@ impl Builder {
     ///
     /// # fn docs() {
     /// let appender = RollingFileAppender::builder()
-    ///     .keep_last_n_logs(5) // only the most recent 5 log files will be kept
+    ///     .max_log_files(5) // only the most recent 5 log files will be kept
     ///     // ...
     ///     .build("/var/log")
     ///     .expect("failed to initialize rolling file appender");
     /// # drop(appender)
     /// # }
     /// ```
-    ///
-    /// If no value is supplied, `RollingAppender` will not remove any files.
-    pub fn keep_last_n_logs(self, n: usize) -> Self {
+    #[must_use]
+    pub fn max_log_files(self, n: usize) -> Self {
         Self {
-            keep_last: Some(n),
+            max_files: Some(n),
             ..self
         }
     }

From e6b69afcb612bccdbef721c97d953f3c6794e0e7 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 18:21:15 +0300
Subject: [PATCH 15/28] logic fix

---
 tracing-appender/src/rolling.rs | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 684594d531..a6b05d4361 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -578,6 +578,12 @@ impl Inner {
                             .expect("prefix is always present"),
                     )
                 })
+                .filter(|entry| {
+                    entry
+                        .metadata()
+                        .and_then(|metadata| metadata.created())
+                        .is_ok()
+                })
                 .collect::<Vec<_>>()
         });
         let mut files = match files {
@@ -593,15 +599,9 @@ impl Inner {
 
         // sort the files by their creation timestamps.
         files.sort_by_key(|file| {
-            match file.metadata().and_then(|metadata| metadata.created()) {
-                Ok(val) => val,
-                Err(why) => {
-                    println!("failed reading the timestamp because: {}", why);
-                    // if we couldn't determine the file's creation timestamp
-                    // for whatever reason, we should return early
-                    return;
-                }
-            };
+            file.metadata()
+                .and_then(|metadata| metadata.created())
+                .expect("metadata is already read above")
         });
 
         // delete files, so that (n-1) files remain, because we will create another log file

From 371fc84480f8387113c43c0dc3cd63db206985c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 20:20:04 +0300
Subject: [PATCH 16/28] Update tracing-appender/src/rolling.rs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Benoît Cortier <bcortier@proton.me>
---
 tracing-appender/src/rolling.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index a6b05d4361..b50bfead38 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -578,11 +578,12 @@ impl Inner {
                             .expect("prefix is always present"),
                     )
                 })
-                .filter(|entry| {
-                    entry
+                .filter_map(|entry| {
+                    let created = entry
                         .metadata()
                         .and_then(|metadata| metadata.created())
-                        .is_ok()
+                        .ok()?;
+                    Some((entry, created))
                 })
                 .collect::<Vec<_>>()
         });

From 0f8f808ac56c0abf7dbb1d14e54128dc3a0d2075 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 20:24:07 +0300
Subject: [PATCH 17/28] remainder of the suggestion

---
 tracing-appender/src/rolling.rs | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index b50bfead38..699f802f07 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -599,18 +599,14 @@ impl Inner {
         }
 
         // sort the files by their creation timestamps.
-        files.sort_by_key(|file| {
-            file.metadata()
-                .and_then(|metadata| metadata.created())
-                .expect("metadata is already read above")
-        });
+        files.sort_by_key(|file| file.1);
 
         // delete files, so that (n-1) files remain, because we will create another log file
         for file in &files[..files.len() - (max_files - 1)] {
-            if let Err(error) = fs::remove_file(file.path()) {
+            if let Err(error) = fs::remove_file(file.0.path()) {
                 eprintln!(
                     "Failed to remove old log file {}: {}",
-                    file.path().display(),
+                    file.0.path().display(),
                     error
                 );
             }
@@ -786,7 +782,7 @@ mod test {
     }
 
     #[test]
-    fn test_path_concatination() {
+    fn test_path_concatenation() {
         let format = format_description::parse(
             "[year]-[month]-[day] [hour]:[minute]:[second] [offset_hour \
          sign:mandatory]:[offset_minute]:[offset_second]",

From 13900ef09d5f33836c6bf014a321c53a42c14587 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 20:39:53 +0300
Subject: [PATCH 18/28] test for max_log_files

---
 tracing-appender/src/rolling.rs | 96 +++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 699f802f07..b604ce44e7 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -921,4 +921,100 @@ mod test {
             }
         }
     }
+
+    #[test]
+    fn test_max_log_files() {
+        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();
+
+        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::HOURLY,
+            directory.path(),
+            Some("test_max_log_files".to_string()),
+            None,
+            Some(2),
+        )
+        .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();
+
+        tracing::info!("file 1");
+
+        // advance time by one second
+        (*clock.lock().unwrap()) += Duration::seconds(1);
+
+        tracing::info!("file 1");
+
+        // advance time by one hour
+        (*clock.lock().unwrap()) += Duration::hours(1);
+
+        tracing::info!("file 2");
+
+        // advance time by one second
+        (*clock.lock().unwrap()) += Duration::seconds(1);
+
+        tracing::info!("file 2");
+
+        // advance time by one hour
+        (*clock.lock().unwrap()) += Duration::hours(1);
+
+        tracing::info!("file 3");
+
+        // advance time by one second
+        (*clock.lock().unwrap()) += Duration::seconds(1);
+
+        tracing::info!("file 3");
+
+        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 = fs::read_to_string(&path).expect("Failed to read file");
+            println!("path={}\nfile={:?}", path.display(), file);
+
+            match path
+                .extension()
+                .expect("found a file without a date!")
+                .to_str()
+                .expect("extension should be UTF8")
+            {
+                "2020-02-01-10" => {
+                    panic!("this file should have been pruned already!");
+                }
+                "2020-02-01-11" => {
+                    assert_eq!("file 2\nfile 2\n", file);
+                }
+                "2020-02-01-12" => {
+                    assert_eq!("file 3\nfile 3\n", file);
+                }
+                x => panic!("unexpected date {}", x),
+            }
+        }
+    }
 }

From c52d1dfee85728ecbae8aff290bb448e5641141f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 21:18:49 +0300
Subject: [PATCH 19/28] Update tracing-appender/src/rolling.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index b604ce44e7..277df0ca51 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -599,14 +599,14 @@ impl Inner {
         }
 
         // sort the files by their creation timestamps.
-        files.sort_by_key(|file| file.1);
+        files.sort_by_key(|(_, created_at)| created_at);
 
         // delete files, so that (n-1) files remain, because we will create another log file
-        for file in &files[..files.len() - (max_files - 1)] {
-            if let Err(error) = fs::remove_file(file.0.path()) {
+        for (file, _) in &files[..files.len() - (max_files - 1)] {
+            if let Err(error) = fs::remove_file(file.path()) {
                 eprintln!(
                     "Failed to remove old log file {}: {}",
-                    file.0.path().display(),
+                    file.path().display(),
                     error
                 );
             }

From 9bf0e2411efc8d273f506b443cb68c358854f5e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 21:19:24 +0300
Subject: [PATCH 20/28] Update tracing-appender/src/rolling/builder.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 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 b4dadab7fc..75d6f2a337 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -187,7 +187,7 @@ impl Builder {
     ///
     /// When a new log file is created, if there are `n` or more
     /// existing log files in the directory, the oldest will be deleted.
-    /// If no value is supplied, `RollingAppender` will not remove any files.
+    /// If no value is supplied, the `RollingAppender` will not remove any files.
     ///
     /// # Examples
     ///

From 86de637f5c196e0c76fdfb36b35751ab9b173f52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCn=20=C3=96zerk?= <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 21:20:01 +0300
Subject: [PATCH 21/28] Update tracing-appender/src/rolling/builder.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-appender/src/rolling/builder.rs | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index 75d6f2a337..3607423736 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -188,7 +188,21 @@ impl Builder {
     /// When a new log file is created, if there are `n` or more
     /// existing log files in the directory, the oldest will be deleted.
     /// If no value is supplied, the `RollingAppender` will not remove any files.
-    ///
+    /// 
+    /// Files are considered candidates for deletion based on the following
+    /// criteria:
+    ///
+    /// * The file must not be a directory or symbolic link.
+    /// * If the appender is configured with a [`filename_prefix`], the file
+    ///   name must start with that prefix.
+    /// * If the appender is configured with a [`filename_suffix`], the file
+    ///   name must end with that suffix.
+    /// * If the appender has neither a filename prefix nor a suffix, then the
+    ///   file name must parse as a valid date based on the appender's date
+    ///   format.
+    ///
+    /// Files matching these criteria may be deleted if the maximum number of
+    /// log files in the directory has been reached.
     /// # Examples
     ///
     /// ```

From 08c44200ae7d966439f673bffc69f549b3dd94d1 Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Fri, 23 Sep 2022 22:14:25 +0300
Subject: [PATCH 22/28] suggestions

---
 tracing-appender/src/rolling.rs         | 72 ++++++++++++++++++-------
 tracing-appender/src/rolling/builder.rs | 12 +++--
 2 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 277df0ca51..958ab0f17f 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -34,7 +34,7 @@ use std::{
     path::{Path, PathBuf},
     sync::atomic::{AtomicUsize, Ordering},
 };
-use time::{format_description, Duration, OffsetDateTime, Time};
+use time::{format_description, Date, Duration, OffsetDateTime, Time};
 
 mod builder;
 pub use builder::{Builder, InitError};
@@ -568,25 +568,57 @@ impl Inner {
     }
 
     fn prune_old_logs(&self, max_files: usize) {
+        let format;
+        match self.rotation {
+            Rotation::MINUTELY => {
+                format = format_description::parse("[year]-[month]-[day]-[hour]-[minute]")
+                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
+            }
+            Rotation::HOURLY => {
+                format = format_description::parse("[year]-[month]-[day]-[hour]")
+                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
+            }
+            Rotation::DAILY => {
+                format = format_description::parse("[year]-[month]-[day]")
+                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
+            }
+            Rotation::NEVER => {
+                unreachable!("maximum log files feature is not available for `Rotation::NEVER`!")
+            }
+        }
+
         let files = fs::read_dir(&self.log_directory).map(|dir| {
-            dir.filter_map(Result::ok)
-                .filter(|entry| {
-                    entry.file_name().to_string_lossy().contains(
-                        &self
-                            .log_filename_prefix
-                            .clone()
-                            .expect("prefix is always present"),
-                    )
-                })
-                .filter_map(|entry| {
-                    let created = entry
-                        .metadata()
-                        .and_then(|metadata| metadata.created())
-                        .ok()?;
-                    Some((entry, created))
-                })
-                .collect::<Vec<_>>()
+            dir.filter_map(|entry| {
+                let entry = entry.ok()?;
+
+                if let Some(prefix) = &self.log_filename_prefix {
+                    if !entry.file_name().to_string_lossy().contains(prefix) {
+                        return None;
+                    }
+                }
+
+                if let Some(suffix) = &self.log_filename_suffix {
+                    if !entry.file_name().to_string_lossy().contains(suffix) {
+                        return None;
+                    }
+                }
+
+                if self.log_filename_prefix.is_none()
+                    && self.log_filename_suffix.is_none()
+                    && Date::parse(entry.file_name().to_str().unwrap(), &format).is_err()
+                {
+                    return None;
+                }
+
+                let created = entry
+                    .metadata()
+                    .and_then(|metadata| metadata.created())
+                    .ok()?;
+                Some((entry, created))
+            })
+            .collect::<Vec<_>>()
         });
+
         let mut files = match files {
             Ok(files) => files,
             Err(error) => {
@@ -599,10 +631,10 @@ impl Inner {
         }
 
         // sort the files by their creation timestamps.
-        files.sort_by_key(|(_, created_at)| created_at);
+        files.sort_by_key(|(_, created_at)| *created_at);
 
         // delete files, so that (n-1) files remain, because we will create another log file
-        for (file, _) in &files[..files.len() - (max_files - 1)] {
+        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 {}: {}",
diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index 3607423736..5b32b133f3 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -39,10 +39,14 @@ impl Builder {
     /// | Parameter | Default Value | Notes |
     /// | :-------- | :------------ | :---- |
     /// | [`rotation`] | [`Rotation::NEVER`] | By default, log files will never be rotated. |
-    /// | [`filename_prefix`] | `""` | By default, log file names will not have a prefix. |
+    /// | [`prefix`] | `""` | By default, log file names will not have a prefix. |
+    /// | [`suffix`] | `""` | By default, log file names will not have a suffix. |
+    /// | [`max_files`] | `None` | By default, there is no limit for maximum log file count. |
     ///
     /// [`rotation`]: Self::rotation
-    /// [`filename_prefix`]: Self::filename_prefix
+    /// [`prefix`]: Self::filename_prefix
+    /// [`suffix`]: Self::filename_suffix
+    /// [`max_files`]: Self::max_log_files
     #[must_use]
     pub const fn new() -> Self {
         Self {
@@ -183,12 +187,12 @@ impl Builder {
         Self { suffix, ..self }
     }
 
-    /// Keep the last `n` log files on disk.
+    /// Keeps the last `n` log files on disk.
     ///
     /// When a new log file is created, if there are `n` or more
     /// existing log files in the directory, the oldest will be deleted.
     /// If no value is supplied, the `RollingAppender` will not remove any files.
-    /// 
+    ///
     /// Files are considered candidates for deletion based on the following
     /// criteria:
     ///

From 24f26bc4210d4957189edd501e95ed04d4836ac5 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Fri, 23 Sep 2022 12:57:15 -0700
Subject: [PATCH 23/28] only parse format descr when creating  appender

---
 tracing-appender/src/rolling.rs | 236 +++++++++++++++++++-------------
 1 file changed, 139 insertions(+), 97 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 958ab0f17f..288f22f3e4 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -104,6 +104,7 @@ struct Inner {
     log_directory: PathBuf,
     log_filename_prefix: Option<String>,
     log_filename_suffix: Option<String>,
+    date_format: Vec<format_description::FormatItem<'static>>,
     rotation: Rotation,
     next_date: AtomicUsize,
     max_files: Option<usize>,
@@ -491,32 +492,14 @@ impl Rotation {
         }
     }
 
-    pub(crate) fn join_date(
-        &self,
-        filename: Option<&str>,
-        date: &OffsetDateTime,
-        suffix: Option<&str>,
-    ) -> String {
-        let format = match *self {
+    fn date_format(&self) -> Vec<format_description::FormatItem<'static>> {
+        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]"),
         }
-        .expect("Unable to create a formatter; this is a bug in tracing-appender");
-        let date = date
-            .format(&format)
-            .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender");
-
-        match (self, 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,
-        }
+        .expect("Unable to create a formatter; this is a bug in tracing-appender")
     }
 }
 
@@ -544,18 +527,13 @@ impl Inner {
         max_files: Option<usize>,
     ) -> Result<(Self, RwLock<File>), builder::InitError> {
         let log_directory = directory.as_ref().to_path_buf();
-        let filename = rotation.join_date(
-            log_filename_prefix.as_deref(),
-            &now,
-            log_filename_suffix.as_deref(),
-        );
+        let date_format = rotation.date_format();
         let next_date = rotation.next_date(&now);
-        let writer = RwLock::new(create_writer(log_directory.as_ref(), &filename)?);
-
         let inner = Inner {
             log_directory,
             log_filename_prefix,
             log_filename_suffix,
+            date_format,
             next_date: AtomicUsize::new(
                 next_date
                     .map(|date| date.unix_timestamp() as usize)
@@ -564,29 +542,33 @@ impl Inner {
             rotation,
             max_files,
         };
+        let filename = inner.join_date(&now);
+        let writer = RwLock::new(create_writer(inner.log_directory.as_ref(), &filename)?);
+
         Ok((inner, writer))
     }
 
-    fn prune_old_logs(&self, max_files: usize) {
-        let format;
-        match self.rotation {
-            Rotation::MINUTELY => {
-                format = format_description::parse("[year]-[month]-[day]-[hour]-[minute]")
-                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
-            }
-            Rotation::HOURLY => {
-                format = format_description::parse("[year]-[month]-[day]-[hour]")
-                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
-            }
-            Rotation::DAILY => {
-                format = format_description::parse("[year]-[month]-[day]")
-                    .expect("Unable to create a formatter; this is a bug in tracing-appender");
-            }
-            Rotation::NEVER => {
-                unreachable!("maximum log files feature is not available for `Rotation::NEVER`!")
-            }
+    pub(crate) fn join_date(&self, date: &OffsetDateTime) -> String {
+        let date = date
+            .format(&self.date_format)
+            .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender");
+
+        match (
+            &self.rotation,
+            &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,
         }
+    }
 
+    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()?;
@@ -605,7 +587,7 @@ impl Inner {
 
                 if self.log_filename_prefix.is_none()
                     && self.log_filename_suffix.is_none()
-                    && Date::parse(entry.file_name().to_str().unwrap(), &format).is_err()
+                    && Date::parse(entry.file_name().to_str().unwrap(), &self.date_format).is_err()
                 {
                     return None;
                 }
@@ -646,11 +628,7 @@ impl Inner {
     }
 
     fn refresh_writer(&self, now: OffsetDateTime, file: &mut File) {
-        let filename = self.rotation.join_date(
-            self.log_filename_prefix.as_deref(),
-            &now,
-            self.log_filename_suffix.as_deref(),
-        );
+        let filename = self.join_date(&now);
 
         if let Some(max_files) = self.max_files {
             self.prune_old_logs(max_files);
@@ -820,56 +798,120 @@ mod test {
          sign:mandatory]:[offset_minute]:[offset_second]",
         )
         .unwrap();
+        let directory = tempfile::tempdir().expect("failed to create tempdir");
 
         let now = OffsetDateTime::parse("2020-02-01 10:01:00 +00:00:00", &format).unwrap();
 
-        // per-minute
-        let path = Rotation::MINUTELY.join_date(Some("app.log"), &now, None);
-        assert_eq!("app.log.2020-02-01-10-01", path);
-
-        // per-hour
-        let path = Rotation::HOURLY.join_date(Some("app.log"), &now, None);
-        assert_eq!("app.log.2020-02-01-10", path);
-
-        // per-day
-        let path = Rotation::DAILY.join_date(Some("app.log"), &now, None);
-        assert_eq!("app.log.2020-02-01", path);
-
-        // never
-        let path = Rotation::NEVER.join_date(Some("app.log"), &now, None);
-        assert_eq!("app.log", path);
-
-        // per-minute with suffix
-        let path = Rotation::MINUTELY.join_date(Some("app"), &now, Some("log"));
-        assert_eq!("app.2020-02-01-10-01.log", path);
-
-        // per-hour with suffix
-        let path = Rotation::HOURLY.join_date(Some("app"), &now, Some("log"));
-        assert_eq!("app.2020-02-01-10.log", path);
-
-        // per-day with suffix
-        let path = Rotation::DAILY.join_date(Some("app"), &now, Some("log"));
-        assert_eq!("app.2020-02-01.log", path);
-
-        // never with suffix
-        let path = Rotation::NEVER.join_date(Some("app"), &now, Some("log"));
-        assert_eq!("app.log", path);
-
-        // per-minute without prefix
-        let path = Rotation::MINUTELY.join_date(None, &now, Some("app.log"));
-        assert_eq!("2020-02-01-10-01.app.log", path);
-
-        // per-hour without prefix
-        let path = Rotation::HOURLY.join_date(None, &now, Some("app.log"));
-        assert_eq!("2020-02-01-10.app.log", path);
+        struct TestCase {
+            expected: &'static str,
+            rotation: Rotation,
+            prefix: Option<&'static str>,
+            suffix: Option<&'static str>,
+        }
 
-        // per-day without prefix
-        let path = Rotation::DAILY.join_date(None, &now, Some("app.log"));
-        assert_eq!("2020-02-01.app.log", path);
+        let test = |TestCase {
+                        expected,
+                        rotation,
+                        prefix,
+                        suffix,
+                    }| {
+            let (inner, _) = Inner::new(
+                now,
+                rotation.clone(),
+                directory.path(),
+                prefix.map(ToString::to_string),
+                suffix.map(ToString::to_string),
+                None,
+            )
+            .unwrap();
+            let path = inner.join_date(&now);
+            assert_eq!(
+                expected, path,
+                "rotation = {:?}, prefix = {:?}, suffix = {:?}",
+                rotation, prefix, suffix
+            );
+        };
 
-        // never without prefix
-        let path = Rotation::NEVER.join_date(None, &now, Some("app.log"));
-        assert_eq!("app.log", path);
+        let test_cases = vec![
+            // prefix only
+            TestCase {
+                expected: "app.log.2020-02-01-10-01",
+                rotation: Rotation::MINUTELY,
+                prefix: Some("app.log"),
+                suffix: None,
+            },
+            TestCase {
+                expected: "app.log.2020-02-01-10",
+                rotation: Rotation::HOURLY,
+                prefix: Some("app.log"),
+                suffix: None,
+            },
+            TestCase {
+                expected: "app.log.2020-02-01",
+                rotation: Rotation::DAILY,
+                prefix: Some("app.log"),
+                suffix: None,
+            },
+            TestCase {
+                expected: "app.log",
+                rotation: Rotation::NEVER,
+                prefix: Some("app.log"),
+                suffix: None,
+            },
+            // prefix and suffix
+            TestCase {
+                expected: "app.2020-02-01-10-01.log",
+                rotation: Rotation::MINUTELY,
+                prefix: Some("app"),
+                suffix: Some("log"),
+            },
+            TestCase {
+                expected: "app.2020-02-01-10.log",
+                rotation: Rotation::HOURLY,
+                prefix: Some("app"),
+                suffix: Some("log"),
+            },
+            TestCase {
+                expected: "app.2020-02-01.log",
+                rotation: Rotation::DAILY,
+                prefix: Some("app"),
+                suffix: Some("log"),
+            },
+            TestCase {
+                expected: "app.log",
+                rotation: Rotation::NEVER,
+                prefix: Some("app"),
+                suffix: Some("log"),
+            },
+            // suffix only
+            TestCase {
+                expected: "2020-02-01-10-01.log",
+                rotation: Rotation::MINUTELY,
+                prefix: None,
+                suffix: Some("log"),
+            },
+            TestCase {
+                expected: "2020-02-01-10.log",
+                rotation: Rotation::HOURLY,
+                prefix: None,
+                suffix: Some("log"),
+            },
+            TestCase {
+                expected: "2020-02-01.log",
+                rotation: Rotation::DAILY,
+                prefix: None,
+                suffix: Some("log"),
+            },
+            TestCase {
+                expected: "log",
+                rotation: Rotation::NEVER,
+                prefix: None,
+                suffix: Some("log"),
+            },
+        ];
+        for test_case in test_cases {
+            test(test_case)
+        }
     }
 
     #[test]

From b224b1e1bd036f553a1df91bcbb497da5994a592 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Fri, 23 Sep 2022 13:20:46 -0700
Subject: [PATCH 24/28] reduce test flakiness due to low resolution timestamps

---
 tracing-appender/src/rolling.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 288f22f3e4..65caba1ea4 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -1044,6 +1044,11 @@ mod test {
         // advance time by one hour
         (*clock.lock().unwrap()) += Duration::hours(1);
 
+        // depending on the filesystem, the creation timestamp's resolution may
+        // be as coarse as one second, so we need to wait a bit here to ensure
+        // that the next file actually is newer than the old one.
+        std::thread::sleep(std::time::Duration::from_secs(1));
+
         tracing::info!("file 2");
 
         // advance time by one second
@@ -1054,6 +1059,9 @@ mod test {
         // advance time by one hour
         (*clock.lock().unwrap()) += Duration::hours(1);
 
+        // again, sleep to ensure that the creation timestamps actually differ.
+        std::thread::sleep(std::time::Duration::from_secs(1));
+
         tracing::info!("file 3");
 
         // advance time by one second

From 1fd04ff43fef49e3e14cee918ec55b464d87f12b Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Fri, 23 Sep 2022 13:22:51 -0700
Subject: [PATCH 25/28] only turn filename into a string once

also, use `starts_with` and `ends_with` for prefix/suffix matching,
rather than `contains`.
---
 tracing-appender/src/rolling.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 65caba1ea4..3732016ad6 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -573,21 +573,24 @@ impl Inner {
             dir.filter_map(|entry| {
                 let entry = entry.ok()?;
 
+                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 !entry.file_name().to_string_lossy().contains(prefix) {
+                    if !dbg!(filename.starts_with(prefix)) {
                         return None;
                     }
                 }
 
                 if let Some(suffix) = &self.log_filename_suffix {
-                    if !entry.file_name().to_string_lossy().contains(suffix) {
+                    if !filename.ends_with(suffix) {
                         return None;
                     }
                 }
 
                 if self.log_filename_prefix.is_none()
                     && self.log_filename_suffix.is_none()
-                    && Date::parse(entry.file_name().to_str().unwrap(), &self.date_format).is_err()
+                    && Date::parse(filename, &self.date_format).is_err()
                 {
                     return None;
                 }

From 19e921f902c09c4a1d592718b4ab26a55a0b12b7 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Fri, 23 Sep 2022 13:24:32 -0700
Subject: [PATCH 26/28] never delete dirs or symlinks

---
 tracing-appender/src/rolling.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 3732016ad6..5ed72f489e 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -572,6 +572,13 @@ impl Inner {
         let files = fs::read_dir(&self.log_directory).map(|dir| {
             dir.filter_map(|entry| {
                 let entry = entry.ok()?;
+                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;
+                }
 
                 let filename = entry.file_name();
                 // if the filename is not a UTF-8 string, skip it.
@@ -595,10 +602,7 @@ impl Inner {
                     return None;
                 }
 
-                let created = entry
-                    .metadata()
-                    .and_then(|metadata| metadata.created())
-                    .ok()?;
+                let created = metadata.created().ok()?;
                 Some((entry, created))
             })
             .collect::<Vec<_>>()

From 5b3e24aa1141650603b7ff726ee5277a93381a3e Mon Sep 17 00:00:00 2001
From: Ozgun Ozerk <ozgun@subspace.network>
Date: Sat, 24 Sep 2022 11:50:04 +0300
Subject: [PATCH 27/28] documentation fixes

---
 tracing-appender/src/rolling/builder.rs | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs
index 5b32b133f3..8bb237ba49 100644
--- a/tracing-appender/src/rolling/builder.rs
+++ b/tracing-appender/src/rolling/builder.rs
@@ -44,9 +44,9 @@ impl Builder {
     /// | [`max_files`] | `None` | By default, there is no limit for maximum log file count. |
     ///
     /// [`rotation`]: Self::rotation
-    /// [`prefix`]: Self::filename_prefix
-    /// [`suffix`]: Self::filename_suffix
-    /// [`max_files`]: Self::max_log_files
+    /// [`filename_prefix`]: Self::filename_prefix
+    /// [`filename_suffix`]: Self::filename_suffix
+    /// [`max_log_files`]: Self::max_log_files
     #[must_use]
     pub const fn new() -> Self {
         Self {
@@ -207,6 +207,10 @@ impl Builder {
     ///
     /// Files matching these criteria may be deleted if the maximum number of
     /// log files in the directory has been reached.
+    ///
+    /// [`filename_prefix`]: Self::filename_prefix
+    /// [`filename_suffix`]: Self::filename_suffix
+    ///
     /// # Examples
     ///
     /// ```

From 9032b4402211e58d880ee2d6717512e25cf1ce9f Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Sat, 24 Sep 2022 11:52:45 -0700
Subject: [PATCH 28/28] remove `dbg!`

---
 tracing-appender/src/rolling.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs
index 5ed72f489e..434d23eeb6 100644
--- a/tracing-appender/src/rolling.rs
+++ b/tracing-appender/src/rolling.rs
@@ -584,7 +584,7 @@ impl Inner {
                 // 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 !dbg!(filename.starts_with(prefix)) {
+                    if !filename.starts_with(prefix) {
                         return None;
                     }
                 }