Skip to content

Add structured logging context fields to LogRecord #751

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.

Sign up for GitHub

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

Merged
tnull merged 1 commit into from
Jan 13, 2026
+219 −8
Merged

Add structured logging context fields to LogRecord #751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 3 additions & 0 deletions bindings/ldk_node.udl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ dictionary LogRecord {
string args;
string module_path;
u32 line;
PublicKey? peer_id;
ChannelId? channel_id;
PaymentHash? payment_hash;
};

[Trait, WithForeign]
Expand Down
213 changes: 208 additions & 5 deletions src/logger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@

//! Logging-related objects.

#[cfg(not(feature = "uniffi"))]
use core::fmt;
use std::fs;
use std::io::Write;
use std::path::Path;
use std::sync::Arc;

use bitcoin::secp256k1::PublicKey;
use chrono::Utc;
use lightning::ln::types::ChannelId;
use lightning::types::payment::PaymentHash;
pub use lightning::util::logger::Level as LogLevel;
pub(crate) use lightning::util::logger::{Logger as LdkLogger, Record as LdkRecord};
pub(crate) use lightning::{log_bytes, log_debug, log_error, log_info, log_trace};
Expand All @@ -32,6 +34,64 @@ pub struct LogRecord<'a> {
pub module_path: &'a str,
/// The line containing the message.
pub line: u32,
/// The node id of the peer pertaining to the logged record.
pub peer_id: Option<PublicKey>,
/// The channel id of the channel pertaining to the logged record.
pub channel_id: Option<ChannelId>,
/// The payment hash pertaining to the logged record.
pub payment_hash: Option<PaymentHash>,
}

/// Structured context fields for log messages.
///
/// Implements `Display` to format context fields (channel_id, peer_id, payment_hash) directly
/// into a formatter, avoiding intermediate heap allocations when used with `format_args!` or
/// `write!` macros.
///
/// Note: LDK's `Record` Display implementation uses fixed-width padded columns and different
/// formatting for test vs production builds. We intentionally use a simpler format here:
/// fields are only included when present (no padding), and the format is consistent across
/// all build configurations.
pub struct LogContext<'a> {
Copy link
Collaborator

@tnull tnull Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Why does this need to be pub? Seems it's an internal utility only? When you drop it, please also drop the redundant doc comments here and on the Display impl.

Copy link
Contributor Author

@joostjager joostjager Jan 13, 2026

Choose a reason for hiding this comment

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

For ldk-node users that have their own logger implementation, but also want to reuse the context fmting?

Copy link
Collaborator

@tnull tnull Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

For ldk-node users that have their own logger implementation, but also want to reuse the context fmting?

Hmm, so do you plan to use it for LDK Server? If not, I'd say we can make it private as other users implementation might look differently. If we want to keep it pub for reuse in Server, let's still drop the docs on impl Display and also the entire pub fn new() which is just unnecessary (and to a degree an anti-pattern) if you have only pub struct fields anyways.

Copy link
Contributor Author

@joostjager joostjager Jan 13, 2026

Choose a reason for hiding this comment

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

Yes, plan to use in server. Dropped new and moved Display docs.

/// The channel id of the channel pertaining to the logged record.
pub channel_id: Option<&'a ChannelId>,
/// The node id of the peer pertaining to the logged record.
pub peer_id: Option<&'a PublicKey>,
/// The payment hash pertaining to the logged record.
pub payment_hash: Option<&'a PaymentHash>,
}

impl fmt::Display for LogContext<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fn truncate(s: &str) -> &str {
&s[..s.len().min(6)]
}

if self.channel_id.is_none() && self.peer_id.is_none() && self.payment_hash.is_none() {
return Ok(());
}

write!(f, " (")?;
let mut need_space = false;
if let Some(c) = self.channel_id {
write!(f, "ch:{}", truncate(&c.to_string()))?;
need_space = true;
}
if let Some(p) = self.peer_id {
if need_space {
write!(f, " ")?;
}
write!(f, "p:{}", truncate(&p.to_string()))?;
need_space = true;
}
if let Some(h) = self.payment_hash {
if need_space {
write!(f, " ")?;
}
write!(f, "h:{}", truncate(&format!("{:?}", h)))?;
}
write!(f, ")")
}
}

/// A unit of logging output with metadata to enable filtering `module_path`,
Expand All @@ -50,6 +110,12 @@ pub struct LogRecord {
pub module_path: String,
/// The line containing the message.
pub line: u32,
/// The node id of the peer pertaining to the logged record.
pub peer_id: Option<PublicKey>,
/// The channel id of the channel pertaining to the logged record.
pub channel_id: Option<ChannelId>,
/// The payment hash pertaining to the logged record.
pub payment_hash: Option<PaymentHash>,
}

#[cfg(feature = "uniffi")]
Expand All @@ -60,6 +126,9 @@ impl<'a> From<LdkRecord<'a>> for LogRecord {
args: record.args.to_string(),
module_path: record.module_path.to_string(),
line: record.line,
peer_id: record.peer_id,
channel_id: record.channel_id,
payment_hash: record.payment_hash,
}
}
}
Expand All @@ -72,6 +141,9 @@ impl<'a> From<LdkRecord<'a>> for LogRecord<'a> {
args: record.args,
module_path: record.module_path,
line: record.line,
peer_id: record.peer_id,
channel_id: record.channel_id,
payment_hash: record.payment_hash,
}
}
}
Expand Down Expand Up @@ -113,19 +185,26 @@ pub(crate) enum Writer {

impl LogWriter for Writer {
fn log(&self, record: LogRecord) {
let context = LogContext {
channel_id: record.channel_id.as_ref(),
peer_id: record.peer_id.as_ref(),
payment_hash: record.payment_hash.as_ref(),
};

match self {
Writer::FileWriter { file_path, max_log_level } => {
if record.level < *max_log_level {
return;
}

let log = format!(
"{} {:<5} [{}:{}] {}\n",
"{} {:<5} [{}:{}] {}{}\n",
Copy link
Collaborator

@tnull tnull Jan 12, 2026

Choose a reason for hiding this comment

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

Just appending it plain looks a bit odd for some messages:

Persistence of ChannelMonitorUpdate id 13 completed ch:d6487d p:02d87e

Can we maybe wrap the context in parentheses or brackets?

Copy link
Contributor Author

@joostjager joostjager Jan 13, 2026

Choose a reason for hiding this comment

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

Parentheses added. Also made the implementation a bit more efficient by avoiding some heap allocs.

Utc::now().format("%Y-%m-%d %H:%M:%S%.3f"),
record.level.to_string(),
record.module_path,
record.line,
record.args
record.args,
context,
);

fs::OpenOptions::new()
Expand Down Expand Up @@ -153,7 +232,7 @@ impl LogWriter for Writer {
.target(record.module_path)
.module_path(Some(record.module_path))
.line(Some(record.line))
.args(format_args!("{}", record.args))
.args(format_args!("{}{}", record.args, context))
.build(),
);
#[cfg(feature = "uniffi")]
Expand All @@ -162,7 +241,7 @@ impl LogWriter for Writer {
.target(&record.module_path)
.module_path(Some(&record.module_path))
.line(Some(record.line))
.args(format_args!("{}", record.args))
.args(format_args!("{}{}", record.args, context))
.build(),
);
},
Expand Down Expand Up @@ -222,3 +301,127 @@ impl LdkLogger for Logger {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use std::sync::Mutex;

/// A minimal log facade logger that captures log output for testing.
struct TestLogger {
log: Arc<Mutex<String>>,
}

impl log::Log for TestLogger {
fn enabled(&self, _metadata: &log::Metadata) -> bool {
true
}

fn log(&self, record: &log::Record) {
*self.log.lock().unwrap() = record.args().to_string();
}

fn flush(&self) {}
}

/// Tests that LogContext correctly formats all three structured fields
/// (channel_id, peer_id, payment_hash) with space prefixes and 6-char truncation.
#[test]
fn test_log_context_all_fields() {
let channel_id = ChannelId::from_bytes([
0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
]);
let peer_id = PublicKey::from_slice(&[
0x02, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf1, 0x23, 0x45,
0x67, 0x89, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf1, 0x23,
0x45, 0x67, 0x89, 0xab, 0xcd,
])
.unwrap();
let payment_hash = PaymentHash([
0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
]);

let context = LogContext {
channel_id: Some(&channel_id),
peer_id: Some(&peer_id),
payment_hash: Some(&payment_hash),
};

assert_eq!(context.to_string(), " (ch:abcdef p:02abcd h:fedcba)");
}

/// Tests that LogContext returns an empty string when no fields are provided.
#[test]
fn test_log_context_no_fields() {
let context = LogContext { channel_id: None, peer_id: None, payment_hash: None };
assert_eq!(context.to_string(), "");
}

/// Tests that LogContext only includes present fields.
#[test]
fn test_log_context_partial_fields() {
let channel_id = ChannelId::from_bytes([
0x12, 0x34, 0x56, 0x78, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
]);

let context =
LogContext { channel_id: Some(&channel_id), peer_id: None, payment_hash: None };
assert_eq!(context.to_string(), " (ch:123456)");
}

/// Tests that LogFacadeWriter appends structured context fields to the log message.
#[test]
fn test_log_facade_writer_includes_structured_context() {
let log = Arc::new(Mutex::new(String::new()));
let test_logger = TestLogger { log: log.clone() };

let _ = log::set_boxed_logger(Box::new(test_logger));
log::set_max_level(log::LevelFilter::Trace);

let writer = Writer::LogFacadeWriter;

let channel_id = ChannelId::from_bytes([
0xab, 0xcd, 0xef, 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
]);
let peer_id = PublicKey::from_slice(&[
0x02, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf1, 0x23, 0x45,
0x67, 0x89, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc, 0xde, 0xf1, 0x23,
0x45, 0x67, 0x89, 0xab, 0xcd,
])
.unwrap();

#[cfg(not(feature = "uniffi"))]
let record = LogRecord {
level: LogLevel::Info,
args: format_args!("Test message"),
module_path: "test_module",
line: 42,
peer_id: Some(peer_id),
channel_id: Some(channel_id),
payment_hash: None,
};

#[cfg(feature = "uniffi")]
let record = LogRecord {
level: LogLevel::Info,
args: "Test message".to_string(),
module_path: "test_module".to_string(),
line: 42,
peer_id: Some(peer_id),
channel_id: Some(channel_id),
payment_hash: None,
};

writer.log(record);

assert_eq!(*log.lock().unwrap(), "Test message (ch:abcdef p:02abcd)");
}
}
11 changes: 8 additions & 3 deletions tests/common/logging.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::sync::{Arc, Mutex};

use chrono::Utc;
use ldk_node::logger::{LogLevel, LogRecord, LogWriter};
use ldk_node::logger::{LogContext, LogLevel, LogRecord, LogWriter};
#[cfg(not(feature = "uniffi"))]
use log::Record as LogFacadeRecord;
use log::{Level as LogFacadeLevel, LevelFilter as LogFacadeLevelFilter, Log as LogFacadeLog};
Expand Down Expand Up @@ -156,13 +156,18 @@ impl MultiNodeLogger {
impl LogWriter for MultiNodeLogger {
fn log(&self, record: LogRecord) {
let log = format!(
"[{}] {} {:<5} [{}:{}] {}\n",
"[{}] {} {:<5} [{}:{}] {}{}\n",
self.node_id,
Utc::now().format("%Y-%m-%d %H:%M:%S%.3f"),
record.level.to_string(),
record.module_path,
record.line,
record.args
record.args,
LogContext {
channel_id: record.channel_id.as_ref(),
peer_id: record.peer_id.as_ref(),
payment_hash: record.payment_hash.as_ref(),
},
);

print!("{}", log);
Expand Down
Loading