feat: log rotation (#5561)

crates/goose-cli/src/logging.rs

@@ -1,5 +1,4 @@
 use anyhow::{Context, Result};
-use std::path::PathBuf;
 use std::sync::Arc;
 use std::sync::Once;
 use tokio::sync::Mutex;
@@ -16,12 +15,6 @@ use goose_bench::error_capture::ErrorCaptureLayer;
 // Used to ensure we only set up tracing once
 static INIT: Once = Once::new();
 
-/// Returns the directory where log files should be stored.
-/// Creates the directory structure if it doesn't exist.
-fn get_log_directory() -> Result<PathBuf> {
-    goose::logging::get_log_directory("cli", true)
-}
-
 /// Sets up the logging infrastructure for the application.
 /// This includes:
 /// - File-based logging with JSON formatting (DEBUG level)
@@ -50,20 +43,15 @@ fn setup_logging_internal(
 
     let mut setup = || {
         result = (|| {
-            // Set up file appender for goose module logs
+            let log_dir = goose::logging::prepare_log_directory("cli", true)?;
-            let log_dir = get_log_directory()?;
             let timestamp = chrono::Local::now().format("%Y%m%d_%H%M%S").to_string();
-
-            // Create log file name by prefixing with timestamp
             let log_filename = if name.is_some() {
                 format!("{}-{}.log", timestamp, name.unwrap())
             } else {
                 format!("{}.log", timestamp)
             };
-
-            // Create non-rolling file appender for detailed logs
             let file_appender = tracing_appender::rolling::RollingFileAppender::new(
-                Rotation::NEVER,
+                Rotation::NEVER, // we do manual rotation via file naming and cleanup_old_logs
                 log_dir,
                 log_filename,
             );
@@ -177,7 +165,7 @@ mod tests {
     #[test]
     fn test_log_directory_creation() {
         let _temp_dir = setup_temp_home();
-        let log_dir = get_log_directory().unwrap();
+        let log_dir = goose::logging::prepare_log_directory("cli", true).unwrap();
         assert!(log_dir.exists());
         assert!(log_dir.is_dir());
 

crates/goose-server/src/logging.rs

@@ -1,5 +1,4 @@
 use anyhow::{Context, Result};
-use std::path::PathBuf;
 use tracing_appender::rolling::Rotation;
 use tracing_subscriber::{
     filter::LevelFilter, fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, Layer,
@@ -8,32 +7,24 @@ use tracing_subscriber::{
 
 use goose::tracing::{langfuse_layer, otlp_layer};
 
-/// Returns the directory where log files should be stored.
-/// Creates the directory structure if it doesn't exist.
-fn get_log_directory() -> Result<PathBuf> {
-    goose::logging::get_log_directory("server", true)
-}
-
 /// Sets up the logging infrastructure for the application.
 /// This includes:
 /// - File-based logging with JSON formatting (DEBUG level)
 /// - Console output for development (INFO level)
 /// - Optional Langfuse integration (DEBUG level)
 pub fn setup_logging(name: Option<&str>) -> Result<()> {
-    // Set up file appender for goose module logs
+    let log_dir = goose::logging::prepare_log_directory("server", true)?;
-    let log_dir = get_log_directory()?;
     let timestamp = chrono::Local::now().format("%Y%m%d_%H%M%S").to_string();
-
-    // Create log file name by prefixing with timestamp
     let log_filename = if name.is_some() {
         format!("{}-{}.log", timestamp, name.unwrap())
     } else {
         format!("{}.log", timestamp)
     };
-
+    let file_appender = tracing_appender::rolling::RollingFileAppender::new(
-    // Create non-rolling file appender for detailed logs
+        Rotation::NEVER, // we do manual rotation via file naming and cleanup_old_logs
-    let file_appender =
+        log_dir,
-        tracing_appender::rolling::RollingFileAppender::new(Rotation::NEVER, log_dir, log_filename);
+        log_filename,
+    );
 
     // Create JSON file logging layer
     let file_layer = fmt::layer()

crates/goose/src/logging.rs

@@ -2,33 +2,59 @@ use crate::config::paths::Paths;
 use anyhow::{Context, Result};
 use std::fs;
 use std::path::PathBuf;
+use std::time::{Duration, SystemTime};
 
 /// Returns the directory where log files should be stored for a specific component.
 /// Creates the directory structure if it doesn't exist.
 ///
 /// # Arguments
 ///
-/// * `component` - The component name (e.g., "cli", "server", "debug")
+/// * `component` - The component name (e.g., "cli", "server", "debug", "llm")
 /// * `use_date_subdir` - Whether to create a date-based subdirectory
-pub fn get_log_directory(component: &str, use_date_subdir: bool) -> Result<PathBuf> {
+pub fn prepare_log_directory(component: &str, use_date_subdir: bool) -> Result<PathBuf> {
     let base_log_dir = Paths::in_state_dir("logs");
 
+    let _ = cleanup_old_logs(component);
+
     let component_dir = base_log_dir.join(component);
 
     let log_dir = if use_date_subdir {
-        // Create date-based subdirectory
+        component_dir.join(chrono::Local::now().format("%Y-%m-%d").to_string())
-        let now = chrono::Local::now();
-        component_dir.join(now.format("%Y-%m-%d").to_string())
     } else {
         component_dir
     };
 
-    // Ensure log directory exists
     fs::create_dir_all(&log_dir).context("Failed to create log directory")?;
 
     Ok(log_dir)
 }
 
+pub fn cleanup_old_logs(component: &str) -> Result<()> {
+    let base_log_dir = Paths::in_state_dir("logs");
+    let component_dir = base_log_dir.join(component);
+
+    if !component_dir.exists() {
+        return Ok(());
+    }
+
+    let two_weeks = SystemTime::now() - Duration::from_secs(14 * 24 * 60 * 60);
+    let entries = fs::read_dir(&component_dir)?;
+
+    for entry in entries.flatten() {
+        let path = entry.path();
+
+        if let Ok(metadata) = entry.metadata() {
+            if let Ok(modified) = metadata.modified() {
+                if modified < two_weeks && path.is_dir() {
+                    let _ = fs::remove_dir_all(&path);
+                }
+            }
+        }
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -37,7 +63,7 @@ mod tests {
     #[test]
     fn test_get_log_directory_basic_functionality() {
         // Test basic directory creation without date subdirectory
-        let result = get_log_directory("cli", false);
+        let result = prepare_log_directory("cli", false);
         assert!(result.is_ok());
 
         let log_dir = result.unwrap();
@@ -59,7 +85,7 @@ mod tests {
     #[test]
     fn test_get_log_directory_with_date_subdir() {
         // Test date-based subdirectory creation
-        let result = get_log_directory("server", true);
+        let result = prepare_log_directory("server", true);
         assert!(result.is_ok());
 
         let log_dir = result.unwrap();
@@ -90,11 +116,11 @@ mod tests {
         // Test that multiple calls return the same result and don't fail
         let component = "debug";
 
-        let result1 = get_log_directory(component, false);
+        let result1 = prepare_log_directory(component, false);
         assert!(result1.is_ok());
         let log_dir1 = result1.unwrap();
 
-        let result2 = get_log_directory(component, false);
+        let result2 = prepare_log_directory(component, false);
         assert!(result2.is_ok());
         let log_dir2 = result2.unwrap();
 
@@ -104,11 +130,11 @@ mod tests {
         assert!(log_dir2.exists());
 
         // Test same behavior with date subdirectories
-        let result3 = get_log_directory(component, true);
+        let result3 = prepare_log_directory(component, true);
         assert!(result3.is_ok());
         let log_dir3 = result3.unwrap();
 
-        let result4 = get_log_directory(component, true);
+        let result4 = prepare_log_directory(component, true);
         assert!(result4.is_ok());
         let log_dir4 = result4.unwrap();
 
@@ -123,7 +149,7 @@ mod tests {
         let mut created_dirs = Vec::new();
 
         for component in &components {
-            let result = get_log_directory(component, false);
+            let result = prepare_log_directory(component, false);
             assert!(result.is_ok(), "Failed for component: {}", component);
 
             let log_dir = result.unwrap();

crates/goose/src/providers/utils.rs

@@ -1,6 +1,5 @@
 use super::base::Usage;
 use super::errors::GoogleErrorCode;
-use crate::config::paths::Paths;
 use crate::model::ModelConfig;
 use crate::providers::errors::{OpenAIError, ProviderError};
 use anyhow::{anyhow, Result};
@@ -471,8 +470,7 @@ impl RequestLog {
     where
         Payload: Serialize,
     {
-        let logs_dir = Paths::in_state_dir("logs");
+        let logs_dir = crate::logging::prepare_log_directory("llm", true)?;
-        std::fs::create_dir_all(&logs_dir)?;
 
         let request_id = Uuid::new_v4();
         let temp_name = format!("llm_request.{request_id}.jsonl");
@@ -529,7 +527,7 @@ impl RequestLog {
     fn finish(&mut self) -> Result<()> {
         if let Some(mut writer) = self.writer.take() {
             writer.flush()?;
-            let logs_dir = Paths::in_state_dir("logs");
+            let logs_dir = crate::logging::prepare_log_directory("llm", true)?;
             let log_path = |i| logs_dir.join(format!("llm_request.{}.jsonl", i));
 
             for i in (0..LOGS_TO_KEEP - 1).rev() {