#33847 kernel: Improve logging API
https://github.com/bitcoin/bitcoin/pull/33847 · · +118/-108 in 5 files, 1 commits · labels: Validation, Needs rebase · draft
Goal
- Make libbitcoinkernel logging safer by configuring loggers directly through context options
- Eliminate an unsafe global disable function and stop buffering logs in memory by default
This PR refactors the libbitcoinkernel C API for logging by passing a btck_LoggingConnection handle to logging option functions and allowing loggers to be associated directly with btck_ContextOptions. It also removes btck_logging_disable() and stops buffering up to 1MB of log messages internally by default when no log stream is attached.
Problem: The initial kernel logging C API exposes global configuration functions separate from kernel context options, retains a 1MB internal message buffer by default, and requires an unsafe global disable function that aborts if called incorrectly.
Category: Kernel (libbitcoinkernel) (#8 of 18)
P3 · cleanup
- P3 because the public C API is experimental and underlying logging remains global until future work
- Improves safety by removing an abort-prone disable call and drops unnecessary in-memory buffering
Worthwhile improvement to the kernel C logging API that eliminates an unsafe btck_logging_disable() function and stops unnecessary 1MB in-memory log buffering. It is reasonably deferrable as the kernel API remains experimental and the underlying logging engine remains global until #30342.
Membership: Changes the libbitcoinkernel public C API in bitcoinkernel.h and implementation in bitcoinkernel.cpp.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 1
Reviewability: Stale: Needs rebase
- Needs rebase due to merge conflicts against master
- Author has been inactive for several months while PR remains in draft
The branch has merge conflicts against current master and has seen no author activity for 237 days.
Author status: silent since 2026-01-22 force push
Open concerns:
- The new API accepts per-connection handles but still modifies global logging state underneath, potentially misleading callers until per-instance loggers are implemented in #30342
- Draft status and preference from reviewers to treat this PR as design discussion while internal logging refactors proceed
Resolved concerns:
- Clarified object lifetime and handle ownership between btck_LoggingConnection and btck_ContextOptions
Agreement: Disputed
- Concept approval for making the logging interface local instead of global
- Concern that new handles mislead callers while logging remains global internally (sedited, stringintech)
- Prefers keeping this as a draft design discussion until multi-logger support lands (stickies-v)
- Converted to draft for conceptual discussion (ryanofsky)
Concept ACK, but reviewers lean against merging API changes before internal multi-logger support lands in #30342
Reviewers support making the logging interface local instead of global, but object to changing the public API ahead of the backend implementation because callers would expect per-connection isolation that does not yet exist.
- sedited Concept ACKed but questioned whether #30342 should land first
- stringintech prefers waiting for #30342 to keep API signatures consistent with current capabilities
- stickies-v gave Concept ACK with approach ~0 leaning NACK, suggesting keeping this as a draft discussion while internal refactorings happen
- ryanofsky converted to draft to continue conceptual discussion
Review verdicts (DrahtBot): 0
- Concept ACK: sedited, stickies-v
Dependencies
Enables:
Files
19 lines under test/bench/ci.
- src/kernel/bitcoinkernel.cpp +44/-40
- src/kernel/bitcoinkernel.h +32/-30
- src/kernel/bitcoinkernel_wrapper.h +31/-26
- src/test/kernel/test_kernel.cpp +9/-10
- src/bitcoin-chainstate.cpp +2/-2
Card
PR #33847 modifies the libbitcoinkernel C API so that logging options and streams attach to explicit logger connection handles and context options instead of global functions, while dropping an unsafe disable function and default 1MB log buffering. The goal is to provide a cleaner public interface for external consumers before the API solidifies, paving the way for multi-logger support in #30342. However, reviewers expressed concerns with changing public signatures ahead of backend per-connection support since settings still alter global state. The PR has been parked in draft with merge conflicts and no activity for several months.