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

cln-plugin: Make logging optional #6797

Merged

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Oct 19, 2023

Under some circumstances we may want to not log to lightningd directly, but rather configure the logging ourselves. This is useful for example if we want to use tracing and tracing-subscriber to add custom handling, or add opentelemetry span tracing.

Changelog-Changed: cln-plugin: Suppress internal logging handler via with_logging(false)

Under some circumstances we may want to not log to `lightningd`
directly, but rather configure the logging ourselves. This is useful
for example if we want to use `tracing` and `tracing-subscriber` to
add custom handling, or add opentelemetry span tracing.

Changelog-Changed: cln-plugin: Suppress internal logging handler via `with_logging(false)`
@cdecker cdecker marked this pull request as ready for review October 25, 2023 15:04
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 9fa63c1

There is a use case where we can not use a feature flag? or it is just a matter of preference?

@vincenzopalazzo vincenzopalazzo merged commit ee8fe11 into ElementsProject:master Oct 26, 2023
38 checks passed
@cdecker
Copy link
Member Author

cdecker commented Oct 26, 2023

The reason to pass in a boolean is that the API already out there automatically starts the logging, and so the deviation from the existing semantics is to disable logging. This would result in a rather confusing without_logging() rather than with_logging(bool) which I think is a bit more ergonomic and simpler to remember.

@cdecker cdecker deleted the 20231019-rs-plugin-opt-loggign branch October 26, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants