#34778 logging: rewrite macros to enforce restrictions at compile-time, improve efficiency and usability

ReviewabilityReviewsAgreementSize
#34778 ryanofsky logging: rewrite macros to enforce restrictions at compile-time, improve efficiency and usabilityReady0 -1Blocked164 + 116 tests
  • Clean up logging macros and enforce correct category arguments at compile time with clear error messages
  • Skip message formatting overhead when logging is fully disabled, benefiting fuzzing and kernel uses
  • Ready to review
  • Concept objection to reintroducing generic LogPrint conflicting with existing work (ajtowns)
  • Objection that exposing LogPrint risks broken rate limiting and hurts greppability (maflcko)
  • Unaddressed objection: invalid LogDebug flags compile but trigger runtime assertion aborts (cyb3ralbert)
  • Unanswered question on whether omitting category prefixes in -loglevelalways is intentional (cyb3ralbert)
  • 164 lines added or modified outside tests
  • 116 lines added or modified in tests
  • 125 lines removed in total
  • 7 files, 6 commits
Utilities (logging, arguments, libraries)#16 of 59
P2 · cleanup
  • P2 after comparing with the other PRs here (assessed alone as P3): Raised from P3 to P2: it is the base of #29256 and #30342 and cannot sit below the logging work it unblocks.
  • P3 because it improves macro maintainability and catches incorrect logging usage at compile time
  • Eliminates unnecessary string formatting overhead when logging is completely turned off
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Refactors the logging macros in util/log.h by consolidating duplicated macro logic into a unified LOG_EMIT macro that takes compile-time options. Replaces opaque macro expansion errors with static assertions that validate category arguments at compile time, and replaces the NO_RATE_LIMIT tag with an explicit ratelimit option. Avoids message formatting overhead when logging is completely disabled while explicitly preserving argument evaluation for Info and higher log levels.

Problem: Logging macros duplicated logic and had undocumented inconsistencies. Misusing category arguments (such as passing a category to LogInfo or omitting one from LogDebug) produced confusing macro expansion syntax errors. In addition, format strings were formatted even when logging was completely disabled, and rate-limiting hints were stored in buffered log entries after rate limiting had already occurred.

Evidence of importance:

  • Compile-time errors for mismatched category arguments now produce clear messages via static_assert ('Missing required category argument for Debug/Trace logging call' and 'Cannot pass category argument to Info/Warning/Error logging call') in util/log.h.
  • Commit f5737f0478 cites PR #34374 review discussion noting that evaluating arguments even when logging is disabled at Info or higher is important to avoid unintended side effects.
  • Author notes in commit 6292d61b55 that skipping formatting when logging is disabled avoids unnecessary strprintf calls in fuzzing and kernel applications.

Scope: Touches the internal logging infrastructure in util/log.h and logging.cpp along with unit tests, but only adjusts one call site in validation.cpp to replace the deprecated NO_RATE_LIMIT tag.

Risk of a mistake: low (other, tests). A mistake could format log lines incorrectly or drop log messages, but does not affect consensus, validation, or network operations.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: address cyb3ralbert's question on -loglevelalways and report of invalid LogDebug flags compiling and aborting 2026-10-14
  • author: respond to maflcko's contested objections regarding LogPrint rate-limiting and greppability 2026-03-16

Agreement

Blocked

objection that needs a change, nobody has replied (maflcko, cyb3ralbert)

