Skip to content

feat: add log rotation for forester #1875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sergeytimoshin merged 1 commit into from
Jul 21, 2025
Merged

feat: add log rotation for forester #1875

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 37 additions & 17 deletions forester/src/telemetry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,54 @@ static INIT: Once = Once::new();

pub fn setup_telemetry() {
INIT.call_once(|| {
let file_appender = RollingFileAppender::new(Rotation::HOURLY, "logs", "forester.log");
let (non_blocking, _guard) = tracing_appender::non_blocking(file_appender);
let file_appender = match RollingFileAppender::builder()
.rotation(Rotation::HOURLY)
.filename_prefix("forester")
.filename_suffix("log")
.max_log_files(48) // 2 days
.build("logs")
{
Ok(appender) => Some(appender),
Err(e) => {
eprintln!(
"Warning: Failed to create log file appender: {}. Logging to stdout only.",
e
);
None
}
};

let env_filter = EnvFilter::try_from_default_env()
.unwrap_or_else(|_| EnvFilter::new("info,forester=debug"));

let file_env_filter = EnvFilter::new("info,forester=debug");
let env_filter =
EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info"));

let stdout_env_filter =
EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("debug"));
EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info"));

let stdout_layer = fmt::Layer::new()
.with_writer(std::io::stdout)
.with_ansi(true)
.with_filter(stdout_env_filter);

let file_layer = fmt::Layer::new()
.with_writer(non_blocking)
.with_filter(file_env_filter);
if let Some(file_appender) = file_appender {
let (non_blocking, _guard) = tracing_appender::non_blocking(file_appender);
let file_env_filter = EnvFilter::new("info");
let file_layer = fmt::Layer::new()
.with_writer(non_blocking)
.with_filter(file_env_filter);

tracing_subscriber::registry()
.with(stdout_layer)
.with(file_layer)
.with(env_filter)
.init();
tracing_subscriber::registry()
.with(stdout_layer)
.with(file_layer)
.with(env_filter)
.init();

// Keep _guard in scope to keep the non-blocking writer alive
std::mem::forget(_guard);
std::mem::forget(_guard);

@coderabbitai coderabbitai Bot Jul 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Memory leak: Replace std::mem::forget with proper guard management.

Using std::mem::forget(_guard) intentionally leaks memory, which is problematic for long-running applications. The guard should be stored and managed properly.

Consider one of these solutions:

Solution 1 (Recommended): Store the guard in a static

+use std::sync::Mutex;
+use once_cell::sync::Lazy;
+
+static GUARD: Lazy<Mutex<Option<tracing_appender::non_blocking::WorkerGuard>>> = 
+    Lazy::new(|| Mutex::new(None));

 // Inside the function:
-std::mem::forget(_guard);
+*GUARD.lock().unwrap() = Some(_guard);

Solution 2: Use a leaked Box for controlled memory management

-std::mem::forget(_guard);
+Box::leak(Box::new(_guard));

Solution 3: Accept the guard will be dropped and document the trade-off

-std::mem::forget(_guard);
+// Note: Guard is intentionally dropped here, which may cause log loss
+// during shutdown, but prevents memory leaks in long-running processes
🤖 Prompt for AI Agents 
In forester/src/telemetry.rs at line 52, the use of std::mem::forget(_guard)
causes a memory leak by intentionally preventing the guard from being dropped.
To fix this, replace std::mem::forget with proper guard management by storing
the guard in a static variable to keep it alive for the program's duration, or
alternatively use a leaked Box to manage the memory more explicitly. Avoid
forgetting the guard without tracking it, ensuring it is properly stored and
managed to prevent leaks.

} else {
tracing_subscriber::registry()
.with(stdout_layer)
.with(env_filter)
.init();
}
});
}

Expand Down