diff --git a/crates/goose-cli/src/logging.rs b/crates/goose-cli/src/logging.rs index f65fe5a4df6a..acd9d16d23ee 100644 --- a/crates/goose-cli/src/logging.rs +++ b/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 { - 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 = get_log_directory()?; + let log_dir = goose::logging::prepare_log_directory("cli", true)?; 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()); diff --git a/crates/goose-server/src/logging.rs b/crates/goose-server/src/logging.rs index 7179e3cf7da3..0a5eadc20cf1 100644 --- a/crates/goose-server/src/logging.rs +++ b/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 { - 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 = get_log_directory()?; + let log_dir = goose::logging::prepare_log_directory("server", true)?; 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, log_dir, log_filename); + let file_appender = tracing_appender::rolling::RollingFileAppender::new( + Rotation::NEVER, // we do manual rotation via file naming and cleanup_old_logs + log_dir, + log_filename, + ); // Create JSON file logging layer let file_layer = fmt::layer() diff --git a/crates/goose/src/logging.rs b/crates/goose/src/logging.rs index a6742d3d1e8f..80b96a935d85 100644 --- a/crates/goose/src/logging.rs +++ b/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 { +pub fn prepare_log_directory(component: &str, use_date_subdir: bool) -> Result { 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 - let now = chrono::Local::now(); - component_dir.join(now.format("%Y-%m-%d").to_string()) + component_dir.join(chrono::Local::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(); diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index c227b4ddf1c7..def0c4e544d7 100644 --- a/crates/goose/src/providers/utils.rs +++ b/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"); - std::fs::create_dir_all(&logs_dir)?; + let logs_dir = crate::logging::prepare_log_directory("llm", true)?; 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() {