ryanofsky
commented at 5:52 pm on June 26, 2024:
contributor
Pass Logger instances to BlockManager, CCoinsViewDB, CDBWrapper, Chainstate, ChainstateManager, CoinsViews, and CTxMemPool classes so libbitcoinkernel applications and tests have the option to control where log output goes instead of having all output sent to the global logger.
This PR is an alternative to #30338. It is more limited because it only changes kernel code while leaving other parts of the codebase alone. It also takes the opportunity to migrate legacy LogPrint / LogPrintf calls to the new log macros introduced in #28318.
#25665 (refactor: Add util::Result failure types and ability to merge result values by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
LLM Linter (✨ experimental)
Possible typos and grammar issues:
logger instances is leaked -> logger instance is leaked [subject-verb agreement: “instances” with singular verb “is” is incorrect; make singular (“instance is leaked”) or plural (“instances are leaked”) to be grammatical]
2026-03-09 19:02:53
sedited
commented at 8:21 pm on June 26, 2024:
contributor
Concept ACK.
ryanofsky force-pushed
on Jun 26, 2024
DrahtBot added the label
CI failed
on Jun 26, 2024
DrahtBot
commented at 10:35 pm on June 26, 2024:
contributor
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot removed the label
CI failed
on Jun 27, 2024
DrahtBot added the label
Needs rebase
on Jul 2, 2024
ryanofsky referenced this in commit
ecadc8f6e5
on Jul 3, 2024
ryanofsky force-pushed
on Jul 9, 2024
ryanofsky
commented at 10:23 pm on July 9, 2024:
contributor
Rebased db1b9f7696af80036de739408cd9eae5d06481ec -> 42d0e06d1a4343c2bf3257f26cf18b79072c6b0d (pr/gklog.2 -> pr/gklog.3, compare) to fix conflict with #30141. Also added a commit to let kernel applications direct low-level util output to custom log instances.
Updated 42d0e06d1a4343c2bf3257f26cf18b79072c6b0d -> fcaed3faf0f12b3b322e65df1e81d455bbee7db5 (pr/gklog.3 -> pr/gklog.4, compare) to fix CI errors
Updated fcaed3faf0f12b3b322e65df1e81d455bbee7db5 -> f005677f762754ae09417c2a418e27d28ef3c7e1 (pr/gklog.4 -> pr/gklog.5, compare) to fix more CI errors
Rebased f005677f762754ae09417c2a418e27d28ef3c7e1 -> dcd055ecf9c20b84930c2ce45d922427cf0112ea (pr/gklog.5 -> pr/gklog.6, compare) due to conflicts with #30425 and #30407
Rebased dcd055ecf9c20b84930c2ce45d922427cf0112ea -> 53d7933727ebe0670dceb88ec9f1020bd7b0b280 (pr/gklog.6 -> pr/gklog.7, compare) due to conflict with #28052
Rebased 53d7933727ebe0670dceb88ec9f1020bd7b0b280 -> 10c275342dca365a6678da5fc016ca9d46d9dfca (pr/gklog.7 -> pr/gklog.8, compare) due to conflict with #30485
Rebased 10c275342dca365a6678da5fc016ca9d46d9dfca -> 493dd26b75d0e35422b3230be3c72a0f96eb5e8a (pr/gklog.8 -> pr/gklog.9, compare) due to various conflicts
Rebased 493dd26b75d0e35422b3230be3c72a0f96eb5e8a -> e971028fa5e570acf585073494acd65f4831ab1c (pr/gklog.9 -> pr/gklog.10, compare) due to conflicts with #31393, #31490, #30965
Rebased 97be3ef0b6239a9e674be3d8c70f99ec5fe5b8eb -> db29e8fb43fa8011eb5c68f376ea74b4f087deda (pr/gklog.18 -> pr/gklog.19, compare) on top of #29256 pr/bclog.34 and #33847 pr/klog.4
Rebased d6c23d4219b30dc8e045f4c6b90a7ad493faf383 -> 75e1e958f5e8a27491723aa88c69bf3cd8de7509 (pr/gklog.20 -> pr/gklog.21, compare) on top of #29256 pr/bclog.34 and #33847 pr/klog.5
Rebased 75e1e958f5e8a27491723aa88c69bf3cd8de7509 -> cc0cfe20c8ac56ffe3a47d9ee3f109fdf0c09c36 (pr/gklog.21 -> pr/gklog.22, compare) due to conflicts with #34253 and #33680
Rebased 7b78335f66b08d039f20dc31fb326883ab5ad3c8 -> aa4d26979f0b1c24adeeec489b36180c87ca484c (pr/gklog.24 -> pr/gklog.25, compare) on top of #29256 pr/bclog.36 and #33847 pr/klog.5
Rebased b6b95a62447c589a0078474fb8f6bc4b2e7e8a51 -> f048e807c6b4327474466c749fe48cb43dd651a0 (pr/gklog.30 -> pr/gklog.31, compare) on top of #29256 pr/bclog.39
DrahtBot removed the label
Needs rebase
on Jul 10, 2024
ryanofsky force-pushed
on Jul 10, 2024
ryanofsky force-pushed
on Jul 10, 2024
DrahtBot added the label
CI failed
on Jul 10, 2024
DrahtBot
commented at 6:22 pm on July 10, 2024:
contributor
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
Make sure to run all tests locally, according to the documentation.
The failure may happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot removed the label
CI failed
on Jul 22, 2024
DrahtBot added the label
Needs rebase
on Jul 26, 2024
ryanofsky force-pushed
on Aug 7, 2024
ryanofsky force-pushed
on Aug 7, 2024
DrahtBot removed the label
Needs rebase
on Aug 7, 2024
DrahtBot added the label
CI failed
on Aug 13, 2024
DrahtBot
commented at 7:50 pm on August 13, 2024:
contributor
Make sure to run all tests locally, according to the documentation.
The failure may happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot added the label
Needs rebase
on Aug 21, 2024
ryanofsky force-pushed
on Dec 9, 2024
DrahtBot removed the label
Needs rebase
on Dec 9, 2024
DrahtBot removed the label
CI failed
on Dec 9, 2024
DrahtBot added the label
CI failed
on Dec 11, 2024
DrahtBot removed the label
CI failed
on Dec 17, 2024
DrahtBot added the label
Needs rebase
on Dec 18, 2024
ryanofsky force-pushed
on Mar 12, 2025
DrahtBot removed the label
Needs rebase
on Mar 12, 2025
DrahtBot added the label
Needs rebase
on Mar 14, 2025
ryanofsky referenced this in commit
6d0745d22b
on Apr 3, 2025
ryanofsky force-pushed
on Apr 3, 2025
DrahtBot removed the label
Needs rebase
on Apr 3, 2025
ryanofsky referenced this in commit
2be41ef85f
on Apr 3, 2025
ryanofsky referenced this in commit
eaac991552
on Apr 4, 2025
ryanofsky force-pushed
on Apr 4, 2025
DrahtBot added the label
CI failed
on Apr 4, 2025
DrahtBot
commented at 3:37 am on April 4, 2025:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot removed the label
CI failed
on Apr 4, 2025
DrahtBot added the label
Needs rebase
on Apr 8, 2025
ryanofsky referenced this in commit
8e3ee9d7e7
on Oct 15, 2025
ryanofsky referenced this in commit
9e7dabc35d
on Oct 15, 2025
ryanofsky force-pushed
on Oct 15, 2025
DrahtBot removed the label
Needs rebase
on Oct 15, 2025
ryanofsky referenced this in commit
a5700c7911
on Oct 16, 2025
ryanofsky referenced this in commit
e4ca362858
on Oct 16, 2025
ryanofsky force-pushed
on Oct 16, 2025
DrahtBot added the label
Needs rebase
on Oct 29, 2025
fanquake referenced this in commit
4da01123df
on Nov 4, 2025
ryanofsky referenced this in commit
af46e97d08
on Nov 10, 2025
ryanofsky referenced this in commit
d0bd114e97
on Nov 10, 2025
ryanofsky referenced this in commit
9883750ab0
on Nov 10, 2025
ryanofsky referenced this in commit
6bbf29c5f8
on Nov 10, 2025
ryanofsky force-pushed
on Nov 10, 2025
DrahtBot removed the label
Needs rebase
on Nov 11, 2025
ryanofsky referenced this in commit
0926479924
on Nov 11, 2025
ryanofsky force-pushed
on Nov 11, 2025
DrahtBot added the label
Needs rebase
on Nov 25, 2025
ryanofsky referenced this in commit
45af98eb5a
on Dec 12, 2025
ryanofsky referenced this in commit
168128bb5e
on Dec 12, 2025
ryanofsky referenced this in commit
7c49fcfc21
on Dec 12, 2025
ryanofsky referenced this in commit
2d0bc18b9f
on Dec 12, 2025
ryanofsky force-pushed
on Dec 12, 2025
DrahtBot removed the label
Needs rebase
on Dec 12, 2025
DrahtBot added the label
CI failed
on Dec 12, 2025
DrahtBot
commented at 7:39 pm on December 12, 2025:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot added the label
Needs rebase
on Dec 14, 2025
ryanofsky referenced this in commit
fdcf06d4bb
on Dec 16, 2025
ryanofsky referenced this in commit
90476d0bb5
on Dec 16, 2025
ryanofsky force-pushed
on Dec 16, 2025
DrahtBot removed the label
Needs rebase
on Dec 16, 2025
DrahtBot added the label
Needs rebase
on Dec 16, 2025
ryanofsky referenced this in commit
5d43d4a39c
on Jan 18, 2026
ryanofsky referenced this in commit
ac3938a2f4
on Jan 18, 2026
ryanofsky referenced this in commit
05d22deffd
on Jan 18, 2026
ryanofsky referenced this in commit
c8f6a474f9
on Jan 18, 2026
ryanofsky referenced this in commit
480f08b3bd
on Jan 18, 2026
ryanofsky force-pushed
on Jan 18, 2026
DrahtBot removed the label
Needs rebase
on Jan 18, 2026
ryanofsky force-pushed
on Jan 18, 2026
DrahtBot removed the label
CI failed
on Jan 18, 2026
DrahtBot added the label
Needs rebase
on Jan 19, 2026
kernel: Simplify logging API
Current kernel logging API is too complicated and too restrictive. This PR
tries to improves it.
I'd expect an API that supported logging to allow creating log streams with
different options and providing some way to specify which library operations
should be logged to which streams.
By contrast, the current API allows creating multiple log streams, but all the
streams receive the same messages because options can only be set globally and
the stream objects can't be passed to any kernel API functions. They are not
even referenced by the `btck_Context` struct which holds other shared state. If
no log streams are created, log messages are generated anyway, but they are
stored in a 1MB buffer and not sent anywhere, unless a log stream is created
later, at which point they are sent in bulk to that stream. More log streams
can be created after that, but they only receive new messages, not the buffered
ones. If log output is not needed, a btck_logging_disable() call is required to
prevent log messages from accumulating in the 1MB buffer. Calling this will
abort the program if any log streams were created before it was called, and
also abort the program if any new log streams are created later.
None of these behaviors seem necessary or ideal, and it would be better to
provide a simpler logging API that allows creating a log stream, setting
options on it, registering it with the `btck_Context` instance and receiving
log messages from it. Another advantage of this approach is that it could allow
(with #30342) different log streams to be used for different purposes, which
would be not be possible with the current interface.
6d370c720a
ryanofsky referenced this in commit
cde4ae89a0
on Jan 22, 2026
ryanofsky force-pushed
on Jan 22, 2026
DrahtBot removed the label
Needs rebase
on Jan 23, 2026
DrahtBot added the label
Needs rebase
on Jan 29, 2026
ryanofsky force-pushed
on Feb 3, 2026
ryanofsky force-pushed
on Feb 3, 2026
DrahtBot added the label
CI failed
on Feb 3, 2026
DrahtBot
commented at 7:53 pm on February 3, 2026:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot removed the label
Needs rebase
on Feb 3, 2026
ryanofsky force-pushed
on Feb 3, 2026
DrahtBot removed the label
CI failed
on Feb 3, 2026
DrahtBot added the label
Needs rebase
on Feb 8, 2026
ryanofsky referenced this in commit
3a73b718e4
on Feb 9, 2026
ryanofsky referenced this in commit
2819b6d067
on Feb 9, 2026
ryanofsky referenced this in commit
8714e10178
on Feb 9, 2026
ryanofsky force-pushed
on Feb 9, 2026
DrahtBot removed the label
Needs rebase
on Feb 9, 2026
ryanofsky force-pushed
on Feb 9, 2026
DrahtBot added the label
CI failed
on Feb 9, 2026
DrahtBot
commented at 4:49 pm on February 9, 2026:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
ryanofsky force-pushed
on Feb 9, 2026
DrahtBot removed the label
CI failed
on Feb 9, 2026
DrahtBot added the label
Needs rebase
on Feb 18, 2026
ryanofsky referenced this in commit
5f5ac95167
on Mar 2, 2026
ryanofsky referenced this in commit
ee3fb8cf39
on Mar 2, 2026
ryanofsky force-pushed
on Mar 2, 2026
DrahtBot added the label
CI failed
on Mar 2, 2026
DrahtBot removed the label
Needs rebase
on Mar 2, 2026
ryanofsky force-pushed
on Mar 2, 2026
ryanofsky force-pushed
on Mar 3, 2026
DrahtBot removed the label
CI failed
on Mar 3, 2026
ryanofsky referenced this in commit
8517c14b77
on Mar 4, 2026
stickies-v
commented at 0:58 am on March 6, 2026:
contributor
I’ve finally started reviewing this PR in more detail, after my recent related work on kernel logging. I’m currently leaning Concept ACK, Approach NACK.
Concept ACK because:
a large part of the bitcoinkernel interface is contextualized, e.g. btck_ChainstateManager is initialized with a btck_Context, so having contextualized logging seems like a natural fit
bitcoinkernel is a library, we should be minimally opinionated on how it’s used, so long as it’s safe. If users have good use cases for e.g. multiple ChainstateManager’s, being able to separate log streams would be good
Approach NACK because I think the below downsides are bigger than the above upsides:
logging interface becomes more cumbersome, i.e. caller has to figure out if a logger is in scope and if it should be used. It is easy to misuse (e.g. forget to log to context when it exists, which seems hard to enforce.)
I think this change might become more desirable if/when we have more demand for this feature (i.e. more actual use cases for contextualized logging) or when it becomes a less invasive code change to do so (e.g. if/when kernel is a separate codebase, or when it becomes feasible to use an interface like m_log.info(...)).
I’ve asked the kernel working group for use cases and motivations for contextualized logging, and so far I’ve not heard much (but I hope people will chime in here rather than me speaking on their behalf). If you have any, perhaps opening a tracking issue for that might be useful so we can better decide when this feature becomes worth it given the complexity? I’m definitely not against the idea itself, I just pragmatically think it’s not worth the current cost.
ryanofsky
commented at 3:39 pm on March 6, 2026:
contributor
Thanks for the review!
logging interface becomes more cumbersome, i.e. caller has to figure out if a logger is in scope and if it should be used. It is easy to misuse (e.g. forget to log to context when it exists, which seems hard to enforce.)
I think it’s straightforward to enforce if there are places where we want to require log contexts. For example if we want to enforce it in kernel files it could look like:
0--- a/src/util/log.h
1+++ b/src/util/log.h
2@@ -156,11 +156,19 @@ void Log(Level level, bool should_ratelimit, SourceLocation&& source_loc, Contex
3 #define FirstArg_Impl(arg, ...) arg
4 #define FirstArg_(args) FirstArg_Impl args
5 6+constexpr bool LOG_REQUIRE_CONTEXT = false;
7+
8+template <typename T>
9+concept LogContext = requires {
10+ requires std::remove_reference_t<T>::log_context;
11+};
12+
13 //! Internal helper to conditionally log. Only evaluates arguments when needed.
14 // Allow __func__ to be used in any context without warnings:
15 // NOLINTBEGIN(bugprone-lambda-function-name)
16 #define LogPrint_(level, should_ratelimit, take_category, ...) \
17 do { \
18+ static_assert(!LOG_REQUIRE_CONTEXT || LogContext<decltype(FirstArg_((__VA_ARGS__)))>, "Log call must pass a log context as first argument"); \
19 auto&& _context{util::log::detail::GetContext<take_category>(FirstArg_((__VA_ARGS__)))}; \
20 if (util::log::ShouldLog(_context.logger, _context.category, (level))) { \
21 util::log::detail::Log((level), (should_ratelimit), SourceLocation{__func__}, \
22--- a/src/validation.cpp
23+++ b/src/validation.cpp
24@@ -76,6 +76,8 @@
25 #include <tuple>
26 #include <utility>
2728+#define LOG_REQUIRE_CONTEXT true
29+
30 using kernel::CCoinsStats;
31 using kernel::ChainstateRole;
32 using kernel::CoinStatsHashType;
Also note that without enforcement, the consequence of a missing context argument is just the log message going to the global logging stream, which already happens now.
I’d be interested to know more about what part of the code change seem difficult to review. The main change is adding a new parameter to a bunch of functions, which seems like the archetype of a change that is trivial to verify. I guess there are a lot of test updates too. Anyway, would like to know more about what specific difficulties you see here.
DrahtBot added the label
Needs rebase
on Mar 6, 2026
Test that LogInfo/LogWarning/LogError always evaluate their arguments
even when logging is disabled.
ajtowns pointed out this behavior was important and could affect non-logging
code if changed in
https://github.com/bitcoin/bitcoin/pull/34374#discussion_r2734793117
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
449fc7ce3b
log test: Add test for all accepted logging arguments
Add new logging test to call macros with all allowed combinations of macro
arguments.
The new test replaces a less comprehensive test that doesn't cover log
statements without format arguments. It's also moved to the top of the test
suite because it's a good illustration of what typical log prints look like,
what logging calls are allowed and disallowed, and what the resulting log
output looks like.
1ee5ae0c17
log refactor: Ensure categories are not logged at info and higher levels
Previously this used to be possible through the LogPrintLevel call but now that
call is removed, this change is just an internal refactoring and has no visible
effect except in tests.
8f3e279bb9
log refactor: log macro rewrite
Rewrite log macros to fix a number of issues: lack of control over rate
limiting, unnecessary strprintf calls during fuzzing, confusing error messages
when macros are called with the wrong arguments.
More specifically, benefits are:
- Control over rate limiting: `LogPrintLevel_` hack used to bypass rate limiting
is dropped. A `LogPrint` macro is added that can control rate limiting (as
well as other options if they are added in the future).
- Unnecessary `strprintf` calls are now skipped in bitcoind when
`-noprinttoconsole -nodebuglogfile` options are used, and skipped in tests and
kernel applications when logging is disabled. This change should not affect
bitcoind noticeably, but it could speed up fuzz tests calling functions that
log.
- Clearer error messages: Differences between macros accepting and not accepting
category arguments has been a source of confusion. Now if you pass a category
to a macro which does not accept it, or forget to pass a category to a macro
which requires it, you will see a clear error message telling you to add or
remove the category instead of more confusing macro errors.
- Previously you could call `LogPrintLevel_` to bypass restrictions on passing
category arguments. Now `LogPrint` enforces all requirements.
- Previously "always evaluate arguments" behavior at Info and higher levels
looked accidental and was undocumented
(https://github.com/bitcoin/bitcoin/pull/34374#discussion_r2734793117). Now
the behavior is documented and explicit.
- There is now a single macro, `LogPrint` implementing all logging functionality
and enforcing all restrictions, instead of different sets of macros with
branching code paths and unexplained differences.
Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-Authored-By: stickies-v <stickies-v@protonmail.com>
e983d97a91
Merge branch 'pr/relog' into pr/bclog7055a641bc
log refactor: Allow log macros to accept context arguments
Allow LogDebug(), LogTrace(), LogInfo(), LogWarning(), and LogError() macros to
accept context arguments to provide more information in log messages and more
control over logging to callers.
This functionality is used in followup PRs:
- https://github.com/bitcoin/bitcoin/pull/30342 - To let libbitcoinkernel send
output to specfic `BCLog::Logger` instances instead of a global instance, so
output can be disambiguated and applications can have more control over
logging.
- https://github.com/bitcoin/bitcoin/pull/30343 - To replace custom
`WalletLogPrintf` calls with standard logging calls that automatically include
wallet names and don't log everything at info level.
This commit does not change behavior of current log prints or require them to
be updated. It includes tests and documentation covering the new functionality.
Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-Authored-By: stickies-v <stickies-v@protonmail.com>
502820b5a5
doc: Add documentation about log levels and macrosc410293dfa
log refactor: Add support for custom log contexts
Custom log contexts allow overridding log formatting and adding metadata, such
as request ids or wallet names to log messages, while still using standard
macros for logging. This is used to replace WalletLogPrintf() functions with
LogInfo() calls in followup PR #30343.
724b5aed21
Merge branch 'pr/bclog' into pr/gklog1cb9ab1c68
Merge branch 'pr/klog' into pr/gklogf72c67a970
refactor: Pass Logger instances to kernel objects
Pass Logger instances to BlockManager, CCoinsViewDB, CDBWrapper,
ChainstateManager, and CoinsViews instances so libbitcoinkernel applications
and test code have the option to control where log output goes instead of
having all output sent to the global logger.
This commit just passes the logger objects without using them. The next commit
updates log print statements to use the new objects.
4c740d3d45
logging: Add LOG_REQUIRE_CONTEXT option
Add option to require context objects to be passed to log macros in a file or
block of code, so new logging calls can't forget to specify them and
accidentally log to the global logging stream.
4ecb05a429
refactor: Log kernel output to local log instances
This is a mechanical change updating kernel code that currently uses the global
log instance to log to local instances instead.
fbf6f92897
kernel: Drop global Logger instance
Change LogInstance() function to no longer allocate (and leak) a BCLog::Logger
instance. Instead allow kernel applications to initialize their own logging
instances that can be returned by LogInstance().
The LogInstance() function is not actually used for the majority of logging in
kernel code. Most kernel log output goes directly to BCLog::Logger instances
specified when kernel objects like ChainstateManager and CTxMemPool are
constructed, which allows applications to create multiple Logger instances and
control which log output is sent to them.
The only kernel code that does rely on LogInstance() is certain low level code in
the util library, like the RNG seeder, that is not passed a specific instance
and needs to rely on the global LogInstance() function.
Other code outside the kernel library uses LogInstance() heavily, and may
continue to do so. bitcoind and test code now just create a log instance before
the first LogInstance() call, which it returns, so all behavior is the same as
before.
71e2a726cb
stickies-v
commented at 6:03 am on March 8, 2026:
contributor
I think it’s straightforward to enforce if there are places where we want to require log contexts. For example if we want to enforce it in kernel files it could look like:
Nice, I hadn’t thought of a per-file approach, that seems elegant.
I’d be interested to know more about what part of the code change seem difficult to review.
I don’t think (and didn’t say) that this PR is difficult to review. But I think “non-trivial code change / review cost, lots of merge conflicts, …” is a reasonable statement for a cumulative diff of this size and 26 PR merge conflicts.
Note: I do think this is an elegant implementation, and I am not necessarily opposed to the changes, I think it would be nice if kernel eventually has contextualized logging. I just don’t think it’s currently important as it seems users (incl myself) don’t actually need this. Just making kernel logging explicitly global (which I planned to open a PR for earlier, but I’ve been focused on v31 review for the past weeksopened #34775) seems like the more pragmatic change to me now. It addresses the interface weirdness with a much smaller change, and keeps the logging interface simpler.
ryanofsky referenced this in commit
a71c30c23d
on Mar 9, 2026
ryanofsky force-pushed
on Mar 9, 2026
DrahtBot removed the label
Needs rebase
on Mar 9, 2026
DrahtBot added the label
CI failed
on Mar 9, 2026
ryanofsky force-pushed
on Mar 9, 2026
DrahtBot removed the label
CI failed
on Mar 9, 2026
DrahtBot added the label
Needs rebase
on Mar 11, 2026
DrahtBot
commented at 2:00 pm on March 11, 2026:
contributor
🐙 This pull request conflicts with the target branch and needs rebase.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-03-23 09:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
