Skip to content

Add configurable log level and log file#76

Merged
tnull merged 1 commit into
lightningdevkit:mainfrom
benthecarman:log
Dec 9, 2025
Merged

Add configurable log level and log file#76
tnull merged 1 commit into
lightningdevkit:mainfrom
benthecarman:log

Conversation

@benthecarman

@benthecarman benthecarman commented Dec 3, 2025

Copy link
Copy Markdown
Collaborator

Before we were dropping all our logs on the floor besides a few printlns
in the code. This implements a logger for the log facade that writes
logs to the console as well as to the log file. We also add to the config
options for setting the log file and log level.

@ldk-reviews-bot

ldk-reviews-bot commented Dec 3, 2025

Copy link
Copy Markdown

I've assigned @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Comment thread ldk-server/src/main.rs Outdated
Comment thread ldk-server/src/main.rs Outdated
.set_time_format_rfc3339() // set time format to rfc3339 (e.g. 2025-12-03T21:44:52.169154084Z)
.build();
if let Err(e) = CombinedLogger::init(vec![
SimpleLogger::new(config_file.log_level, log_config.clone()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wasn't really clear from the docs as to why we need this. Does it got to stdout?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yeah, added a comment

Comment thread ldk-server/src/main.rs
Comment thread ldk-server/Cargo.toml Outdated
rand = { version = "0.8.5", default-features = false }
async-trait = { version = "0.1.85", default-features = false }
toml = { version = "0.8.9", default-features = false, features = ["parse"] }
simplelog = { version = "0.12.0", default-features = false }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rather than pulling in that depedency, can we just add a simple logger that implements the log facade? It's really trivial, for reference see the MockLogFacadeLogger in LDK Node tests: https://github.com/lightningdevkit/ldk-node/blob/198ff306bb70d1b6eb7bf9a4213716e22ae1db90/tests/common/logging.rs#L22

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done!

Comment thread ldk-server/src/util/logger.rs Outdated
/// The minimum log level to display
level: LevelFilter,
/// The file to write logs to, buffered and protected by a mutex for thread-safe access
file: Mutex<BufWriter<File>>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Keeping the open file handle in a Mutex works, but then we should handle SIGHUP and make sure we drop and reopen the file handle to ensure compatibility with system tools such as logrotate.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added handling for this, made things a little more complicated but nothing too bad

Comment thread ldk-server/src/util/logger.rs Outdated
Comment thread ldk-server/src/util/logger.rs
Comment thread ldk-server/src/util/logger.rs Outdated
@benthecarman benthecarman force-pushed the log branch 2 times, most recently from 320676e to b345b27 Compare December 8, 2025 05:10

@tnull tnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Basically LGTM, just two minor comments.

Comment thread ldk-server/src/main.rs Outdated
Comment thread ldk-server/src/util/logger.rs Outdated
Before we were dropping all our logs on the floor besides a few printlns
in the code. This implements a logger for the log facade that writes
logs to the console as well as to the log file. We also add to the config
options for setting the log file and log level.
@tnull tnull merged commit e389521 into lightningdevkit:main Dec 9, 2025
7 checks passed
@benthecarman benthecarman deleted the log branch December 9, 2025 08:35
rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Add configurable log level and log file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants