From 9ebd5ac44b88a5cc27829a0b24b1164038af3272 Mon Sep 17 00:00:00 2001 From: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:05:52 -0500 Subject: [PATCH 1/4] feat: add size-based operational log rotation Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> --- crates/cli/src/configuration/logging.rs | 21 +- .../cli/tests/coverage/shared/config_tests.rs | 50 ++++ crates/core/src/logging/config.rs | 72 ++++++ crates/core/src/logging/mod.rs | 6 +- crates/core/src/logging/rotation.rs | 151 ++++++++++++ crates/core/src/logging/sink.rs | 105 ++++++-- crates/core/tests/coverage/logging_tests.rs | 225 +++++++++++++++++- docs/reference/operational-logging.mdx | 9 +- 8 files changed, 605 insertions(+), 34 deletions(-) create mode 100644 crates/core/src/logging/rotation.rs diff --git a/crates/cli/src/configuration/logging.rs b/crates/cli/src/configuration/logging.rs index 85cf7b8f8..69b1909a7 100644 --- a/crates/cli/src/configuration/logging.rs +++ b/crates/cli/src/configuration/logging.rs @@ -7,8 +7,9 @@ use std::path::PathBuf; use nemo_relay::error::FlowError; use nemo_relay::logging::{ - DEFAULT_FILE_FLUSH_INTERVAL_MILLIS, DEFAULT_FILE_SINK_QUEUE_ENTRIES, FileLogSinkConfig, - LogFormat, LogLevel, LogSinkConfig, LoggingConfig, MAX_FILE_SINK_QUEUE_ENTRIES, + DEFAULT_FILE_FLUSH_INTERVAL_MILLIS, DEFAULT_FILE_SINK_QUEUE_ENTRIES, FileLogRotationConfig, + FileLogSinkConfig, LogFormat, LogLevel, LogSinkConfig, LoggingConfig, + MAX_FILE_SINK_QUEUE_ENTRIES, }; use serde::Deserialize; @@ -36,6 +37,8 @@ struct RawFileLogSinkConfig { /// Optional advanced: pending async queue entries per file sink (default /// [`DEFAULT_FILE_SINK_QUEUE_ENTRIES`]). queue_capacity: Option, + max_file_size_bytes: Option, + retained_files: Option, } pub(super) fn apply_file_logging_config( @@ -100,11 +103,25 @@ fn parse_file_log_sink( Some(capacity) => capacity, None => DEFAULT_FILE_SINK_QUEUE_ENTRIES, }; + let rotation = match (config.max_file_size_bytes, config.retained_files) { + (None, None) => None, + (Some(max_file_size_bytes), Some(retained_files)) => Some( + FileLogRotationConfig::new(max_file_size_bytes, retained_files) + .map_err(logging_parse_error)?, + ), + _ => { + return Err(CliError::Config( + "logging sink max_file_size_bytes and retained_files must be configured together" + .into(), + )); + } + }; Ok(LogSinkConfig::File(FileLogSinkConfig { path, level, format, queue_capacity, + rotation, })) } diff --git a/crates/cli/tests/coverage/shared/config_tests.rs b/crates/cli/tests/coverage/shared/config_tests.rs index f74db743a..103e3ff54 100644 --- a/crates/cli/tests/coverage/shared/config_tests.rs +++ b/crates/cli/tests/coverage/shared/config_tests.rs @@ -3271,6 +3271,56 @@ format = "human" } } +#[test] +fn logging_rotation_cli_config_preserves_pair_and_rejects_incomplete_pair() { + let temp = tempfile::tempdir().unwrap(); + let config_path = isolated_config_path(&temp); + let log_path = temp.path().join("relay.log.jsonl"); + std::fs::write( + &config_path, + format!( + r#" +[[logging.sinks]] +path = {} +max_file_size_bytes = 1024 +retained_files = 2 +"#, + toml_basic_string(log_path.to_string_lossy().as_ref()) + ), + ) + .unwrap(); + + let resolved = resolve_server_config(&GatewayOverrides { + config: Some(config_path), + ..GatewayOverrides::default() + }) + .unwrap(); + let LogSinkConfig::File(sink) = &resolved.logging.sinks[0]; + let rotation = sink.rotation.expect("complete rotation configuration"); + assert_eq!(rotation.max_file_size_bytes(), 1024); + assert_eq!(rotation.retained_files(), 2); + + let incomplete_path = isolated_config_path(&temp); + std::fs::write( + &incomplete_path, + r#" +[[logging.sinks]] +path = "relay.log.jsonl" +max_file_size_bytes = 1024 +"#, + ) + .unwrap(); + let error = resolve_server_config(&GatewayOverrides { + config: Some(incomplete_path), + ..GatewayOverrides::default() + }) + .unwrap_err() + .to_string(); + assert!(error.contains( + "logging sink max_file_size_bytes and retained_files must be configured together" + )); +} + #[test] fn logging_rejects_invalid_level_format_missing_path_and_zero_queue() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/core/src/logging/config.rs b/crates/core/src/logging/config.rs index 868881fb9..d1dbca04a 100644 --- a/crates/core/src/logging/config.rs +++ b/crates/core/src/logging/config.rs @@ -28,6 +28,13 @@ pub const DEFAULT_FILE_FLUSH_INTERVAL_MILLIS: u64 = 1000; /// configuration above this bound is rejected with a config error. It cannot be raised. pub const MAX_FILE_SINK_QUEUE_ENTRIES: usize = 8_192; +/// Fixed hard maximum number of retained backup files per rotating file sink. +/// +/// Size-based rotation renames existing backup files on each rotation, so an unbounded value can +/// make one log write perform excessive filesystem work. This limit counts backup files and does +/// not include the active log file. +pub const MAX_FILE_SINK_RETAINED_FILES: usize = 100; + /// Operational logging configuration for [`LoggingRuntime::configure`](super::LoggingRuntime::configure). /// /// `level` is the process-wide **minimum severity**: call sites may emit any level, but records @@ -219,6 +226,8 @@ pub struct FileLogSinkConfig { /// Maximum pending asynchronous queue entries for this file sink. Must be greater than 0 and /// at most [`MAX_FILE_SINK_QUEUE_ENTRIES`]. pub queue_capacity: usize, + /// Optional size-based rotation and retention settings. + pub rotation: Option, } impl Default for FileLogSinkConfig { @@ -228,10 +237,56 @@ impl Default for FileLogSinkConfig { level: LogLevel::Info, format: LogFormat::Jsonl, queue_capacity: DEFAULT_FILE_SINK_QUEUE_ENTRIES, + rotation: None, } } } +/// Size-based rotation settings for a file log sink. +/// +/// `retained_files` counts previous log files and excludes the active file. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct FileLogRotationConfig { + max_file_size_bytes: u64, + retained_files: usize, +} + +impl FileLogRotationConfig { + /// Creates validated size-based rotation settings. + pub fn new(max_file_size_bytes: u64, retained_files: usize) -> Result { + if max_file_size_bytes == 0 { + return Err(FlowError::InvalidArgument( + "logging sink max_file_size_bytes must be greater than 0".into(), + )); + } + if retained_files == 0 { + return Err(FlowError::InvalidArgument( + "logging sink retained_files must be greater than 0".into(), + )); + } + if retained_files > MAX_FILE_SINK_RETAINED_FILES { + return Err(FlowError::InvalidArgument(format!( + "logging sink retained_files {retained_files} exceeds maximum \ + {MAX_FILE_SINK_RETAINED_FILES} backup files per sink" + ))); + } + Ok(Self { + max_file_size_bytes, + retained_files, + }) + } + + /// Maximum active file size before the next record triggers rotation. + pub fn max_file_size_bytes(self) -> u64 { + self.max_file_size_bytes + } + + /// Number of previous log files retained in addition to the active file. + pub fn retained_files(self) -> usize { + self.retained_files + } +} + #[derive(Debug, Deserialize)] struct LoggingDocument { logging: Option, @@ -277,6 +332,8 @@ struct RawFileLogSinkConfig { level: Option, format: Option, queue_capacity: Option, + max_file_size_bytes: Option, + retained_files: Option, } impl RawFileLogSinkConfig { @@ -318,11 +375,26 @@ impl RawFileLogSinkConfig { None => DEFAULT_FILE_SINK_QUEUE_ENTRIES, }; + let rotation = match (self.max_file_size_bytes, self.retained_files) { + (None, None) => None, + (Some(max_file_size_bytes), Some(retained_files)) => Some(FileLogRotationConfig::new( + max_file_size_bytes, + retained_files, + )?), + _ => { + return Err(FlowError::InvalidArgument( + "logging sink max_file_size_bytes and retained_files must be configured \ + together" + .into(), + )); + } + }; Ok(LogSinkConfig::File(FileLogSinkConfig { path, level, format, queue_capacity, + rotation, })) } } diff --git a/crates/core/src/logging/mod.rs b/crates/core/src/logging/mod.rs index 5c146888e..9b3c7528c 100644 --- a/crates/core/src/logging/mod.rs +++ b/crates/core/src/logging/mod.rs @@ -8,6 +8,7 @@ mod config; mod format; +mod rotation; mod sink; use std::io::{self, Write}; @@ -21,8 +22,9 @@ use uuid::Uuid; use crate::error::{FlowError, Result}; pub use config::{ - DEFAULT_FILE_FLUSH_INTERVAL_MILLIS, DEFAULT_FILE_SINK_QUEUE_ENTRIES, FileLogSinkConfig, - LogFormat, LogLevel, LogSinkConfig, LoggingConfig, MAX_FILE_SINK_QUEUE_ENTRIES, + DEFAULT_FILE_FLUSH_INTERVAL_MILLIS, DEFAULT_FILE_SINK_QUEUE_ENTRIES, FileLogRotationConfig, + FileLogSinkConfig, LogFormat, LogLevel, LogSinkConfig, LoggingConfig, + MAX_FILE_SINK_QUEUE_ENTRIES, MAX_FILE_SINK_RETAINED_FILES, }; pub(crate) use sink::build_logger; use sink::log_level_filter; diff --git a/crates/core/src/logging/rotation.rs b/crates/core/src/logging/rotation.rs new file mode 100644 index 000000000..d3a0d7931 --- /dev/null +++ b/crates/core/src/logging/rotation.rs @@ -0,0 +1,151 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Size-based file rotation for operational log sinks. + +use std::fs::{self, File, OpenOptions}; +use std::io::{self, BufWriter, Write}; +use std::path::{Path, PathBuf}; + +pub(crate) struct SizeRotatingFileWriter { + base_path: PathBuf, + file: Option>, + current_size: u64, + max_file_size_bytes: u64, + retained_files: usize, +} + +impl SizeRotatingFileWriter { + pub(crate) fn new( + base_path: PathBuf, + max_file_size_bytes: u64, + retained_files: usize, + ) -> io::Result { + create_parent_directory(&base_path)?; + let file = open_active_file(&base_path, false)?; + let current_size = file.get_ref().metadata()?.len(); + + Ok(Self { + base_path, + file: Some(file), + current_size, + max_file_size_bytes, + retained_files, + }) + } + + fn rotate_if_needed(&mut self, incoming_bytes: usize) -> io::Result<()> { + if self.current_size == 0 + || self.current_size.saturating_add(incoming_bytes as u64) <= self.max_file_size_bytes + { + return Ok(()); + } + + self.rotate() + } + + fn rotate(&mut self) -> io::Result<()> { + let mut file = self + .file + .take() + .ok_or_else(|| io::Error::other("rotating log file is not open"))?; + if let Err(error) = file.flush() { + self.file = Some(file); + return Err(error); + } + drop(file); + + if let Err(error) = rotate_files(&self.base_path, self.retained_files) { + return match self.reopen_after_failed_rotation() { + Ok(()) => Err(error), + Err(reopen_error) => Err(io::Error::new( + reopen_error.kind(), + format!( + "log rotation failed: {error}; failed to reopen active log file: \ + {reopen_error}" + ), + )), + }; + } + + self.file = Some(open_active_file(&self.base_path, true)?); + self.current_size = 0; + Ok(()) + } + + fn reopen_after_failed_rotation(&mut self) -> io::Result<()> { + let file = open_active_file(&self.base_path, false)?; + self.current_size = file.get_ref().metadata()?.len(); + self.file = Some(file); + Ok(()) + } +} + +impl Write for SizeRotatingFileWriter { + fn write(&mut self, buffer: &[u8]) -> io::Result { + self.rotate_if_needed(buffer.len())?; + self.file + .as_mut() + .ok_or_else(|| io::Error::other("rotating log file is not open"))? + .write_all(buffer)?; + self.current_size = self.current_size.saturating_add(buffer.len() as u64); + Ok(buffer.len()) + } + + fn flush(&mut self) -> io::Result<()> { + self.file + .as_mut() + .ok_or_else(|| io::Error::other("rotating log file is not open"))? + .flush() + } +} + +fn create_parent_directory(path: &Path) -> io::Result<()> { + if let Some(parent) = path.parent() + && !parent.as_os_str().is_empty() + { + fs::create_dir_all(parent)?; + } + Ok(()) +} + +fn open_active_file(path: &Path, truncate: bool) -> io::Result> { + let file = OpenOptions::new() + .create(true) + .write(true) + .append(!truncate) + .truncate(truncate) + .open(path)?; + Ok(BufWriter::new(file)) +} + +fn rotate_files(base_path: &Path, retained_files: usize) -> io::Result<()> { + for index in (1..=retained_files).rev() { + let source = if index == 1 { + base_path.to_path_buf() + } else { + rotated_log_path(base_path, index - 1) + }; + if !source.exists() { + continue; + } + + let destination = rotated_log_path(base_path, index); + if destination.exists() { + fs::remove_file(&destination)?; + } + fs::rename(source, destination)?; + } + Ok(()) +} + +pub(crate) fn rotated_log_path(base_path: &Path, index: usize) -> PathBuf { + let stem = base_path.file_stem().unwrap_or(base_path.as_os_str()); + let mut file_name = stem.to_os_string(); + file_name.push(format!("_{index}")); + if let Some(extension) = base_path.extension() { + file_name.push("."); + file_name.push(extension); + } + base_path.with_file_name(file_name) +} diff --git a/crates/core/src/logging/sink.rs b/crates/core/src/logging/sink.rs index bb0456804..51ac42d10 100644 --- a/crates/core/src/logging/sink.rs +++ b/crates/core/src/logging/sink.rs @@ -10,12 +10,13 @@ use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use spdlog::sink::{AsyncPoolSink, FileSink, OverflowPolicy, StdStreamSink}; +use spdlog::sink::{AsyncPoolSink, FileSink, OverflowPolicy, StdStreamSink, WriteSink}; use spdlog::terminal_style::StyleMode; use spdlog::{Level, LevelFilter, Logger, ThreadPool}; use super::config::{LogLevel, LogSinkConfig, LoggingConfig, MAX_FILE_SINK_QUEUE_ENTRIES}; use super::format::RelayFormatter; +use super::rotation::{SizeRotatingFileWriter, rotated_log_path}; use crate::error::{FlowError, Result}; pub(crate) fn build_logger( @@ -24,7 +25,8 @@ pub(crate) fn build_logger( ) -> Result<(Arc, Vec>)> { let mut sinks: Vec> = Vec::new(); let mut thread_pools = Vec::new(); - let mut resolved_paths: Vec = Vec::new(); + let mut active_paths: Vec = Vec::new(); + let mut reserved_paths: Vec = Vec::new(); let stderr_sink = StdStreamSink::builder() .stderr() @@ -44,36 +46,70 @@ pub(crate) fn build_logger( for sink in &config.sinks { let LogSinkConfig::File(file_sink) = sink; let resolved_path = resolve_log_path(&file_sink.path)?; - if resolved_paths - .iter() - .any(|existing| existing == &resolved_path) - { + if active_paths.contains(&resolved_path) { return Err(FlowError::InvalidArgument(format!( "duplicate logging sink path {}", resolved_path.display() ))); } - resolved_paths.push(resolved_path.clone()); + let candidate_paths = reserved_sink_paths(&resolved_path, file_sink.rotation); + if let Some(collision) = candidate_paths + .iter() + .find(|candidate| reserved_paths.contains(candidate)) + { + return Err(FlowError::InvalidArgument(format!( + "logging sink path conflicts with another active or rotated file: {}", + collision.display() + ))); + } + active_paths.push(resolved_path.clone()); + reserved_paths.extend(candidate_paths); - // FileSink performs the real open/append. AsyncPoolSink is spdlog's stock bounded queue + - // worker pool in front of that file so hot paths enqueue instead of blocking on disk I/O. - // Overflow drops incoming records so a stuck disk cannot stall the process. - let file = FileSink::builder() - .path(&resolved_path) - .truncate(false) - .formatter(RelayFormatter { - format: file_sink.format, - root_relay_id: root_relay_id.clone(), - }) - .level_filter(spdlog_level_filter(file_sink.level)) - .error_handler(stderr_error_handler(&resolved_path.display().to_string())) - .build_arc() - .map_err(|error| { - FlowError::InvalidArgument(format!( - "failed to open logging sink {}: {error}", - resolved_path.display() - )) - })?; + let file: Arc = match file_sink.rotation { + None => FileSink::builder() + .path(&resolved_path) + .truncate(false) + .formatter(RelayFormatter { + format: file_sink.format, + root_relay_id: root_relay_id.clone(), + }) + .level_filter(spdlog_level_filter(file_sink.level)) + .error_handler(stderr_error_handler(&resolved_path.display().to_string())) + .build_arc() + .map_err(|error| { + FlowError::InvalidArgument(format!( + "failed to open logging sink {}: {error}", + resolved_path.display() + )) + })?, + Some(rotation) => WriteSink::builder() + .target( + SizeRotatingFileWriter::new( + resolved_path.clone(), + rotation.max_file_size_bytes(), + rotation.retained_files(), + ) + .map_err(|error| { + FlowError::InvalidArgument(format!( + "failed to open rotating logging sink {}: {error}", + resolved_path.display() + )) + })?, + ) + .formatter(RelayFormatter { + format: file_sink.format, + root_relay_id: root_relay_id.clone(), + }) + .level_filter(spdlog_level_filter(file_sink.level)) + .error_handler(stderr_error_handler(&resolved_path.display().to_string())) + .build_arc() + .map_err(|error| { + FlowError::InvalidArgument(format!( + "failed to open rotating logging sink {}: {error}", + resolved_path.display() + )) + })?, + }; if file_sink.queue_capacity > MAX_FILE_SINK_QUEUE_ENTRIES { return Err(FlowError::InvalidArgument(format!( @@ -132,6 +168,23 @@ pub(crate) fn build_logger( Ok((logger, thread_pools)) } +fn reserved_sink_paths( + resolved_path: &Path, + rotation: Option, +) -> Vec { + let mut paths = vec![resolved_path.to_path_buf()]; + if let Some(rotation) = rotation { + paths.reserve(rotation.retained_files()); + for index in 1..=rotation.retained_files() { + paths.push(logging_path_identity(&rotated_log_path( + resolved_path, + index, + ))); + } + } + paths +} + fn resolve_log_path(path: &Path) -> Result { // Relative paths resolve against process CWD. Absolute paths are unchanged. No `~` or env // expansion. diff --git a/crates/core/tests/coverage/logging_tests.rs b/crates/core/tests/coverage/logging_tests.rs index 2fc6fe558..cc231221f 100644 --- a/crates/core/tests/coverage/logging_tests.rs +++ b/crates/core/tests/coverage/logging_tests.rs @@ -2,13 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 use crate::logging::{ - FileLogSinkConfig, LogFormat, LogLevel, LogSinkConfig, LoggingConfig, LoggingRuntime, - MAX_FILE_SINK_QUEUE_ENTRIES, build_logger, format_event_for_test, init_logging, + FileLogRotationConfig, FileLogSinkConfig, LogFormat, LogLevel, LogSinkConfig, LoggingConfig, + LoggingRuntime, MAX_FILE_SINK_QUEUE_ENTRIES, MAX_FILE_SINK_RETAINED_FILES, build_logger, + format_event_for_test, init_logging, }; use serde_json::Value; use spdlog::Level; use std::ffi::{OsStr, OsString}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Barrier, Mutex, MutexGuard}; static LOGGING_TEST_LOCK: Mutex<()> = Mutex::new(()); @@ -239,6 +240,7 @@ queue_capacity = 7 level: LogLevel::Warn, format: LogFormat::Human, queue_capacity: 7, + rotation: None, })] ); } @@ -295,6 +297,97 @@ queue_capacity = 7 ); } +#[test] +fn logging_rotation_configuration_parses_complete_pair_and_rejects_invalid_values() { + let config = LoggingConfig::from_toml_document( + r#" +[logging] + +[[logging.sinks]] +path = "relay.log.jsonl" +max_file_size_bytes = 1024 +retained_files = 2 +"#, + ) + .unwrap(); + + let LogSinkConfig::File(sink) = &config.sinks[0]; + assert_eq!( + sink.rotation, + Some(FileLogRotationConfig::new(1024, 2).unwrap()) + ); + + let invalid_documents = [ + ( + "missing retained_files", + r#" +[logging] +[[logging.sinks]] +path = "relay.log.jsonl" +max_file_size_bytes = 1024 +"# + .to_owned(), + "must be configured together", + ), + ( + "missing max_file_size_bytes", + r#" +[logging] +[[logging.sinks]] +path = "relay.log.jsonl" +retained_files = 2 +"# + .to_owned(), + "must be configured together", + ), + ( + "zero max_file_size_bytes", + r#" +[logging] +[[logging.sinks]] +path = "relay.log.jsonl" +max_file_size_bytes = 0 +retained_files = 2 +"# + .to_owned(), + "max_file_size_bytes must be greater than 0", + ), + ( + "zero retained_files", + r#" +[logging] +[[logging.sinks]] +path = "relay.log.jsonl" +max_file_size_bytes = 1024 +retained_files = 0 +"# + .to_owned(), + "retained_files must be greater than 0", + ), + ( + "retained_files over maximum", + format!( + r#" +[logging] +[[logging.sinks]] +path = "relay.log.jsonl" +max_file_size_bytes = 1024 +retained_files = {} +"#, + MAX_FILE_SINK_RETAINED_FILES + 1 + ), + "exceeds maximum", + ), + ]; + + for (name, document, expected) in invalid_documents { + let error = LoggingConfig::from_toml_document(&document) + .unwrap_err() + .to_string(); + assert!(error.contains(expected), "{name}: {error}"); + } +} + #[test] #[cfg(unix)] fn logging_environment_rejects_non_unicode_values() { @@ -452,6 +545,132 @@ fn wait_for_log_line(path: &std::path::Path, ready: impl Fn(&str) -> bool) -> St std::fs::read_to_string(path).unwrap_or_default() } +fn read_single_jsonl_record(path: &Path) -> Value { + let contents = std::fs::read_to_string(path).unwrap(); + let mut lines = contents.lines(); + let record = serde_json::from_str(lines.next().expect("one JSONL record")).unwrap(); + assert!( + lines.next().is_none(), + "expected exactly one JSONL record in {}, got {contents:?}", + path.display() + ); + record +} + +#[test] +fn logging_rotation_retains_newest_backups_and_complete_records() { + let _lock = lock_logging_tests(); + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("relay.log.jsonl"); + let config = LoggingConfig { + sinks: vec![LogSinkConfig::File(FileLogSinkConfig { + path: path.clone(), + rotation: Some(FileLogRotationConfig::new(1, 2).unwrap()), + ..FileLogSinkConfig::default() + })], + ..default_config() + }; + + let runtime = init_logging(&config).unwrap(); + log::info!(target: "nemo_relay.rotation_test", event = "rotation_one"; "rotation one"); + log::info!(target: "nemo_relay.rotation_test", event = "rotation_two"; "rotation two"); + log::info!(target: "nemo_relay.rotation_test", event = "rotation_three"; "rotation three"); + runtime.shutdown(); + + let active = read_single_jsonl_record(&path); + let newest_backup = read_single_jsonl_record(&temp.path().join("relay.log_1.jsonl")); + let oldest_backup = read_single_jsonl_record(&temp.path().join("relay.log_2.jsonl")); + + assert_eq!(active["event"], "logging_shutdown_started"); + assert_eq!(newest_backup["event"], "rotation_three"); + assert_eq!(oldest_backup["event"], "rotation_two"); + assert!(!temp.path().join("relay.log_3.jsonl").exists()); +} + +#[test] +fn logging_rotation_rotates_existing_file_at_boundary() { + let _lock = lock_logging_tests(); + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("relay.log.jsonl"); + let existing_record = "x".repeat(64); + std::fs::write(&path, &existing_record).unwrap(); + let config = LoggingConfig { + sinks: vec![LogSinkConfig::File(FileLogSinkConfig { + path: path.clone(), + rotation: Some(FileLogRotationConfig::new(64, 2).unwrap()), + ..FileLogSinkConfig::default() + })], + ..default_config() + }; + + let runtime = init_logging(&config).unwrap(); + runtime.shutdown(); + + assert_eq!( + std::fs::read_to_string(temp.path().join("relay.log_2.jsonl")).unwrap(), + existing_record + ); + assert_eq!( + read_single_jsonl_record(&path)["event"], + "logging_shutdown_started" + ); +} + +#[test] +fn logging_rotation_preserves_historical_backups_outside_retention_window() { + let _lock = lock_logging_tests(); + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("relay.log.jsonl"); + let historical_path = temp.path().join("relay.log_3.jsonl"); + let historical_contents = "historical backup outside current retention window\n"; + std::fs::write(&historical_path, historical_contents).unwrap(); + let config = LoggingConfig { + sinks: vec![LogSinkConfig::File(FileLogSinkConfig { + path, + rotation: Some(FileLogRotationConfig::new(1, 2).unwrap()), + ..FileLogSinkConfig::default() + })], + ..default_config() + }; + + let runtime = init_logging(&config).unwrap(); + runtime.shutdown(); + + assert_eq!( + std::fs::read_to_string(historical_path).unwrap(), + historical_contents + ); +} + +#[test] +fn logging_rotation_rejects_generated_backup_path_collision() { + let _lock = lock_logging_tests(); + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("relay.log.jsonl"); + let config = LoggingConfig { + sinks: vec![ + LogSinkConfig::File(FileLogSinkConfig { + path: path.clone(), + rotation: Some(FileLogRotationConfig::new(1024, 1).unwrap()), + ..FileLogSinkConfig::default() + }), + LogSinkConfig::File(FileLogSinkConfig { + path: temp.path().join("relay.log_1.jsonl"), + ..FileLogSinkConfig::default() + }), + ], + ..default_config() + }; + + let error = build_logger(&config, "root".into()) + .err() + .expect("generated backup path collision should fail") + .to_string(); + + assert!(error.contains("conflicts with another active or rotated file")); + assert!(error.contains("relay.log_1.jsonl")); +} + #[test] fn sink_level_filter_drops_events_below_sink_minimum() { let _lock = lock_logging_tests(); diff --git a/docs/reference/operational-logging.mdx b/docs/reference/operational-logging.mdx index 2202fccde..94767df93 100644 --- a/docs/reference/operational-logging.mdx +++ b/docs/reference/operational-logging.mdx @@ -100,11 +100,18 @@ path = ".nemo-relay/logs/relay.log.jsonl" format = "jsonl" level = "debug" queue_capacity = 1024 +max_file_size_bytes = 10485760 +retained_files = 5 ``` File sink paths are resolved relative to the process working directory. File sinks use asynchronous queues, and `queue_capacity` cannot exceed 8,192 entries -per sink. +per sink. Size-based rotation is optional; when enabled, +`max_file_size_bytes` and `retained_files` must be configured together. +`retained_files` counts backup files in addition to the active file and cannot +exceed 100. A record larger than `max_file_size_bytes` remains intact rather +than being split. File sinks remain append-only when rotation settings are +omitted. ## Rust Library API From 47c07391732406d9b9f6aa34618ce3281139f09f Mon Sep 17 00:00:00 2001 From: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:32:12 -0500 Subject: [PATCH 2/4] fix(logging): preserve backups during rotation Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> --- crates/core/src/logging/rotation.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/core/src/logging/rotation.rs b/crates/core/src/logging/rotation.rs index d3a0d7931..b8b7a5d09 100644 --- a/crates/core/src/logging/rotation.rs +++ b/crates/core/src/logging/rotation.rs @@ -131,9 +131,6 @@ fn rotate_files(base_path: &Path, retained_files: usize) -> io::Result<()> { } let destination = rotated_log_path(base_path, index); - if destination.exists() { - fs::remove_file(&destination)?; - } fs::rename(source, destination)?; } Ok(()) From c5d19bb6aa62440299b9971491fda3caff3c0d91 Mon Sep 17 00:00:00 2001 From: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:49:58 -0500 Subject: [PATCH 3/4] fix(logging): cap retained backup count Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> --- crates/core/src/logging/config.rs | 2 +- docs/reference/operational-logging.mdx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/core/src/logging/config.rs b/crates/core/src/logging/config.rs index d1dbca04a..6dcd41fb9 100644 --- a/crates/core/src/logging/config.rs +++ b/crates/core/src/logging/config.rs @@ -33,7 +33,7 @@ pub const MAX_FILE_SINK_QUEUE_ENTRIES: usize = 8_192; /// Size-based rotation renames existing backup files on each rotation, so an unbounded value can /// make one log write perform excessive filesystem work. This limit counts backup files and does /// not include the active log file. -pub const MAX_FILE_SINK_RETAINED_FILES: usize = 100; +pub const MAX_FILE_SINK_RETAINED_FILES: usize = 9; /// Operational logging configuration for [`LoggingRuntime::configure`](super::LoggingRuntime::configure). /// diff --git a/docs/reference/operational-logging.mdx b/docs/reference/operational-logging.mdx index 94767df93..29f2d092e 100644 --- a/docs/reference/operational-logging.mdx +++ b/docs/reference/operational-logging.mdx @@ -109,7 +109,7 @@ sinks use asynchronous queues, and `queue_capacity` cannot exceed 8,192 entries per sink. Size-based rotation is optional; when enabled, `max_file_size_bytes` and `retained_files` must be configured together. `retained_files` counts backup files in addition to the active file and cannot -exceed 100. A record larger than `max_file_size_bytes` remains intact rather +exceed 9. A record larger than `max_file_size_bytes` remains intact rather than being split. File sinks remain append-only when rotation settings are omitted. From ebcc9e27b6ae64d4e9847086bce8c166eef142b4 Mon Sep 17 00:00:00 2001 From: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> Date: Fri, 24 Jul 2026 18:33:53 -0500 Subject: [PATCH 4/4] fix(logging): use dotted rotation suffixes Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com> --- crates/core/src/logging/rotation.rs | 2 +- crates/core/tests/coverage/logging_tests.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/core/src/logging/rotation.rs b/crates/core/src/logging/rotation.rs index b8b7a5d09..3314377c1 100644 --- a/crates/core/src/logging/rotation.rs +++ b/crates/core/src/logging/rotation.rs @@ -139,7 +139,7 @@ fn rotate_files(base_path: &Path, retained_files: usize) -> io::Result<()> { pub(crate) fn rotated_log_path(base_path: &Path, index: usize) -> PathBuf { let stem = base_path.file_stem().unwrap_or(base_path.as_os_str()); let mut file_name = stem.to_os_string(); - file_name.push(format!("_{index}")); + file_name.push(format!(".{index}")); if let Some(extension) = base_path.extension() { file_name.push("."); file_name.push(extension); diff --git a/crates/core/tests/coverage/logging_tests.rs b/crates/core/tests/coverage/logging_tests.rs index cc231221f..bf2df1fed 100644 --- a/crates/core/tests/coverage/logging_tests.rs +++ b/crates/core/tests/coverage/logging_tests.rs @@ -578,13 +578,13 @@ fn logging_rotation_retains_newest_backups_and_complete_records() { runtime.shutdown(); let active = read_single_jsonl_record(&path); - let newest_backup = read_single_jsonl_record(&temp.path().join("relay.log_1.jsonl")); - let oldest_backup = read_single_jsonl_record(&temp.path().join("relay.log_2.jsonl")); + let newest_backup = read_single_jsonl_record(&temp.path().join("relay.log.1.jsonl")); + let oldest_backup = read_single_jsonl_record(&temp.path().join("relay.log.2.jsonl")); assert_eq!(active["event"], "logging_shutdown_started"); assert_eq!(newest_backup["event"], "rotation_three"); assert_eq!(oldest_backup["event"], "rotation_two"); - assert!(!temp.path().join("relay.log_3.jsonl").exists()); + assert!(!temp.path().join("relay.log.3.jsonl").exists()); } #[test] @@ -607,7 +607,7 @@ fn logging_rotation_rotates_existing_file_at_boundary() { runtime.shutdown(); assert_eq!( - std::fs::read_to_string(temp.path().join("relay.log_2.jsonl")).unwrap(), + std::fs::read_to_string(temp.path().join("relay.log.2.jsonl")).unwrap(), existing_record ); assert_eq!( @@ -621,7 +621,7 @@ fn logging_rotation_preserves_historical_backups_outside_retention_window() { let _lock = lock_logging_tests(); let temp = tempfile::tempdir().unwrap(); let path = temp.path().join("relay.log.jsonl"); - let historical_path = temp.path().join("relay.log_3.jsonl"); + let historical_path = temp.path().join("relay.log.3.jsonl"); let historical_contents = "historical backup outside current retention window\n"; std::fs::write(&historical_path, historical_contents).unwrap(); let config = LoggingConfig { @@ -655,7 +655,7 @@ fn logging_rotation_rejects_generated_backup_path_collision() { ..FileLogSinkConfig::default() }), LogSinkConfig::File(FileLogSinkConfig { - path: temp.path().join("relay.log_1.jsonl"), + path: temp.path().join("relay.log.1.jsonl"), ..FileLogSinkConfig::default() }), ], @@ -668,7 +668,7 @@ fn logging_rotation_rejects_generated_backup_path_collision() { .to_string(); assert!(error.contains("conflicts with another active or rotated file")); - assert!(error.contains("relay.log_1.jsonl")); + assert!(error.contains("relay.log.1.jsonl")); } #[test]