From f969c20e714105a245de8a035c7752e0ca386480 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Fri, 5 Dec 2025 20:42:22 -0700 Subject: [PATCH] move log-message formatting code to its own module --- signal-gateway/src/gateway/log_handler.rs | 87 ++------------------ signal-gateway/src/lib.rs | 3 +- signal-gateway/src/log_format.rs | 98 +++++++++++++++++++++++ 3 files changed, 107 insertions(+), 81 deletions(-) create mode 100644 signal-gateway/src/log_format.rs diff --git a/signal-gateway/src/gateway/log_handler.rs b/signal-gateway/src/gateway/log_handler.rs index aa24db9..bccf1ea 100644 --- a/signal-gateway/src/gateway/log_handler.rs +++ b/signal-gateway/src/gateway/log_handler.rs @@ -3,10 +3,10 @@ use super::route::{Destination, Limit, Route}; use super::{AdminMessage, LimitResult, Limiter, LimiterSet}; use crate::{ concurrent_map::ConcurrentMap, - human_duration::HumanTMinus, + log_format::LogFormatConfig, log_message::{LogMessage, Origin}, }; -use chrono::{TimeDelta, Utc}; +use chrono::Utc; use conf::Conf; use std::fmt; use tokio::sync::{Mutex, mpsc::UnboundedSender}; @@ -54,13 +54,12 @@ pub struct LogHandlerConfig { /// Overall rate limits applied after route checks pass. #[conf(long, env, value_parser = serde_json::from_str, default_value = "[]")] pub overall_limits: Vec, - #[conf(long, env)] - pub format_module: bool, - #[conf(long, env)] - pub format_source_location: bool, /// Number of recent log messages to buffer per origin #[conf(long, env, default_value = "64")] pub log_buffer_size: usize, + /// Log message formatting options. + #[conf(flatten)] + pub log_format: LogFormatConfig, } /// The log handler takes log messages and decides what to do with them. @@ -132,7 +131,7 @@ impl LogHandler { buffer.with_iter(|iter| { writeln!(&mut text, "{} log messages (newest first):", iter.len()).unwrap(); for log_msg in iter { - self.write_log_msg(&mut text, log_msg, now); + self.config.log_format.write_log_msg(&mut text, log_msg, now); } }); text.push('\n'); @@ -177,7 +176,7 @@ impl LogHandler { let now = Utc::now().timestamp(); buffer.push_back_and_drain(log_msg, |log_msg| { - self.write_log_msg(&mut text, log_msg, now); + self.config.log_format.write_log_msg(&mut text, log_msg, now); }); Some(text) @@ -201,65 +200,6 @@ impl LogHandler { } } - // Format a log message into a Writer, followed by \n, and using any config options to do so - fn write_log_msg(&self, mut writer: impl std::fmt::Write, log_msg: &LogMessage, now: i64) { - let sev = log_msg.level.to_str(); - let msg = &log_msg.msg; - - // Format relative timestamp if available - let time_str = if let Some(ts) = log_msg.timestamp { - let diff_secs = now.saturating_sub(ts); - HumanTMinus(TimeDelta::seconds(diff_secs)).to_string() - } else { - "T-?".to_owned() - }; - - // Extract metadata from structured data if configured - let mut metadata_parts = Vec::new(); - - if self.config.format_module - && let Some(module) = log_msg.module_path.as_ref() - { - metadata_parts.push(module.to_string()); - } - - if self.config.format_source_location { - let file_opt = log_msg.file.as_ref(); - let line_opt = log_msg.line.as_ref(); - - let location = if let Some(file) = file_opt { - // Strip /home/{username}/ prefix if present - let trimmed_file = strip_prefix_and_one_slash(file, "/home/"); - // Strip .cargo/registry/src/{hash}/ if present - let trimmed_file = strip_prefix_and_one_slash(trimmed_file, ".cargo/registry/src/"); - - if let Some(line) = line_opt { - format!("{trimmed_file}:{line}") - } else { - format!("{trimmed_file}:?") - } - } else { - // No file present, use "?" even if line is present - "?".to_owned() - }; - - metadata_parts.push(location); - } - - // Format: "ERROR T-10s [foo bar.rs:42]: message" - // Pad severity to 5 chars (left-aligned), time to 8 chars (right-aligned) - let result = if metadata_parts.is_empty() { - writeln!(writer, "{:<5} {:>8}: {}", sev, time_str, msg) - } else { - let metadata = metadata_parts.join(" "); - writeln!(writer, "{:<5} {:>8} [{}]: {}", sev, time_str, metadata, msg) - }; - - if let Err(err) = result { - error!("Couldn't write log message ({err}): {sev}: {msg}"); - } - } - /// Check if a log message passes all rate limiters. /// /// Tests the message against each route's filter in succession (no early return). @@ -326,16 +266,3 @@ impl LogHandler { Ok(first_destination) } } - -// Strip a prefix, then find the first remaining slash and skip up to that as well. -fn strip_prefix_and_one_slash<'a>(target: &'a str, prefix: &str) -> &'a str { - let Some(target) = target.strip_prefix(prefix) else { - return target; - }; - - if let Some((_, after)) = target.split_once('/') { - after - } else { - target - } -} diff --git a/signal-gateway/src/lib.rs b/signal-gateway/src/lib.rs index b5d9fb4..0ef925c 100644 --- a/signal-gateway/src/lib.rs +++ b/signal-gateway/src/lib.rs @@ -12,8 +12,9 @@ pub mod message_handler; pub(crate) mod circular_buffer; pub(crate) mod concurrent_map; pub(crate) mod human_duration; -pub(crate) mod signal_jsonrpc; +pub(crate) mod log_format; pub(crate) mod log_message; +pub(crate) mod signal_jsonrpc; pub(crate) mod prometheus; pub(crate) mod transports; diff --git a/signal-gateway/src/log_format.rs b/signal-gateway/src/log_format.rs new file mode 100644 index 0000000..d078122 --- /dev/null +++ b/signal-gateway/src/log_format.rs @@ -0,0 +1,98 @@ +//! Log message formatting configuration. +//! +//! This module provides configuration and formatting for log messages, +//! controlling how they are rendered for alerts. + +use crate::human_duration::HumanTMinus; +use crate::log_message::LogMessage; +use chrono::TimeDelta; +use conf::Conf; +use tracing::error; + +/// Configuration for log message formatting. +#[derive(Clone, Conf, Debug, Default)] +pub struct LogFormatConfig { + /// Include the module path in formatted output. + #[conf(long, env)] + pub format_module: bool, + /// Include the source file and line number in formatted output. + #[conf(long, env)] + pub format_source_location: bool, +} + +impl LogFormatConfig { + /// Format a log message into a Writer, followed by \n. + /// + /// The `now` parameter is the current timestamp in seconds, used to + /// calculate relative timestamps (e.g., "T-10s"). + pub fn write_log_msg(&self, mut writer: impl std::fmt::Write, log_msg: &LogMessage, now: i64) { + let sev = log_msg.level.to_str(); + let msg = &log_msg.msg; + + // Format relative timestamp if available + let time_str = if let Some(ts) = log_msg.timestamp { + let diff_secs = now.saturating_sub(ts); + HumanTMinus(TimeDelta::seconds(diff_secs)).to_string() + } else { + "T-?".to_owned() + }; + + // Extract metadata from structured data if configured + let mut metadata_parts = Vec::new(); + + if self.format_module + && let Some(module) = log_msg.module_path.as_ref() + { + metadata_parts.push(module.to_string()); + } + + if self.format_source_location { + let file_opt = log_msg.file.as_ref(); + let line_opt = log_msg.line.as_ref(); + + let location = if let Some(file) = file_opt { + // Strip /home/{username}/ prefix if present + let trimmed_file = strip_prefix_and_one_slash(file, "/home/"); + // Strip .cargo/registry/src/{hash}/ if present + let trimmed_file = strip_prefix_and_one_slash(trimmed_file, ".cargo/registry/src/"); + + if let Some(line) = line_opt { + format!("{trimmed_file}:{line}") + } else { + format!("{trimmed_file}:?") + } + } else { + // No file present, use "?" even if line is present + "?".to_owned() + }; + + metadata_parts.push(location); + } + + // Format: "ERROR T-10s [foo bar.rs:42]: message" + // Pad severity to 5 chars (left-aligned), time to 8 chars (right-aligned) + let result = if metadata_parts.is_empty() { + writeln!(writer, "{:<5} {:>8}: {}", sev, time_str, msg) + } else { + let metadata = metadata_parts.join(" "); + writeln!(writer, "{:<5} {:>8} [{}]: {}", sev, time_str, metadata, msg) + }; + + if let Err(err) = result { + error!("Couldn't write log message ({err}): {sev}: {msg}"); + } + } +} + +// Strip a prefix, then find the first remaining slash and skip up to that as well. +fn strip_prefix_and_one_slash<'a>(target: &'a str, prefix: &str) -> &'a str { + let Some(target) = target.strip_prefix(prefix) else { + return target; + }; + + if let Some((_, after)) = target.split_once('/') { + after + } else { + target + } +}