Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add customizable logging system #401

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ panic = 'abort' # Abort on panic

[features]
default = []
log_relay = ["log"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use any features in LDK Node so far (mostly to keep ~feature-parity of the Rust vs bindings API, for which everything needs to be configurable via the Builder), so please drop this and just take a non-optional dependency on log for now.


[dependencies]
lightning = { version = "0.0.125", features = ["std"] }
Expand Down Expand Up @@ -75,7 +76,7 @@ libc = "0.2"
uniffi = { version = "0.27.3", features = ["build"], optional = true }
serde = { version = "1.0.210", default-features = false, features = ["std", "derive"] }
serde_json = { version = "1.0.128", default-features = false, features = ["std"] }

log = { version = "0.4.22", optional = true}
vss-client = "0.3"
prost = { version = "0.11.6", default-features = false}

Expand Down
80 changes: 44 additions & 36 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
// accordance with one or both of these licenses.

use crate::chain::{ChainSource, DEFAULT_ESPLORA_SERVER_URL};
use crate::config::{default_user_config, Config, EsploraSyncConfig, WALLET_KEYS_SEED_LEN};
use crate::config::{
default_user_config, Config, EsploraSyncConfig, FormatterConfig, LoggerConfig, WriterConfig,
WriterType, WALLET_KEYS_SEED_LEN,
};

use crate::connection::ConnectionManager;
use crate::event::EventQueue;
Expand All @@ -16,7 +19,7 @@ use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{read_node_metrics, write_node_metrics};
use crate::io::vss_store::VssStore;
use crate::liquidity::LiquiditySource;
use crate::logger::{log_error, log_info, FilesystemLogger, Logger};
use crate::logger::{build_formatter, log_error, log_info, LdkNodeLogger, Logger, Writer};
use crate::message_handler::NodeCustomMessageHandler;
use crate::payment::store::PaymentStore;
use crate::peer_store::PeerStore;
Expand All @@ -27,8 +30,8 @@ use crate::types::{
};
use crate::wallet::persist::KVStoreWalletPersister;
use crate::wallet::Wallet;
use crate::Node;
use crate::{io, NodeMetrics};
use crate::{LogLevel, Node};

use lightning::chain::{chainmonitor, BestBlock, Watch};
use lightning::io::Cursor;
Expand Down Expand Up @@ -298,9 +301,15 @@ impl NodeBuilder {
self
}

/// 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);
/// Sets the logger's writer config.
pub fn set_log_writer_config(&mut self, writer_config: WriterConfig) -> &mut Self {
self.config.logger_config.writer = writer_config;
self
}

/// Sets the logger's formatter config.
pub fn set_log_formatter_config(&mut self, formatter_config: FormatterConfig) -> &mut Self {
self.config.logger_config.formatter = formatter_config;
self
}

Expand Down Expand Up @@ -333,12 +342,6 @@ impl NodeBuilder {
Ok(self)
}

/// Sets the level at which [`Node`] will log messages.
pub fn set_log_level(&mut self, level: LogLevel) -> &mut Self {
self.config.log_level = level;
self
}

/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
/// previously configured.
pub fn build(&self) -> Result<Node, BuildError> {
Expand Down Expand Up @@ -391,7 +394,7 @@ impl NodeBuilder {
) -> Result<Node, BuildError> {
use bitcoin::key::Secp256k1;

let logger = setup_logger(&self.config)?;
let logger = setup_logger(&self.config.logger_config)?;

let seed_bytes = seed_bytes_from_config(
&self.config,
Expand Down Expand Up @@ -456,7 +459,7 @@ impl NodeBuilder {
pub fn build_with_vss_store_and_header_provider(
&self, vss_url: String, store_id: String, header_provider: Arc<dyn VssHeaderProvider>,
) -> Result<Node, BuildError> {
let logger = setup_logger(&self.config)?;
let logger = setup_logger(&self.config.logger_config)?;

let seed_bytes = seed_bytes_from_config(
&self.config,
Expand Down Expand Up @@ -488,7 +491,7 @@ impl NodeBuilder {

/// Builds a [`Node`] instance according to the options previously configured.
pub fn build_with_store(&self, kv_store: Arc<DynStore>) -> Result<Node, BuildError> {
let logger = setup_logger(&self.config)?;
let logger = setup_logger(&self.config.logger_config)?;
let seed_bytes = seed_bytes_from_config(
&self.config,
self.entropy_source_config.as_ref(),
Expand Down Expand Up @@ -610,9 +613,14 @@ impl ArcedNodeBuilder {
self.inner.write().unwrap().set_storage_dir_path(storage_dir_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);
/// Sets the logger's writer config.
pub fn set_log_writer_config(&mut self, writer_config: WriterConfig) -> &mut Self {
self.inner.write().unwrap().set_log_writer_config(writer_config);
}

/// Sets the logger's formatter config.
pub fn set_log_formatter_config(&mut self, formatter_config: FormatterConfig) -> &mut Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not quite sure why we'd need separate configurations for the formatter? This is still related to a filesystem writer only, no?

self.inner.write().unwrap().set_log_formatter_config(formatter_config);
}

/// Sets the Bitcoin network used.
Expand All @@ -635,11 +643,6 @@ impl ArcedNodeBuilder {
self.inner.write().unwrap().set_node_alias(node_alias).map(|_| ())
}

/// Sets the level at which [`Node`] will log messages.
pub fn set_log_level(&self, level: LogLevel) {
self.inner.write().unwrap().set_log_level(level);
}

/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
/// previously configured.
pub fn build(&self) -> Result<Arc<Node>, BuildError> {
Expand Down Expand Up @@ -734,7 +737,7 @@ fn build_with_store_internal(
config: Arc<Config>, chain_data_source_config: Option<&ChainDataSourceConfig>,
gossip_source_config: Option<&GossipSourceConfig>,
liquidity_source_config: Option<&LiquiditySourceConfig>, seed_bytes: [u8; 64],
logger: Arc<FilesystemLogger>, kv_store: Arc<DynStore>,
logger: Arc<LdkNodeLogger>, kv_store: Arc<DynStore>,
) -> Result<Node, BuildError> {
// Initialize the status fields.
let is_listening = Arc::new(AtomicBool::new(false));
Expand Down Expand Up @@ -1231,23 +1234,28 @@ 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_file_path = match &config.log_file_path {
Some(log_dir) => String::from(log_dir),
None => format!("{}/{}", config.storage_dir_path.clone(), "ldk_node.log"),
/// Sets up the node logger.
fn setup_logger(config: &LoggerConfig) -> Result<Arc<LdkNodeLogger>, BuildError> {
let level = match &config.writer.writer_type {
WriterType::File(file_writer_config) => file_writer_config.level,
WriterType::LogRelay(log_relay_writer_config) => log_relay_writer_config.level,
WriterType::Custom(custom_writer_config) => custom_writer_config.level,
};

Ok(Arc::new(
FilesystemLogger::new(log_file_path, config.log_level)
.map_err(|_| BuildError::LoggerSetupFailed)?,
))
let writer =
Writer::new(&config.writer.writer_type).map_err(|_e| BuildError::LoggerSetupFailed)?;

let formatter = build_formatter(config.formatter.clone());

let ldk_node_logger =
LdkNodeLogger::new(level, formatter, writer).map_err(|_e| BuildError::LoggerSetupFailed)?;

Ok(Arc::new(ldk_node_logger))
}

fn seed_bytes_from_config(
config: &Config, entropy_source_config: Option<&EntropySourceConfig>,
logger: Arc<FilesystemLogger>,
logger: Arc<LdkNodeLogger>,
) -> Result<[u8; 64], BuildError> {
match entropy_source_config {
Some(EntropySourceConfig::SeedBytes(bytes)) => Ok(bytes.clone()),
Expand All @@ -1269,7 +1277,7 @@ fn seed_bytes_from_config(
}

fn derive_vss_xprv(
config: Arc<Config>, seed_bytes: &[u8; 64], logger: Arc<FilesystemLogger>,
config: Arc<Config>, seed_bytes: &[u8; 64], logger: Arc<LdkNodeLogger>,
) -> Result<Xpriv, BuildError> {
use bitcoin::key::Secp256k1;

Expand Down
14 changes: 7 additions & 7 deletions src/chain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::fee_estimator::{
ConfirmationTarget, OnchainFeeEstimator,
};
use crate::io::utils::write_node_metrics;
use crate::logger::{log_bytes, log_error, log_info, log_trace, FilesystemLogger, Logger};
use crate::logger::{log_bytes, log_error, log_info, log_trace, LdkNodeLogger, Logger};
use crate::types::{Broadcaster, ChainMonitor, ChannelManager, DynStore, Sweeper, Wallet};
use crate::{Error, NodeMetrics};

Expand Down Expand Up @@ -112,13 +112,13 @@ pub(crate) enum ChainSource {
esplora_client: EsploraAsyncClient,
onchain_wallet: Arc<Wallet>,
onchain_wallet_sync_status: Mutex<WalletSyncStatus>,
tx_sync: Arc<EsploraSyncClient<Arc<FilesystemLogger>>>,
tx_sync: Arc<EsploraSyncClient<Arc<LdkNodeLogger>>>,
lightning_wallet_sync_status: Mutex<WalletSyncStatus>,
fee_estimator: Arc<OnchainFeeEstimator>,
tx_broadcaster: Arc<Broadcaster>,
kv_store: Arc<DynStore>,
config: Arc<Config>,
logger: Arc<FilesystemLogger>,
logger: Arc<LdkNodeLogger>,
node_metrics: Arc<RwLock<NodeMetrics>>,
},
BitcoindRpc {
Expand All @@ -131,7 +131,7 @@ pub(crate) enum ChainSource {
tx_broadcaster: Arc<Broadcaster>,
kv_store: Arc<DynStore>,
config: Arc<Config>,
logger: Arc<FilesystemLogger>,
logger: Arc<LdkNodeLogger>,
node_metrics: Arc<RwLock<NodeMetrics>>,
},
}
Expand All @@ -140,7 +140,7 @@ impl ChainSource {
pub(crate) fn new_esplora(
server_url: String, sync_config: EsploraSyncConfig, onchain_wallet: Arc<Wallet>,
fee_estimator: Arc<OnchainFeeEstimator>, tx_broadcaster: Arc<Broadcaster>,
kv_store: Arc<DynStore>, config: Arc<Config>, logger: Arc<FilesystemLogger>,
kv_store: Arc<DynStore>, config: Arc<Config>, logger: Arc<LdkNodeLogger>,
node_metrics: Arc<RwLock<NodeMetrics>>,
) -> Self {
let mut client_builder = esplora_client::Builder::new(&server_url);
Expand Down Expand Up @@ -170,7 +170,7 @@ impl ChainSource {
host: String, port: u16, rpc_user: String, rpc_password: String,
onchain_wallet: Arc<Wallet>, fee_estimator: Arc<OnchainFeeEstimator>,
tx_broadcaster: Arc<Broadcaster>, kv_store: Arc<DynStore>, config: Arc<Config>,
logger: Arc<FilesystemLogger>, node_metrics: Arc<RwLock<NodeMetrics>>,
logger: Arc<LdkNodeLogger>, node_metrics: Arc<RwLock<NodeMetrics>>,
) -> Self {
let bitcoind_rpc_client =
Arc::new(BitcoindRpcClient::new(host, port, rpc_user, rpc_password));
Expand Down Expand Up @@ -1123,7 +1123,7 @@ impl Filter for ChainSource {

fn periodically_archive_fully_resolved_monitors(
channel_manager: Arc<ChannelManager>, chain_monitor: Arc<ChainMonitor>,
kv_store: Arc<DynStore>, logger: Arc<FilesystemLogger>, node_metrics: Arc<RwLock<NodeMetrics>>,
kv_store: Arc<DynStore>, logger: Arc<LdkNodeLogger>, node_metrics: Arc<RwLock<NodeMetrics>>,
) -> Result<(), Error> {
let mut locked_node_metrics = node_metrics.write().unwrap();
let cur_height = channel_manager.current_best_block().height;
Expand Down
Loading
Loading