Skip to content

Commit

Permalink
Merge pull request #454 from tnull/2025-01-fix-env-logger
Browse files Browse the repository at this point in the history 
Fix `log`-facade logging and clarify max level
  • Loading branch information
@tnull
tnull authored Feb 4, 2025
2 parents b0f4443 + 9f2714c commit 8bfa8ec
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 46 deletions.
4 changes: 2 additions & 2 deletions bindings/ldk_node.udl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ interface Builder {
void set_gossip_source_rgs(string rgs_server_url);
void set_liquidity_source_lsps2(SocketAddress address, PublicKey node_id, string? token);
void set_storage_dir_path(string storage_dir_path);
void set_filesystem_logger(string? log_file_path, LogLevel? log_level);
void set_log_facade_logger(LogLevel log_level);
void set_filesystem_logger(string? log_file_path, LogLevel? max_log_level);
void set_log_facade_logger(LogLevel? max_log_level);
void set_custom_logger(LogWriter log_writer);
void set_network(Network network);
[Throws=BuildError]
Expand Down
47 changes: 31 additions & 16 deletions src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,22 @@ impl Default for LiquiditySourceConfig {

#[derive(Clone)]
enum LogWriterConfig {
File { log_file_path: Option<String>, log_level: Option<LogLevel> },
Log(LogLevel),
File { log_file_path: Option<String>, max_log_level: Option<LogLevel> },
Log { max_log_level: Option<LogLevel> },
Custom(Arc<dyn LogWriter>),
}

impl std::fmt::Debug for LogWriterConfig {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
LogWriterConfig::File { log_level, log_file_path } => f
LogWriterConfig::File { max_log_level, log_file_path } => f
.debug_struct("LogWriterConfig")
.field("log_level", log_level)
.field("max_log_level", max_log_level)
.field("log_file_path", log_file_path)
.finish(),
LogWriterConfig::Log(level) => f.debug_tuple("Log").field(level).finish(),
LogWriterConfig::Log { max_log_level } => {
f.debug_tuple("Log").field(max_log_level).finish()
},
LogWriterConfig::Custom(_) => {
f.debug_tuple("Custom").field(&"<config internal to custom log writer>").finish()
},
Expand Down Expand Up @@ -331,19 +333,24 @@ impl NodeBuilder {
///
/// The `log_file_path` defaults to [`DEFAULT_LOG_FILENAME`] in the configured
/// [`Config::storage_dir_path`] if set to `None`.
/// The `log_level` defaults to [`DEFAULT_LOG_LEVEL`] if set to `None`.
///
/// If set, the `max_log_level` sets the maximum log level. Otherwise, the latter defaults to
/// [`DEFAULT_LOG_LEVEL`].
///
/// [`DEFAULT_LOG_FILENAME`]: crate::config::DEFAULT_LOG_FILENAME
pub fn set_filesystem_logger(
&mut self, log_file_path: Option<String>, log_level: Option<LogLevel>,
&mut self, log_file_path: Option<String>, max_log_level: Option<LogLevel>,
) -> &mut Self {
self.log_writer_config = Some(LogWriterConfig::File { log_file_path, log_level });
self.log_writer_config = Some(LogWriterConfig::File { log_file_path, max_log_level });
self
}

/// Configures the [`Node`] instance to write logs to the [`log`](https://crates.io/crates/log) facade.
pub fn set_log_facade_logger(&mut self, log_level: LogLevel) -> &mut Self {
self.log_writer_config = Some(LogWriterConfig::Log(log_level));
///
/// If set, the `max_log_level` sets the maximum log level. Otherwise, the latter defaults to
/// [`DEFAULT_LOG_LEVEL`].
pub fn set_log_facade_logger(&mut self, max_log_level: Option<LogLevel>) -> &mut Self {
self.log_writer_config = Some(LogWriterConfig::Log { max_log_level });
self
}

Expand Down Expand Up @@ -658,7 +665,9 @@ impl ArcedNodeBuilder {
///
/// The `log_file_path` defaults to [`DEFAULT_LOG_FILENAME`] in the configured
/// [`Config::storage_dir_path`] if set to `None`.
/// The `log_level` defaults to [`DEFAULT_LOG_LEVEL`] if set to `None`.
///
/// If set, the `max_log_level` sets the maximum log level. Otherwise, the latter defaults to
/// [`DEFAULT_LOG_LEVEL`].
///
/// [`DEFAULT_LOG_FILENAME`]: crate::config::DEFAULT_LOG_FILENAME
pub fn set_filesystem_logger(
Expand All @@ -668,7 +677,10 @@ impl ArcedNodeBuilder {
}

/// Configures the [`Node`] instance to write logs to the [`log`](https://crates.io/crates/log) facade.
pub fn set_log_facade_logger(&self, log_level: LogLevel) {
///
/// If set, the `max_log_level` sets the maximum log level. Otherwise, the latter defaults to
/// [`DEFAULT_LOG_LEVEL`].
pub fn set_log_facade_logger(&self, log_level: Option<LogLevel>) {
self.inner.write().unwrap().set_log_facade_logger(log_level);
}

Expand Down Expand Up @@ -1305,16 +1317,19 @@ fn setup_logger(
log_writer_config: &Option<LogWriterConfig>, config: &Config,
) -> Result<Arc<Logger>, BuildError> {
let logger = match log_writer_config {
Some(LogWriterConfig::File { log_file_path, log_level }) => {
Some(LogWriterConfig::File { log_file_path, max_log_level }) => {
let log_file_path = log_file_path
.clone()
.unwrap_or_else(|| format!("{}/{}", config.storage_dir_path, DEFAULT_LOG_FILENAME));
let log_level = log_level.unwrap_or_else(|| DEFAULT_LOG_LEVEL);
let max_log_level = max_log_level.unwrap_or_else(|| DEFAULT_LOG_LEVEL);

Logger::new_fs_writer(log_file_path, log_level)
Logger::new_fs_writer(log_file_path, max_log_level)
.map_err(|_| BuildError::LoggerSetupFailed)?
},
Some(LogWriterConfig::Log(log_level)) => Logger::new_log_facade(*log_level),
Some(LogWriterConfig::Log { max_log_level }) => {
let max_log_level = max_log_level.unwrap_or_else(|| DEFAULT_LOG_LEVEL);
Logger::new_log_facade(max_log_level)
},

Some(LogWriterConfig::Custom(custom_log_writer)) => {
Logger::new_custom_writer(Arc::clone(&custom_log_writer))
Expand Down
52 changes: 24 additions & 28 deletions src/logger.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,18 @@ pub trait LogWriter: Send + Sync {
/// Defines a writer for [`Logger`].
pub(crate) enum Writer {
/// Writes logs to the file system.
FileWriter { file_path: String, level: LogLevel },
FileWriter { file_path: String, max_log_level: LogLevel },
/// Forwards logs to the `log` facade.
LogFacadeWriter { level: LogLevel },
LogFacadeWriter { max_log_level: LogLevel },
/// Forwards logs to a custom writer.
CustomWriter(Arc<dyn LogWriter>),
}

impl LogWriter for Writer {
fn log(&self, record: LogRecord) {
match self {
Writer::FileWriter { file_path, level } => {
if record.level < *level {
Writer::FileWriter { file_path, max_log_level } => {
if record.level < *max_log_level {
return;
}

Expand All @@ -138,28 +138,24 @@ impl LogWriter for Writer {
.write_all(log.as_bytes())
.expect("Failed to write to log file")
},
Writer::LogFacadeWriter { level } => {
Writer::LogFacadeWriter { max_log_level } => {
if record.level < *max_log_level {
return;
}
macro_rules! log_with_level {
($log_level:expr, $($args:tt)*) => {
($log_level:expr, $target: expr, $($args:tt)*) => {
match $log_level {
LogLevel::Gossip | LogLevel::Trace => trace!($($args)*),
LogLevel::Debug => debug!($($args)*),
LogLevel::Info => info!($($args)*),
LogLevel::Warn => warn!($($args)*),
LogLevel::Error => error!($($args)*),
LogLevel::Gossip | LogLevel::Trace => trace!(target: $target, $($args)*),
LogLevel::Debug => debug!(target: $target, $($args)*),
LogLevel::Info => info!(target: $target, $($args)*),
LogLevel::Warn => warn!(target: $target, $($args)*),
LogLevel::Error => error!(target: $target, $($args)*),
}
};
}

log_with_level!(
level,
"{} {:<5} [{}:{}] {}",
Utc::now().format("%Y-%m-%d %H:%M:%S"),
record.level,
record.module_path,
record.line,
record.args
)
let target = format!("[{}:{}]", record.module_path, record.line);
log_with_level!(record.level, &target, " {}", record.args)
},
Writer::CustomWriter(custom_logger) => custom_logger.log(record),
}
Expand All @@ -174,7 +170,7 @@ pub(crate) struct Logger {
impl Logger {
/// Creates a new logger with a filesystem writer. The parameters to this function
/// are the path to the log file, and the log level.
pub fn new_fs_writer(file_path: String, level: LogLevel) -> Result<Self, ()> {
pub fn new_fs_writer(file_path: String, max_log_level: LogLevel) -> Result<Self, ()> {
if let Some(parent_dir) = Path::new(&file_path).parent() {
fs::create_dir_all(parent_dir)
.map_err(|e| eprintln!("ERROR: Failed to create log parent directory: {}", e))?;
Expand All @@ -187,11 +183,11 @@ impl Logger {
.map_err(|e| eprintln!("ERROR: Failed to open log file: {}", e))?;
}

Ok(Self { writer: Writer::FileWriter { file_path, level } })
Ok(Self { writer: Writer::FileWriter { file_path, max_log_level } })
}

pub fn new_log_facade(level: LogLevel) -> Self {
Self { writer: Writer::LogFacadeWriter { level } }
pub fn new_log_facade(max_log_level: LogLevel) -> Self {
Self { writer: Writer::LogFacadeWriter { max_log_level } }
}

pub fn new_custom_writer(log_writer: Arc<dyn LogWriter>) -> Self {
Expand All @@ -202,14 +198,14 @@ impl Logger {
impl LdkLogger for Logger {
fn log(&self, record: LdkRecord) {
match &self.writer {
Writer::FileWriter { file_path: _, level } => {
if record.level < *level {
Writer::FileWriter { file_path: _, max_log_level } => {
if record.level < *max_log_level {
return;
}
self.writer.log(record.into());
},
Writer::LogFacadeWriter { level } => {
if record.level < *level {
Writer::LogFacadeWriter { max_log_level } => {
if record.level < *max_log_level {
return;
}
self.writer.log(record.into());
Expand Down

0 comments on commit 8bfa8ec

Please sign in to comment.