kernel: make logging struct-based#34466
Closed
stickies-v wants to merge 10 commits intobitcoin:masterfrom
Closed
Conversation
This is preparatory work for a future commit so SourceLocation can be used without requiring logging.h.
This is preparatory work for a future commit so Level can be used without requiring logging.h.
Add a minimal, callback-based dispatcher that forwards struct-based log entries to registered callbacks. This provides the foundation for separating log generation from log consumption, enabling different consumers (node sink, kernel C API) to receive the same log entries.
Preparatory work for a future commit where the logging macros are moved to the util library. The macros require a single global entry point to dispatch log entries. Make BCLog::Logger aware of util::log::Dispatcher by giving it ownership of a Dispatcher instance which it can register callbacks on. Currently, only a single sink (i.e. BCLog::Logger) exists, but in future commits this will change as kernel removes its dependency on logging.cpp and introduces its own global logging sink.
Log macros now call util::log::g_dispatcher().Log() directly instead of going through LogInstance(). BCLog::Logger registers a callback on its Dispatcher to receive entries and handle formatting, rate limiting, and output.
Expose btck_log_level_get_name and btck_log_category_get_name to allow library users to get string representations of log levels and categories. Use constexpr lookup tables as a single source of truth for mapping btck types to both BCLog equivalents and string names.
Ensure all levels and categories that may be logged by kernel code are well defined. Necessary preparation for future work to move from string-based to struct-based logging.
Preparatory work for a future commit where the btck_LogCallback exposes btck_LogEntry instead of a string. Contains a lot of move-only changes, consider reviewing with --color-moved=dimmed-zebra
Replace the string-based logging callback with btck_LogEntry containing full metadata (message, source location, timestamp, level, category). This lets consumers format and filter logs as needed. Use the global log dispatcher instead of BCLog::Logger's buffered callback system. Messages are discarded when no callbacks are registered.
As logging is struct-based, there is no more need for the user to configure how logs are printed, so simplify the interface and remove dead code.
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. |
Contributor
Author
|
Didn't see @ryanofsky already opened #34465 just before this, I'll close this for now since there's a lot of duplication. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
bitcoinkernellibrary (#27587) exposes functionality to interface with kernel logging. This includes registering callbacks for log statements, level/category filtering, string formatting options, and more.This PR improves kernel logging by making it struct-based instead of string-based. Rather than adding this functionality onto an already large
BCLog::Loggerclass, the PR first separates log generation (i.e. producing the log message viaLogInfo(msg)) from log consumption (i.e. formatting, ratelimiting, printing the message) by moving all log generation code toutil/log.h. While not strictly necessary for adding struct-based logging, it better separates concerns and is a prerequisite for follow-up work in #34374 that completely removeskernel's dependency onlogging.cpp.Approach:
Separate log generation: Add a minimal
util::log::Dispatcherthat is used to generate logs, and forwards struct-based log entries to registered callbacks. No formatting, rate-limiting, or output handling - that's the consumer's responsibility.Register
BCLog::Loggeron Dispatcher: Register a callback on the global Dispatcher to receive entries and handle node-specific concerns (formatting, rate-limiting, file output) without changing most of its logic. Route logging macros through this global dispatcher.Update bitcoinkernel C API: Replace the string-based callback with
btck_LogEntrycontaining full metadata and removebtck_LoggingOptionsand related functions.Note: with this change,
bitcoinkernellogging callbacks are no longer routed throughBCLog::Logger. This means that they are no longer buffered (i.e. log messages produced before a logging callback is registered) are dropped, and there is no longer a need forbtck_logging_disable. In practice, this means users can no longer see the log message produced during the static context initialization.Carve-out of #34374 to narrow the focus while maintaining tangible benefits (i.e. having struct-based logging).