kernel, logging: Pass Logger instances to kernel objects

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:

logging: Improve new LogDebug/Trace/Info/Warning/Error Macros

Improve new LogDebug(), LogTrace(), LogInfo(),  LogWarning(), LogError() macros
introduced in #28318:

- Make them always accept log categories to make it possible to only log
  messages from a particular component.
- Make them not rely on a global LogInstance, to provide better control of
  logging in controlled environments, such as unit tests that want to
  selectively capture log output, or libbitcoinkernel applications that want to
  have multiple instances of validation objects without mixing their log output.
- Make them consistent, now all accepting the same parameters.
- Make them less verbose by not requiring BCLog category constants to be
  specified in individual log prints

9d9c1e8633

logging: Add preprocessor workaround for MSVC

Needed to fix errors like:

const Source &_LogSource(const Source &)': expects 1 arguments - 3 provided
const Source &_LogSource(const Source &)': 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

5f64eab013

Merge remote-tracking branch 'origin/pull/29256/head' 8c5ecae45f

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.

e9b91f5c5c

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.

facc2d0edf

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.

db1b9f7696

DrahtBot commented at 5:52 pm on June 26, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30364 (logging: Replace LogError and LogWarning with LogAlert by ryanofsky)
#30361 (doc: Drop description of LogError messages as fatal by ryanofsky)
#30343 (wallet, logging: Replace WalletLogPrintf() with LogInfo() by ryanofsky)
#30320 (assumeutxo: Don’t load a snapshot if it’s not in the best header chain by mzumsande)
#30267 (assumeutxo: Check snapshot base block is not in invalid chain by fjahr)
#30155 (validation: Make ReplayBlocks interruptible by mzumsande)
#30141 (kernel: De-globalize validation caches by TheCharlatan)
#30059 (Add option dbfilesize to control LevelDB target (“max”) file size by luke-jr)
#30039 (dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate by maciejsszmigiero)
#29656 (chainparams: Change nChainTx type to uint64_t by fjahr)
#29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)
#29640 (Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties) by sr-gi)
#29256 (Improve new LogDebug/Trace/Info/Warning/Error Macros by ryanofsky)
#28676 ([WIP] Cluster mempool implementation by sdaftuar)
#27826 (validation: log which peer sent us a header by Sjors)
#19463 (Prune locks by luke-jr)

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.

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.

Debug: https://github.com/bitcoin/bitcoin/runs/26722091796

ryanofsky commented at 10:36 pm on June 26, 2024: contributor

Updated 45423355735770f01d2bac738cc8b0b41415e921 -> db1b9f7696af80036de739408cd9eae5d06481ec (pr/gklog.1 -> pr/gklog.2, compare) moving code to an earlier commit to fix a “test-each-commit” test failure https://github.com/bitcoin/bitcoin/actions/runs/9684398780/job/26722073921?pr=30342, and cleaning up code to fix a clang-tidy warning https://cirrus-ci.com/task/5893151045451776

DrahtBot removed the label CI failed on Jun 27, 2024

DrahtBot added the label Needs rebase on Jul 2, 2024

DrahtBot commented at 9:50 pm on July 2, 2024: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

kernel, logging: Pass Logger instances to kernel objects #30342

Code Coverage

Reviews

Conflicts