Thread notes: Added author's rebase comment c:4386647289; it reports rebases and updating kernel log hooks after #34374 merged, without addressing cyb3ralbert's findings or maflcko's objections.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ajtowns (member)objection, approachreintroducing a generic LogPrint macro conflicts with the existing approach in PR #34038answered · author explained macro cleanup rationale and proposed supporting NO_RATE_LIMIT flag; reviewer has not repliedchangedyes2026-03-10: 'Concept NACK to reintroducing a generic `LogPrint`.'
Status from: 2026-03-10: ryanofsky: 'This bigger goal of this PR is to replace current implementation of the logging macros which is messy and inconsistent w'
maflcko (member)objection, interfaceexposing the internal LogPrint macro invites helper patterns that mishandle rate limiting across log levels by silently dropping warnings or errors when info logs overflowcontested · reviewer pushed back in rc:2940897215 noting WalletLogPrintf as a real-world example, and author did not replychangedyes2026-03-10: 'this new macro may encourage to re-introduce helpers of the form: `[](auto level, auto msg) { LogPrint((.level=level),"%s", msg); }`, which will then work incorrectly with regard to rate-limiting'
Status from: 2026-03-16: maflcko: 'You give a real place yourself (`WalletLogPrintf`), where a dev (or an LLM) could apply the example today'
maflcko (member)objection, interfaceexposing generic LogPrint introduces an alias for level macros, hurting greppability and consistency across the codebaseopenchangedno2026-03-16: 'Just as the logging was fixed to be consistent, so that one can call `git grep LogInfo` to find all info logs, this will introduce a new alias to grep for.'
cyb3ralbert (contributor)question, correctnesswhether the change in -loglevelalways output omitting category prefixes is intentional, and if help text and release notes should be updatedopenansweredno2026-10-14: 'Is this change intentional? The help text still says "Always prepend a category and level", and the `98f3bd6` commit message says there is no visible effect except in tests.'
cyb3ralbert (contributor)objection, correctnessLogDebug accepts invalid flag types such as plain integers or OR-ed masks, which compile but trigger runtime assertion abortsopenchangedno2026-10-14: 'LogDebug now compiles when passed a plain integer or an OR-ed mask, whereas both failed to compile on the merge-base. For example, `LogDebug(BCLog::NET | BCLog::HTTP, "x")` aborts in a unit test'

Participants:

  • ajtowns (member, objection, 1 statements 2026-03-10 to 2026-03-10): Concept NACK to reintroducing a generic LogPrint macro
  • maflcko (member, objection, 6 statements 2026-03-10 to 2026-03-16): objected to exposing generic LogPrint, citing rate-limiting across levels, source locations, and greppability
  • sedited (member, neutral, 1 statements 2026-09-17 to 2026-09-17): suggested closing due to inactivity and unresolved objections
  • cyb3ralbert (contributor, objection, 1 statements 2026-10-14 to 2026-10-14): tested and reported -loglevelalways output changes and runtime assert failures with invalid LogDebug flags

Dependencies

Enables:

Base for: #29256, #30342, #30343

Files

137 lines under test/bench/ci.

  • src/util/log.h +136/-63
  • src/test/logging_tests.cpp +116/-21
  • src/logging.cpp +17/-23
  • src/kernel/bitcoinkernel.cpp +5/-12
  • src/validation.cpp +3/-2
  • src/logging.h +2/-2
  • doc/developer-notes.md +1/-2

Card

The short text the ranking pass compares across PRs.

PR #34778 rewrites logging macros in util/log.h to route through a single LOG_EMIT macro taking compile-time options. It enforces logging category rules at compile time via static assertions, eliminating cryptic compiler syntax errors when categories are omitted or misplaced. It also avoids message formatting when logging is disabled while preserving argument evaluation semantics for high-severity logs, and cleans up rate-limiting parameter plumbing. This is an internal developer refactoring that also serves as a base for kernel and wallet logging improvements in PR #30342 and PR #30343.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:16, confidence high, input hash b64e20b71a462ab2
thread read through 2026-10-14 (16 statements), head 4323986a35
labels: none

Categories

Utilities (logging, arguments, libraries)

P2 · cleanup (assessed alone as P3; Raised from P3 to P2: it is the base of #29256 and #30342 and cannot sit below the logging work it unblocks.)

This refactor modernizes logging macro infrastructure by replacing opaque expansion failures with clear static_assert error messages and eliminating formatting overhead when logging is disabled. The author notes static assertions catch cases like 'Missing required category argument for Debug/Trace logging call' at compile time and eliminate 'unnecessary strprintf calls in fuzzing and kernel applications'.

Membership: Rewrites core logging macros and utilities across src/util/log.h and src/logging.cpp.

Factors: security/stability 0, bug 0, performance 1, user value 0, leverage 1