#29256 log, refactor: Allow log macros to accept context arguments
https://github.com/bitcoin/bitcoin/pull/29256 · · +417/-114 in 6 files, 10 commits · labels: Utils/log/libs · draft
Goal
- Allow logging macros to accept context arguments instead of requiring a global singleton
- Enable callers to prepend metadata like wallet names or route logs to isolated streams
- Help library consumers like libbitcoinkernel separate logging instances
This pull request refactors the logging macro infrastructure (`LogTrace`, `LogDebug`, `LogInfo`, `LogWarning`, `LogError`) to optionally accept context arguments (`util::log::Context`). This allows callers to pass specific logger instances or formatting hooks to prepend metadata like wallet names or request IDs, and avoids mandatory reliance on the global `BCLog::Logger` singleton.
Problem: Current logging macros assume a single global logging instance, preventing callers such as `libbitcoinkernel` or multi-wallet setups from redirecting log output to isolated streams or automatically prepending contextual identifiers without dedicated wrapper functions.
Category: Utilities (logging, arguments, libraries) (#46 of 66)
P3 · unblocks #30342
- P3 because it lets macros attach context cleanly without separate macro implementations per subsystem
- Unblocks instanced kernel logging in #30342 and wallet logging refactoring in #30343
- Does not fix an active bug or performance issue in bitcoind
P3 because it provides a clean mechanism to attach context and custom loggers to log macros without requiring separate macro implementations for subsystems. While it unblocks #30342 (passing logger instances to kernel objects) and #30343 (wallet logging refactor), it does not fix an active bug or performance issue in bitcoind.
Membership: Modifies shared logging infrastructure in src/util/log.h and src/logging.cpp.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 2
Reviewability: Stale: Author silent 113d
- Review #34778 first
- Author inactive for over 100 days
The author has been inactive for 113 days, exceeding the 60-day project staleness threshold. Additionally, the PR is based on open PR 34778.
Author status: silent since 2026-05-27
Open concerns:
- ajtowns holds an active Concept NACK, arguing that instanced logging for kernel is an unnecessary architectural complication that could be handled via separate processes, and that adding context arguments adds code churn and complexity across call sites without commensurate benefit
- hodlinator raised concerns about maintaining output behavior and preventing category arguments at higher severity levels, which the author partially addressed with restrictions and static asserts
- l0rinc requested splitting up large commits and questioned several template abstractions
Resolved concerns:
- Restrictions from #28318 forbidding category arguments at Info/Warning/Error and requiring them at Debug/Trace were preserved to reach compromise
- Dropped initial controversial changes that removed category restrictions across log levels
Agreement: Disputed
- Concept objection: instanced logging is unnecessary complexity and churn (ajtowns)
- Concept approval: useful for distinguishing concurrent outputs like multiple chainstates (sedited)
- Concept approval: supports simpler and more consistent logging APIs across the project (jonatack)
- Requested changes regarding output behavior and category filtering enforcement (hodlinator)
Disputed: ajtowns maintains a Concept NACK against instanced logger contexts; author and supporters disagree.
ajtowns has registered a blocking Concept NACK against modifying logging macros to support non-global contexts and instanced loggers, viewing it as a design error for kernel and unnecessary churn for wallet. The author and reviewers sedited and jonatack maintain that multi-instance logging is valuable for external library consumers.
- ajtowns Concept NACK: 'This PR isn't solving any problems other than its author's aesthetic objection to the current logging API... adding multiple logger instances to the bitcoinkernel project still seems to be a massive design error to me'
- sedited Concept ACK: 'There are use cases for having e.g. two chainstate managers running in parallel... Having something to distinguishing logs issued between the two sounds useful'
- jonatack Concept ACK: supportive of simpler, consistent logging APIs across the project
- hodlinator reviewed and requested changes regarding output behavior and category filtering enforcement
Review verdicts (DrahtBot): 0 -2
- Concept ACK: sedited, jonatack
- Concept NACK: hodlinator, ajtowns
Dependencies
Depends on: #34778
Enables:
Based on (shares commits with): #34778
Base for: #30342, #30343
Files
228 lines under test/bench/ci.
- src/util/log.h +185/-62
- src/test/logging_tests.cpp +207/-21
- src/logging.cpp +18/-24
- src/logging.h +3/-3
- src/validation.cpp +3/-2
- doc/developer-notes.md +1/-2
Card
PR 29256 generalizes the core logging macros (LogInfo, LogDebug, etc.) to optionally accept context parameters containing logger pointers and formatting hooks. This enables downstream work in libbitcoinkernel (#30342) to isolate log streams and in the wallet (#30343) to embed wallet names without custom wrapper functions. The change is conceptually disputed, with ajtowns maintaining a Concept NACK against non-global logging contexts in kernel while others support it. The PR is currently marked draft, is stacked on #34778, and has seen no author activity for over 100 days.