Page MenuHomePhabricator

Severity-based logging, step 2
ClosedPublic

Authored by PiRK on Jun 17 2025, 20:03.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCfc4bc0043a01: Severity-based logging, step 2
Summary

user-configurable, per-category severity log levels based on an idea by John Newbery and refined in GitHub discussions by Wladimir Van der Laan and Marco Falke.

  • simplify the BCLog::Level enum class and the LogLevelToStr() function and add documentation
  • update the logging logic to filter logs by log level both globally and per-category
  • add a hidden -loglevel help-debug config option to allow testing setting the global or per-category severity level on startup for logging categories enabled with the -debug configuration option or the logging RPC (Klement Tan)
  • add a trace log severity level selectable by the user; the plan is for the current debug messages to become trace, LogPrint ones to become debug, and LogPrintf ones to become info, warning, or error

Simplify BCLog::Level enum class and LogLevelToStr() function

  • simplify the BCLog::Level enum class (and future changes to it) by only setting the value of the first enumerator
  • move the BCLog::Level:None enumerator to the end of the BCLog::Level enum class and LogLevelToStr() member function, as the None enumerator is only used internally, and by being the highest BCLog::Level value it can be used to iterate over the enumerators
  • replace the unused BCLog::Level:None string "none" with an empty string as the case will never be hit
  • add documentation

(Note that core#29419 (D17217) deduplicated the category names.)

Add BCLog::Logger::m_log_level data member and getter/setter

Co-authored-by: "klementtan <klementtan@gmail.com>"

Add BCLog::Logger::m_category_log_levels data member and getter/setter

Co-authored-by: "klementtan <klementtan@gmail.com>"

Add BCLog::Logger::SetLogLevel()/SetCategoryLogLevel() for string inputs

and remove unnecessary param constness in LogPrintStr()

Co-authored-by: jonatack <jon@atack.com>

Update LogAcceptCategory() and unit tests with log severity levels

Co-authored-by: "Jon Atack <jon@atack.com>"

Create BCLog::Logger::LogLevelsString() helper function

Co-authored-by: "Jon Atack <jon@atack.com>"

Unit test coverage for log severity levels

Co-authored-by: "Jon Atack <jon@atack.com>"

Create -loglevel configuration option

  • add a -loglevel=<level>|<category:level> config option to allow users to set a global -loglevel and category-specific log levels. LogPrintLevel messages with a higher severity level than -loglevel will not be printed in the debug log.
  • for now, this config option is debug-only during the migration to severity-based logging
  • update unit and functional tests

Co-authored-by: "Jon Atack <jon@atack.com>"

Unit test coverage for -loglevel configuration option

Co-authored-by: "Jon Atack <jon@atack.com>"

Note that the std namespace added for the << operator is needed so that BOOST_CHECK_EQUAL can digest enum class BCLog::Level).
It would normally be added in core#23497

Create BCLog::Level::Trace log severity level

for verbose log messages for development or debugging only, as bitcoind may run
more slowly, that are more granular/frequent than the Debug log level, i.e. for
very high-frequency, low-level messages to be logged distinctly from
higher-level, less-frequent debug logging that could still be usable in production.

An example would be to log higher-level peer events (connection, disconnection,
misbehavior, eviction) as Debug, versus Trace for low-level, high-volume p2p
messages in the BCLog::NET category. This will enable the user to log only the
former without the latter, in order to focus on high-level peer management events.

With respect to the name, "trace" is suggested as the most granular level
in resources like the following:

Update the test framework and add test coverage.

This is a backport of core#25614

Test Plan

ninja all check-all

Add LogPrintLevel lines for each level in a function frequently called, start bitcoind -loglevel=... with the various levels (trace, debug, info) and check that only the expected messages are visible in the terminal.

Event Timeline

PiRK requested review of this revision.Jun 17 2025, 20:03
src/test/util/setup_common.h
33

part of core#23497, without this BOOST_CHECK_EQUAL cannot deal with
the BCLog::Level enum class in the new logging_Conf test

Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/logging.h
88

Is this default going to change in the future ? It's a bit weird wrt the above comments

This revision is now accepted and ready to land.Jun 18 2025, 07:38
src/logging.h
88

The default is not going to change. But note that it is still only printed when the corresponding -debug=... category is enabled.
Also Info is going to be logged unconditionnaly in the future, so we will not even be able to how only warning and errors.
The whole thing is a bit unintuitive imo: https://github.com/bitcoin/bitcoin/pull/28318

  • LogInfo(...), LogWarning(...), LogError(...) for unconditional (uncategorised) logging (replaces LogPrintf)
  • LogDebug(CATEGORY, ...) and LogTrace(CATEGORY, ...) for conditional logging (replaces LogPrint)
  • LogPrintLevel(CATEGORY, LEVEL, ...) for when the level isn't known in advance, or a category needs to be added for an info/warning/error log message (mostly unchanged, but rarely needed)

Related developer notes after the next backport:
https://github.com/ajtowns/bitcoin/blob/f7ce5ac08c669ac763e275bb7c82dcfb2b1b6c33/doc/developer-notes.md#logging

This revision was automatically updated to reflect the committed changes.