From 24cf5659c208dc0bc67b633c4b15d174c356af2c Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 31 Oct 2024 17:53:50 +0100 Subject: [PATCH 1/5] revert: removes date from & symlink to log files (PR #116) This commit reverts the changes introduce by PR #116 which added the date to log files when the node is started & a symlink to track the latest log file. This reversal is necessary to simplify the work required to integrate with OS-level log tools. --- src/builder.rs | 2 ++ src/logger.rs | 21 +++------------------ tests/integration_tests_rust.rs | 4 ++-- 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index c14ffcf5a..3a34a8a01 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1231,6 +1231,8 @@ fn build_with_store_internal( }) } +/// Sets up the node logger, creating a new log file if it does not exist, or utilizing +/// the existing log file. fn setup_logger(config: &Config) -> Result, BuildError> { let log_dir = match &config.log_dir_path { Some(log_dir) => String::from(log_dir), diff --git a/src/logger.rs b/src/logger.rs index 19df24367..ea2effb74 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -14,8 +14,6 @@ use chrono::Utc; use std::fs; use std::io::Write; -#[cfg(not(target_os = "windows"))] -use std::os::unix::fs::symlink; use std::path::Path; pub(crate) struct FilesystemLogger { @@ -24,33 +22,20 @@ pub(crate) struct FilesystemLogger { } impl FilesystemLogger { + /// Creates a new filesystem logger given the path to the log file directory and the log level. pub(crate) fn new(log_dir: String, level: Level) -> Result { - let log_file_name = - format!("ldk_node_{}.log", chrono::offset::Local::now().format("%Y_%m_%d")); + let log_file_name = "ldk_node.log"; let log_file_path = format!("{}/{}", log_dir, log_file_name); if let Some(parent_dir) = Path::new(&log_file_path).parent() { fs::create_dir_all(parent_dir).expect("Failed to create log parent directory"); - // make sure the file exists, so that the symlink has something to point to. + // make sure the file exists. fs::OpenOptions::new() .create(true) .append(true) .open(log_file_path.clone()) .map_err(|e| eprintln!("ERROR: Failed to open log file: {}", e))?; - - #[cfg(not(target_os = "windows"))] - { - // Create a symlink to the current log file, with prior cleanup - let log_file_symlink = parent_dir.join("ldk_node_latest.log"); - if log_file_symlink.as_path().is_symlink() { - fs::remove_file(&log_file_symlink).map_err(|e| { - eprintln!("ERROR: Failed to remove log file symlink: {}", e) - })?; - } - symlink(&log_file_name, &log_file_symlink) - .map_err(|e| eprintln!("ERROR: Failed to create log file symlink: {}", e))?; - } } Ok(Self { file_path: log_file_path, level }) diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index dc5c4b818..85174bec7 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -232,8 +232,8 @@ fn start_stop_reinit() { node.sync_wallets().unwrap(); assert_eq!(node.list_balances().spendable_onchain_balance_sats, expected_amount.to_sat()); - let log_file_symlink = format!("{}/logs/ldk_node_latest.log", config.clone().storage_dir_path); - assert!(std::path::Path::new(&log_file_symlink).is_symlink()); + let log_file = format!("{}/logs/ldk_node.log", config.clone().storage_dir_path); + assert!(std::path::Path::new(&log_file).exists()); node.stop().unwrap(); assert_eq!(node.stop(), Err(NodeError::NotRunning)); From dc9f8404679f6492a5694400db7993f2b334b96d Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Sun, 3 Nov 2024 21:14:35 +0100 Subject: [PATCH 2/5] chore: default log file to storage_dir_path/ldk_node.log --- src/builder.rs | 2 +- tests/integration_tests_rust.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 3a34a8a01..b06a1efc9 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1236,7 +1236,7 @@ fn build_with_store_internal( fn setup_logger(config: &Config) -> Result, BuildError> { let log_dir = match &config.log_dir_path { Some(log_dir) => String::from(log_dir), - None => config.storage_dir_path.clone() + "/logs", + None => config.storage_dir_path.clone(), }; Ok(Arc::new( diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 85174bec7..7c6fecd75 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -232,7 +232,7 @@ fn start_stop_reinit() { node.sync_wallets().unwrap(); assert_eq!(node.list_balances().spendable_onchain_balance_sats, expected_amount.to_sat()); - let log_file = format!("{}/logs/ldk_node.log", config.clone().storage_dir_path); + let log_file = format!("{}/ldk_node.log", config.clone().storage_dir_path); assert!(std::path::Path::new(&log_file).exists()); node.stop().unwrap(); From fa071fdbcfb9137915d0d219e5e06b225c68ac3f Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 7 Nov 2024 12:00:07 +0100 Subject: [PATCH 3/5] refactor: replace log directory path to log file path --- bindings/ldk_node.udl | 2 +- src/builder.rs | 16 ++++++++-------- src/config.rs | 6 +++--- src/logger.rs | 9 +++------ 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 6ec7208b2..57eeacdf3 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -5,7 +5,7 @@ namespace ldk_node { dictionary Config { string storage_dir_path; - string? log_dir_path; + string? log_file_path; Network network; sequence? listening_addresses; NodeAlias? node_alias; diff --git a/src/builder.rs b/src/builder.rs index b06a1efc9..dfd86c4f6 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -298,9 +298,9 @@ impl NodeBuilder { self } - /// Sets the log dir path if logs need to live separate from the storage directory path. - pub fn set_log_dir_path(&mut self, log_dir_path: String) -> &mut Self { - self.config.log_dir_path = Some(log_dir_path); + /// Sets the log file path if the log file needs to live separate from the storage directory path. + pub fn set_log_file_path(&mut self, log_dir_path: String) -> &mut Self { + self.config.log_file_path = Some(log_dir_path); self } @@ -611,8 +611,8 @@ impl ArcedNodeBuilder { } /// Sets the log dir path if logs need to live separate from the storage directory path. - pub fn set_log_dir_path(&self, log_dir_path: String) { - self.inner.write().unwrap().set_log_dir_path(log_dir_path); + pub fn set_log_file_path(&self, log_file_path: String) { + self.inner.write().unwrap().set_log_file_path(log_file_path); } /// Sets the Bitcoin network used. @@ -1234,13 +1234,13 @@ fn build_with_store_internal( /// Sets up the node logger, creating a new log file if it does not exist, or utilizing /// the existing log file. fn setup_logger(config: &Config) -> Result, BuildError> { - let log_dir = match &config.log_dir_path { + let log_file_path = match &config.log_file_path { Some(log_dir) => String::from(log_dir), - None => config.storage_dir_path.clone(), + None => format!("{}/{}", config.storage_dir_path.clone(), "ldk_node.log"), }; Ok(Arc::new( - FilesystemLogger::new(log_dir, config.log_level) + FilesystemLogger::new(log_file_path, config.log_level) .map_err(|_| BuildError::LoggerSetupFailed)?, )) } diff --git a/src/config.rs b/src/config.rs index d82b64f32..b026dce01 100644 --- a/src/config.rs +++ b/src/config.rs @@ -105,8 +105,8 @@ pub struct Config { pub storage_dir_path: String, /// The path where logs are stored. /// - /// If set to `None`, logs can be found in the `logs` subdirectory in [`Config::storage_dir_path`]. - pub log_dir_path: Option, + /// If set to `None`, logs can be found in [`Config::storage_dir_path`] directory. + pub log_file_path: Option, /// The used Bitcoin network. pub network: Network, /// The addresses on which the node will listen for incoming connections. @@ -167,7 +167,7 @@ impl Default for Config { fn default() -> Self { Self { storage_dir_path: DEFAULT_STORAGE_DIR_PATH.to_string(), - log_dir_path: None, + log_file_path: None, network: DEFAULT_NETWORK, listening_addresses: None, trusted_peers_0conf: Vec::new(), diff --git a/src/logger.rs b/src/logger.rs index ea2effb74..e58c6b99c 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -22,11 +22,8 @@ pub(crate) struct FilesystemLogger { } impl FilesystemLogger { - /// Creates a new filesystem logger given the path to the log file directory and the log level. - pub(crate) fn new(log_dir: String, level: Level) -> Result { - let log_file_name = "ldk_node.log"; - let log_file_path = format!("{}/{}", log_dir, log_file_name); - + /// Creates a new filesystem logger given the path to the log file and the log level. + pub(crate) fn new(log_file_path: String, level: Level) -> Result { if let Some(parent_dir) = Path::new(&log_file_path).parent() { fs::create_dir_all(parent_dir).expect("Failed to create log parent directory"); @@ -34,7 +31,7 @@ impl FilesystemLogger { fs::OpenOptions::new() .create(true) .append(true) - .open(log_file_path.clone()) + .open(&log_file_path) .map_err(|e| eprintln!("ERROR: Failed to open log file: {}", e))?; } From 8c55876c11df8faa19872faf86ae9bdd690caa12 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 7 Nov 2024 13:13:29 +0100 Subject: [PATCH 4/5] docs: add default log filename to documentation - In addition, update docs for `ArcedNodeBuilder::set_log_file_path` --- src/builder.rs | 2 +- src/config.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index dfd86c4f6..fac2ae0c5 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -610,7 +610,7 @@ impl ArcedNodeBuilder { self.inner.write().unwrap().set_storage_dir_path(storage_dir_path); } - /// Sets the log dir path if logs need to live separate from the storage directory path. + /// Sets the log file path if logs need to live separate from the storage directory path. pub fn set_log_file_path(&self, log_file_path: String) { self.inner.write().unwrap().set_log_file_path(log_file_path); } diff --git a/src/config.rs b/src/config.rs index b026dce01..00b147e21 100644 --- a/src/config.rs +++ b/src/config.rs @@ -105,7 +105,8 @@ pub struct Config { pub storage_dir_path: String, /// The path where logs are stored. /// - /// If set to `None`, logs can be found in [`Config::storage_dir_path`] directory. + /// If set to `None`, logs can be found in `ldk_node.log` in the [`Config::storage_dir_path`] + /// directory. pub log_file_path: Option, /// The used Bitcoin network. pub network: Network, From f033ba200087216b220973f1f470f8b9fdafb2b6 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 7 Nov 2024 13:19:23 +0100 Subject: [PATCH 5/5] refactor: return error when creating logger directory --- src/logger.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/logger.rs b/src/logger.rs index e58c6b99c..bde4faff0 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -25,7 +25,8 @@ impl FilesystemLogger { /// Creates a new filesystem logger given the path to the log file and the log level. pub(crate) fn new(log_file_path: String, level: Level) -> Result { if let Some(parent_dir) = Path::new(&log_file_path).parent() { - fs::create_dir_all(parent_dir).expect("Failed to create log parent directory"); + fs::create_dir_all(parent_dir) + .map_err(|e| eprintln!("ERROR: Failed to create log parent directory: {}", e))?; // make sure the file exists. fs::OpenOptions::new()