refactor(obs): enhance log cleanup and rotation (#2040)

crates/obs/src/cleaner/compress.rs

@@ -20,6 +20,7 @@
 
 use flate2::Compression;
 use flate2::write::GzEncoder;
+use rustfs_config::observability::DEFAULT_OBS_LOG_GZIP_COMPRESSION_EXTENSION;
 use std::fs::File;
 use std::io::{BufReader, BufWriter, Write};
 use std::path::Path;
@@ -39,7 +40,7 @@ use tracing::{debug, info};
 /// Propagates any I/O error encountered while opening, reading, writing, or
 /// flushing files.
 pub(super) fn compress_file(path: &Path, level: u32, dry_run: bool) -> Result<(), std::io::Error> {
-    let gz_path = path.with_extension("gz");
+    let gz_path = path.with_extension(DEFAULT_OBS_LOG_GZIP_COMPRESSION_EXTENSION);
 
     if gz_path.exists() {
         debug!("Compressed file already exists, skipping: {:?}", gz_path);

crates/obs/src/cleaner/core.rs

@@ -47,7 +47,7 @@ pub struct LogCleaner {
     pub(super) match_mode: FileMatchMode,
     /// The cleaner will never delete files if doing so would leave fewer than
     /// this many files in the directory.
-    pub(super) keep_count: usize,
+    pub(super) keep_files: usize,
     /// Hard ceiling on the total bytes of all managed files; `0` = no limit.
     pub(super) max_total_size_bytes: u64,
     /// Hard ceiling on a single file's size; `0` = no per-file limit.
@@ -81,7 +81,7 @@ impl LogCleaner {
         log_dir: PathBuf,
         file_pattern: String,
         match_mode: FileMatchMode,
-        keep_count: usize,
+        keep_files: usize,
         max_total_size_bytes: u64,
         max_single_file_size_bytes: u64,
         compress_old_files: bool,
@@ -101,7 +101,7 @@ impl LogCleaner {
             log_dir,
             file_pattern,
             match_mode,
-            keep_count,
+            keep_files,
             max_total_size_bytes,
             max_single_file_size_bytes,
             compress_old_files,
@@ -207,7 +207,7 @@ impl LogCleaner {
     /// Choose which files from `files` (sorted oldest-first) should be deleted or rotated.
     ///
     /// The algorithm respects three constraints in order:
-    /// 1. Always keep at least `keep_count` files.
+    /// 1. Always keep at least `keep_files` files.
     /// 2. Delete old files while the total size exceeds `max_total_size_bytes`.
     /// 3. Delete any file whose individual size exceeds `max_single_file_size_bytes`.
     ///
@@ -226,27 +226,20 @@ impl LogCleaner {
         // We will protect this file from size-based deletion.
         let active_file_idx = files.len() - 1;
 
-        // The number of files we are allowed to delete.
-        // Any file with index >= max_deletable_count is protected by keep_count.
-        let max_deletable_count = files.len().saturating_sub(self.keep_count);
+        // Calculate how many files we *must* delete to satisfy keep_files.
+        let must_delete_count = files.len().saturating_sub(self.keep_files);
 
         let mut current_size = total_size;
 
         for (idx, file) in files.iter().enumerate() {
-            // If we are in the protected range, we stop deleting.
-            if idx >= max_deletable_count {
-                // However, if the active file is too large, we might rotate it.
-                if idx == active_file_idx {
-                    let over_single = self.max_single_file_size_bytes > 0 && file.size > self.max_single_file_size_bytes;
-                    if over_single {
-                        to_rotate = Some(file.clone());
-                    }
-                }
+            // Condition 1: Enforce keep_files.
+            // If we are in the range of files that exceed the count limit, delete them.
+            if idx < must_delete_count {
+                current_size = current_size.saturating_sub(file.size);
+                to_delete.push(file.clone());
                 continue;
             }
 
-            // We are in the deletable range. Check if we *should* delete.
-
             // Condition 2: Enforce max_total_size_bytes.
             let over_total = self.max_total_size_bytes > 0 && current_size > self.max_total_size_bytes;
 

crates/obs/src/cleaner/mod.rs

@@ -37,7 +37,7 @@
 //!     PathBuf::from("/var/log/rustfs"),
 //!     "rustfs.log.".to_string(),
 //!     FileMatchMode::Prefix,
-//!     10,              // keep_count
+//!     10,              // keep_files
 //!     2 * 1024 * 1024 * 1024, // max_total_size_bytes (2 GiB)
 //!     0,               // max_single_file_size_bytes (unlimited)
 //!     true,            // compress_old_files
@@ -106,7 +106,7 @@ mod tests {
         create_log_file(&dir, "app.log.2024-01-03", 1024)?;
         create_log_file(&dir, "other.log", 1024)?; // not managed
 
-        // Total managed = 3 072 bytes; limit = 2 048; keep_count = 2 → must delete 1.
+        // Total managed = 3 072 bytes; limit = 2 048; keep_files = 2 → must delete 1.
         let cleaner = make_cleaner(dir.clone(), 2, 2048);
         let (deleted, freed) = cleaner.cleanup()?;
 
@@ -116,7 +116,7 @@ mod tests {
     }
 
     #[test]
-    fn test_cleanup_respects_keep_count() -> std::io::Result<()> {
+    fn test_cleanup_respects_keep_files() -> std::io::Result<()> {
         let tmp = TempDir::new()?;
         let dir = tmp.path().to_path_buf();
 
@@ -124,10 +124,11 @@ mod tests {
             create_log_file(&dir, &format!("app.log.2024-01-0{i}"), 1024)?;
         }
 
-        // No size limit, keep_count = 3 → nothing to delete (5 > 3 but size == 0 limit).
         let cleaner = make_cleaner(dir.clone(), 3, 0);
         let (deleted, _) = cleaner.cleanup()?;
-        assert_eq!(deleted, 0, "keep_count prevents deletion when no size limit");
+
+        // Updated expectation: keep_files acts as a limit (ceiling), so excess files are deleted.
+        assert_eq!(deleted, 2, "keep_files should enforce a maximum file count");
         Ok(())
     }
 

crates/obs/src/cleaner/scanner.rs

@@ -18,6 +18,7 @@
 //! compress any files — it only reports what it found.
 
 use super::types::{FileInfo, FileMatchMode};
+use rustfs_config::observability::DEFAULT_OBS_LOG_GZIP_COMPRESSION_ALL_EXTENSION;
 use std::path::Path;
 use std::time::{Duration, SystemTime};
 use tracing::debug;
@@ -81,7 +82,7 @@ pub(super) fn collect_log_files(
         }
 
         // Compressed files are handled by collect_compressed_files.
-        if filename.ends_with(".gz") {
+        if filename.ends_with(DEFAULT_OBS_LOG_GZIP_COMPRESSION_ALL_EXTENSION) {
             continue;
         }
 
@@ -180,7 +181,7 @@ pub(super) fn collect_expired_compressed_files(
             None => continue,
         };
 
-        if !filename.ends_with(".gz") {
+        if !filename.ends_with(DEFAULT_OBS_LOG_GZIP_COMPRESSION_ALL_EXTENSION) {
             continue;
         }
 

crates/obs/src/cleaner/types.rs

@@ -14,6 +14,7 @@
 
 //! Shared types used across the log-cleanup sub-modules.
 
+use std::fmt;
 use std::path::PathBuf;
 use std::time::SystemTime;
 
@@ -21,11 +22,29 @@ use std::time::SystemTime;
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum FileMatchMode {
     /// The filename must start with the pattern (e.g. "app.log." matches "app.log.2024-01-01").
+    /// Corresponds to config value "prefix".
     Prefix,
     /// The filename must end with the pattern (e.g. ".log" matches "2024-01-01.log").
+    /// Corresponds to config value "suffix".
     Suffix,
 }
 
+impl FileMatchMode {
+    /// Returns the string representation of the match mode.
+    pub fn as_str(&self) -> &'static str {
+        match self {
+            FileMatchMode::Prefix => "prefix",
+            FileMatchMode::Suffix => "suffix",
+        }
+    }
+}
+
+impl fmt::Display for FileMatchMode {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self.as_str())
+    }
+}
+
 /// Metadata for a single log file discovered by the scanner.
 ///
 /// Carries enough information to make cleanup decisions (sort by age, compare

crates/obs/src/config.rs

@@ -25,16 +25,16 @@
 use rustfs_config::observability::{
     DEFAULT_OBS_ENVIRONMENT_PRODUCTION, DEFAULT_OBS_LOG_CLEANUP_INTERVAL_SECONDS, DEFAULT_OBS_LOG_COMPRESS_OLD_FILES,
     DEFAULT_OBS_LOG_COMPRESSED_FILE_RETENTION_DAYS, DEFAULT_OBS_LOG_DELETE_EMPTY_FILES, DEFAULT_OBS_LOG_DRY_RUN,
-    DEFAULT_OBS_LOG_GZIP_COMPRESSION_LEVEL, DEFAULT_OBS_LOG_KEEP_COUNT, DEFAULT_OBS_LOG_MATCH_MODE,
-    DEFAULT_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES, DEFAULT_OBS_LOG_MAX_TOTAL_SIZE_BYTES, DEFAULT_OBS_LOG_MIN_FILE_AGE_SECONDS,
-    ENV_OBS_ENDPOINT, ENV_OBS_ENVIRONMENT, ENV_OBS_LOG_CLEANUP_INTERVAL_SECONDS, ENV_OBS_LOG_COMPRESS_OLD_FILES,
-    ENV_OBS_LOG_COMPRESSED_FILE_RETENTION_DAYS, ENV_OBS_LOG_DELETE_EMPTY_FILES, ENV_OBS_LOG_DIRECTORY, ENV_OBS_LOG_DRY_RUN,
-    ENV_OBS_LOG_ENDPOINT, ENV_OBS_LOG_EXCLUDE_PATTERNS, ENV_OBS_LOG_FILENAME, ENV_OBS_LOG_GZIP_COMPRESSION_LEVEL,
-    ENV_OBS_LOG_KEEP_COUNT, ENV_OBS_LOG_KEEP_FILES, ENV_OBS_LOG_MATCH_MODE, ENV_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES,
-    ENV_OBS_LOG_MAX_TOTAL_SIZE_BYTES, ENV_OBS_LOG_MIN_FILE_AGE_SECONDS, ENV_OBS_LOG_ROTATION_TIME, ENV_OBS_LOG_STDOUT_ENABLED,
-    ENV_OBS_LOGGER_LEVEL, ENV_OBS_LOGS_EXPORT_ENABLED, ENV_OBS_METER_INTERVAL, ENV_OBS_METRIC_ENDPOINT,
-    ENV_OBS_METRICS_EXPORT_ENABLED, ENV_OBS_SAMPLE_RATIO, ENV_OBS_SERVICE_NAME, ENV_OBS_SERVICE_VERSION, ENV_OBS_TRACE_ENDPOINT,
-    ENV_OBS_TRACES_EXPORT_ENABLED, ENV_OBS_USE_STDOUT,
+    DEFAULT_OBS_LOG_GZIP_COMPRESSION_LEVEL, DEFAULT_OBS_LOG_MATCH_MODE, DEFAULT_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES,
+    DEFAULT_OBS_LOG_MAX_TOTAL_SIZE_BYTES, DEFAULT_OBS_LOG_MIN_FILE_AGE_SECONDS, ENV_OBS_ENDPOINT, ENV_OBS_ENVIRONMENT,
+    ENV_OBS_LOG_CLEANUP_INTERVAL_SECONDS, ENV_OBS_LOG_COMPRESS_OLD_FILES, ENV_OBS_LOG_COMPRESSED_FILE_RETENTION_DAYS,
+    ENV_OBS_LOG_DELETE_EMPTY_FILES, ENV_OBS_LOG_DIRECTORY, ENV_OBS_LOG_DRY_RUN, ENV_OBS_LOG_ENDPOINT,
+    ENV_OBS_LOG_EXCLUDE_PATTERNS, ENV_OBS_LOG_FILENAME, ENV_OBS_LOG_GZIP_COMPRESSION_LEVEL, ENV_OBS_LOG_KEEP_FILES,
+    ENV_OBS_LOG_MATCH_MODE, ENV_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES, ENV_OBS_LOG_MAX_TOTAL_SIZE_BYTES,
+    ENV_OBS_LOG_MIN_FILE_AGE_SECONDS, ENV_OBS_LOG_ROTATION_TIME, ENV_OBS_LOG_STDOUT_ENABLED, ENV_OBS_LOGGER_LEVEL,
+    ENV_OBS_LOGS_EXPORT_ENABLED, ENV_OBS_METER_INTERVAL, ENV_OBS_METRIC_ENDPOINT, ENV_OBS_METRICS_EXPORT_ENABLED,
+    ENV_OBS_SAMPLE_RATIO, ENV_OBS_SERVICE_NAME, ENV_OBS_SERVICE_VERSION, ENV_OBS_TRACE_ENDPOINT, ENV_OBS_TRACES_EXPORT_ENABLED,
+    ENV_OBS_USE_STDOUT,
 };
 use rustfs_config::{
     APP_NAME, DEFAULT_LOG_KEEP_FILES, DEFAULT_LOG_LEVEL, DEFAULT_LOG_ROTATION_TIME, DEFAULT_OBS_LOG_FILENAME,
@@ -65,8 +65,8 @@ use std::env;
 ///
 /// - All fields are `Option<T>` to allow partial configuration via environment
 ///   variables with sensible defaults provided by constants in `rustfs-config`.
-/// - `log_keep_count` represents the cleaner's minimum retention; `log_keep_files`
-///   controls the rolling-appender's file limit (both typically set to the same value).
+/// - `log_keep_files` is used to derive the rolling-appender's upper bound on
+///   retained files (if enabled) and to set the cleaner's minimum retention count.
 ///
 /// # Example
 /// ```no_run
@@ -128,13 +128,11 @@ pub struct OtelConfig {
     /// Rotation time granularity: `"hourly"` or `"daily"` (default: `"daily"`).
     pub log_rotation_time: Option<String>,
     /// Number of rolling log files to retain (default: `30`).
-    /// The rolling-appender will delete the oldest file when this limit is exceeded.
+    /// Used by both the rolling-appender (as a loose upper bound) and the
+    /// background cleaner (as the minimum retention count).
     pub log_keep_files: Option<usize>,
 
     // ── Log cleanup ───────────────────────────────────────────────────────────
-    /// Minimum number of files the cleaner must always preserve.
-    /// Typically set to the same value as `log_keep_files`.
-    pub log_keep_count: Option<usize>,
     /// Hard ceiling on the total size (bytes) of all log files (default: 2 GiB).
     pub log_max_total_size_bytes: Option<u64>,
     /// Per-file size ceiling (bytes); `0` means unlimited (default: `0`).
@@ -205,12 +203,9 @@ impl OtelConfig {
             _ => None,
         };
 
-        // `log_keep_files` (legacy) and `log_keep_count` (new) share the same
-        // environment variables but have slightly different semantics.
-        // `log_keep_files` is the rolling-appender retention count; `log_keep_count`
-        // is the cleaner's minimum-keep threshold.  Both default to the same value.
+        // `log_keep_files` is the single source of truth for file retention count.
+        // It defaults to `DEFAULT_LOG_KEEP_FILES` (30).
         let log_keep_files = Some(get_env_usize(ENV_OBS_LOG_KEEP_FILES, DEFAULT_LOG_KEEP_FILES));
-        let log_keep_count = Some(get_env_usize(ENV_OBS_LOG_KEEP_COUNT, DEFAULT_OBS_LOG_KEEP_COUNT));
 
         // `log_rotation_time` drives the rolling-appender rotation period.
         let log_rotation_time = Some(get_env_str(ENV_OBS_LOG_ROTATION_TIME, DEFAULT_LOG_ROTATION_TIME));
@@ -238,7 +233,6 @@ impl OtelConfig {
             log_rotation_time,
             log_keep_files,
             // Log cleanup
-            log_keep_count,
             log_max_total_size_bytes: Some(get_env_u64(ENV_OBS_LOG_MAX_TOTAL_SIZE_BYTES, DEFAULT_OBS_LOG_MAX_TOTAL_SIZE_BYTES)),
             log_max_single_file_size_bytes: Some(get_env_u64(
                 ENV_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES,

crates/obs/src/telemetry/guard.rs

@@ -23,6 +23,7 @@
 //! 4. Cleanup task — aborted to prevent lingering background work.
 //! 5. Tracing worker guard — flushes buffered log lines written by
 //!    `tracing_appender`.
+//! 6. Stdout worker guard — flushes buffered log lines written to stdout.
 
 use opentelemetry_sdk::{logs::SdkLoggerProvider, metrics::SdkMeterProvider, trace::SdkTracerProvider};
 
@@ -39,13 +40,13 @@ pub struct OtelGuard {
     pub(crate) meter_provider: Option<SdkMeterProvider>,
     /// Optional logger provider for OTLP log export.
     pub(crate) logger_provider: Option<SdkLoggerProvider>,
+    /// Handle to the background log-cleanup task; aborted on drop.
+    pub(crate) cleanup_handle: Option<tokio::task::JoinHandle<()>>,
     /// Worker guard that keeps the non-blocking `tracing_appender` thread
     /// alive.  Dropping it blocks until all buffered records are flushed.
     pub(crate) tracing_guard: Option<tracing_appender::non_blocking::WorkerGuard>,
     /// Optional guard for stdout logging; kept separate to allow independent flushing and shutdown.
     pub(crate) stdout_guard: Option<tracing_appender::non_blocking::WorkerGuard>,
-    /// Handle to the background log-cleanup task; aborted on drop.
-    pub(crate) cleanup_handle: Option<tokio::task::JoinHandle<()>>,
 }
 
 impl std::fmt::Debug for OtelGuard {
@@ -54,9 +55,9 @@ impl std::fmt::Debug for OtelGuard {
             .field("tracer_provider", &self.tracer_provider.is_some())
             .field("meter_provider", &self.meter_provider.is_some())
             .field("logger_provider", &self.logger_provider.is_some())
+            .field("cleanup_handle", &self.cleanup_handle.is_some())
             .field("tracing_guard", &self.tracing_guard.is_some())
             .field("stdout_guard", &self.stdout_guard.is_some())
-            .field("cleanup_handle", &self.cleanup_handle.is_some())
             .finish()
     }
 }

crates/obs/src/telemetry/local.rs

@@ -195,7 +195,7 @@ fn init_file_logging_internal(
 
         let mut builder = RollingFileAppender::builder()
             .rotation(rotation)
-            .max_log_files(keep_files * 2); // Make sure there are some data files to archive to avoid premature deletion
+            .max_log_files(keep_files * 3); // Make sure there are some data files to archive to avoid premature deletion
 
         match match_mode {
             FileMatchMode::Prefix => builder = builder.filename_prefix(log_filename),
@@ -355,10 +355,9 @@ fn spawn_cleanup_task(
         _ => FileMatchMode::Suffix,
     };
 
-    let keep_count = config.log_keep_count.unwrap_or(keep_files);
     let max_total_size = config
         .log_max_total_size_bytes
-        .unwrap_or(DEFAULT_OBS_LOG_MAX_TOTAL_SIZE_BYTES * keep_count as u64);
+        .unwrap_or(DEFAULT_OBS_LOG_MAX_TOTAL_SIZE_BYTES);
     let max_single_file_size = config
         .log_max_single_file_size_bytes
         .unwrap_or(DEFAULT_OBS_LOG_MAX_SINGLE_FILE_SIZE_BYTES);
@@ -387,7 +386,7 @@ fn spawn_cleanup_task(
         log_dir,
         file_pattern,
         match_mode,
-        keep_count,
+        keep_files,
         max_total_size,
         max_single_file_size,
         compress,