Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace ldk_node {

dictionary Config {
string storage_dir_path;
string? log_dir_path;
string? log_file_path;
Network network;
sequence<SocketAddress>? listening_addresses;
NodeAlias? node_alias;
Expand Down
18 changes: 10 additions & 8 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1231,14 +1231,16 @@ 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<Arc<FilesystemLogger>, 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),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github won't comment on the line below this, but let's drop the {storage_path_dir}/logs folder now that we'll only have one log file. Rather, let's just default to {storage_path_dir}/ldk_node.log.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This variable names should be adjusted to reflect it's now a file path, not the dir path anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for this. I'll address in another PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries at all, let's just include a fixup commit in whatever logging-related PR you open next!

None => config.storage_dir_path.clone() + "/logs",
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)?,
))
}
Expand Down
6 changes: 3 additions & 3 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
/// If set to `None`, logs can be found in [`Config::storage_dir_path`] directory.
pub log_file_path: Option<String>,
/// The used Bitcoin network.
pub network: Network,
/// The addresses on which the node will listen for incoming connections.
Expand Down Expand Up @@ -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(),
Expand Down
26 changes: 4 additions & 22 deletions src/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -24,33 +22,17 @@ pub(crate) struct FilesystemLogger {
}

impl FilesystemLogger {
pub(crate) fn new(log_dir: String, level: Level) -> Result<Self, ()> {
let log_file_name =
format!("ldk_node_{}.log", chrono::offset::Local::now().format("%Y_%m_%d"));
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<Self, ()> {
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())
.open(&log_file_path)
.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 })
Expand Down
4 changes: 2 additions & 2 deletions tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!("{}/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));
Expand Down