Fix log-facade logging and clarify max level

Previously, the log facade logger would always use a `logger` target but
then *also* add the correct module paht and line as part of the normal
log messages. Here, we override the log target correctly.

Moreover, we previously would log all messages at the configured
*maximum* level, not the level in the log record. We therefore also fix
this and rename `level` to `max_log_level` everywhere to clear up any
confusions regarding what the variable does.

Author: tnull

bindings/ldk_node.udl

@@ -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]

src/builder.rs

@@ -111,20 +111,22 @@ impl Default for LiquiditySourceConfig {
 
 #[derive(Clone)]
 enum LogWriterConfig {
-	File { log_file_path: Option<String>, log_level: Option<LogLevel> },
+	File { log_file_path: Option<String>, max_log_level: Option<LogLevel> },
 	Log(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()
 			},
@@ -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
+	/// 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
+	/// defaults to [`DEFAULT_LOG_LEVEL`].
+	pub fn set_log_facade_logger(&mut self, max_log_level: LogLevel) -> &mut Self {
+		self.log_writer_config = Some(LogWriterConfig::Log(max_log_level));
 		self
 	}
 
@@ -1305,13 +1312,13 @@ 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),

src/logger.rs

@@ -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;
 				}
 
@@ -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),
 		}
@@ -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))?;
@@ -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 {
@@ -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());