Expose logger interface

[tnull]

We should allow users to implement custom loggers, e.g., to implement their own rotation logic or to target a database backend.

[tnull]

This is probably slipping to 0.5, but we should still modify our logger to make it compatible with logrotate.

[enigbe]

Hi @tnull

Can you provide a bit more context about targeting a database backend? My initial thought is that the custom logger can save generated logs to any database server that can handle key(timestamp)-value(log) objects but I am not sure about the correctness of the data model and if kv storage is considered appropriate.

Additionally, the strategy I have thought to address this issue is to:

1. Create a LoggerBuilder that builds a CustomLogger which implements Logger,

2. Furnish the builder with methods to:

   1. configure a database backend,
   2. configure logrotate rotation schedule and/or configuration file(s),
   3. etc...,

3. Default to the existing CustomLogger::FilesystemLogger if custom build options are not configured,

4. Possibly build logger from config file,

5. Test against a database server.

enum CustomLogger {
    FilesystemLogger,
    DatabaseLogger
}

Please let me know if you think this approach is sound or needs refinement, and if I can start working on an implementation. Happy to get any feedback/recommendations from you.

[G8XSU]

Afaiu,
We don't need a database logger or to implement one.
We only need support to build node with logging interface instead of directly logging to a file(ldk_node_logs).

Also we should explore log crate as part of this.

[enigbe]

Thanks for the responding @G8XSU
I'll start work on this today.

[tnull]

Thanks for the responding @G8XSU I'll start work on this today.

Excuse the delayed response here. Great to hear you're interested in picking this up!

I think the first steps would be:

1. Open PR to drop the current insufficient FilesystemLogger log rotation logic we have and rather than writing multiple log files and the symlink in the dictionary revert back to simply writing a simple ldk_node.log file in the storage dir path (possibly still keep the dir path configurable though). This would mostly be reverting Add the date when the node was started to the name of the logfile.  #116 and is technically a feature regression, but it would simplify the logic on our end and makes integration with OS-level tooling such as logrotate trivial (as we currently open the file on every write, we don't even have to do anything. If we were to keep the file open we'd need to catch SIGHUP and re-open the file).

2. Explore if/how we can take the log facade as an optional dependency in rust-lighting. This would be great as log is the defacto standard in the Rust ecosystem and finally have LDK (not only LDK Node) integrate with others projects in this regard would be a big win, IMO. We however need to see whether we can accommodate all the custom logging macros through log. If we can, we should open a PR to that extend upstream, otherwise we need to find a way how to adapt LDK's Logger trait here based on log.

3. Possibly still expose a CustomLogger trait that would be usable in bindings, as we know that some of our bindings users would be interested in deploying custom loggers. The approach for this depends on how 2) pans out. If we can't use log properly, we might end up with a trait-based adaptor anyways that we can just make configurable via the Builder.

I hope all of these make sense to you, let me know (here or on Discord) if you have any more questions.

[enigbe]

Thanks for providing additional context @tnull. I understand the steps you have outlined and will contact you if I have any questions.

[tnull]

Recently some LDK devs had another discussion regarding the customized logging interface in LDK Node, as more and more users request this feature. Given that it might take some time to get the optional dependency added to rust-lightning and ship a release, we concluded that it might make sense to start with a custom interface that optionally adapts log in LDK Node first, and then we can see what parts of it can be upstreamed.

So essentially we would start off with a refactor to the one attempted in #393, but with three writer variants: a) a FileWriter which is essentially our current filesystemlogger, b) a writer that would forward to log and c) a custom writer taking an Arc<dyn LogWriter + Send + Sync> where LogWriter would be a new bindings-compatible log-writer trait.

In contrast to the approach taken in #393 this however needs to be configurable via Builder methods (potentially also taking , e.g, a specific FileWriterConfig struct as an argument), rather than adding all this to to the main Config itself, as the latter isn't bindings compatible.

Let me know if that makes sense to you and if you have any further questions! Also happy to pick some of this up myself if you prefer.

(cc @G8XSU)

[enigbe]

Thanks for the update. Switching here now.