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.
This is based on #29256. The non-base commits are:
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:
NOTE: the logger instances is leaked on exit. -> instance [subject-verb agreement: “instances is” should be “instance is”]
not wanting to allow users filter by category -> not wanting to allow users to filter by category [missing “to” before verb “filter”]
drahtbot_id_5_m
TheCharlatan
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
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
log, test: Add test for currently accepted logging arguments
Test will be extended in next commit to cover support for context objects.
260170d014
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>
e4ca362858
logging: 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.
98e2cef781
log, refactor: Add preprocessor workaround for MSVC
Needed to fix errors like:
const Context &GetContext(const Context &)': expects 1 arguments - 3 provided
due to a compiler bug:
https://stackoverflow.com/questions/5134523/msvc-doesnt-expand-va-args-correctly/5134656#5134656
Example CI failure:
https://github.com/bitcoin/bitcoin/actions/runs/8072891468/job/22055528830?pr=29256
b8b97370ce
log, refactor: Disallow category args to logging macros
Disallow passing BCLog category constants to LogInfo(), LogWarning(), and
LogError() macros, and disallow omitting BCLog categories when calling
LogDebug() and LogTrace() macros.
Additionally, drop category information from log output at higher levels as
suggested by Hodlinator
https://github.com/bitcoin/bitcoin/pull/29256#discussion_r2040671181.
None of these restrictions are technically necessary, and not everybody
believes they are good. However, they have existed since the
Log{Trace,Debug,Info,Warning,Error} macros were introduced in #28318, and
removing these restrictions is orthogonal to the goals of this PR which are:
(1) to allow the Log macros to work without accessing global state and (2) to
support local logging customization with additional metadata.
This change just more clearly documents the current logging restrictions and
provides better error messages to developers when enforcing them.
Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
8428899fb9
Merge branch 'pr/bclog' into pr/gklog19b90ba7c7
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.
8656985439
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.
bcfecce29b
refactor: Pass Logger instance to CTxMemPool
This allows libbitcoinkernel applications and test code to have the option to
control where log output goes instead of having all output sent to the global
logger.
7e569a5bd4
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.
d0e2944bb1
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 force-pushed
on Oct 16, 2025
DrahtBot added the label
Needs rebase
on Oct 29, 2025
DrahtBot
commented at 0:03 am on October 29, 2025:
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: 2025-10-31 12:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
