#34730 util/log: Combine the warning/error log levels into a single alert level

ReviewabilityReviewsAgreementSize
#34730 ajtowns util/log: Combine the warning/error log levels into a single alert levelWaiting on author1Mild48 + 15 tests
  • Simplify log interpretation by removing the confusing distinction between warning and error levels
  • Clarify logging for node operators since node shutdown already makes actual failures obvious
  • Review #34038 first and needs rebase
  • Waiting on author response to an open objection
  • Code review ACK because it aligns logging macros with their actual usage (ryanofsky)
  • Nonblocking objection: losing distinct tags makes separating actual from potential issues harder (ryanofsky)
  • 48 lines added or modified outside tests
  • 15 lines added or modified in tests
  • 57 lines removed in total
  • 7 files, 2 commits
Utilities (logging, arguments, libraries)#50 of 59
P3 · cleanup
  • P3 because it eliminates a recurring source of confusion across the codebase with minimal disruption
  • Provides modest internal cleanup value by streamlining alert logging conventions for developers
  • Ranking pass: Moved down: cosmetic log-level consolidation that is gated on the #34038 decision.
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

Replaces the separate `Warning` and `Error` logging levels and macros with a single `Alert` level and `LogAlert` macro. `LogWarning` and `LogError` are retained as deprecated compatibility aliases for `LogAlert`. Existing warning and error log calls in `src/node/blockstorage.cpp` are converted to `LogAlert` via scripted diff.

Problem: The distinction between `Warning` and `Error` log levels was minor and confusing in practice: `Error` indicated that the node was about to shut down, while `Warning` did not, a distinction the operator already observes from whether the node halted.

Evidence of importance:

  • PR description: 'There's little benefit in having two error levels that the node operator should pay attention to when the only difference between them is that one means the node is about to shut down -- the admin can easily see that just by looking at whether the node did shutdown.'
  • Author references issue #30348 ('RFC: Misused LogError and LogWarning macros') and closed PR #30364 ('Replace LogError and LogWarning with LogAlert').

Scope: Includes a scripted-diff commit converting log calls in `src/node/blockstorage.cpp` and stripping trailing newlines from them.

Reviewability

Paused · Waiting on author

Open objection from ryanofsky with no author reply for 240 days; review would repeat what has been asked.

Agreement

Mild

objection that needs an answer has none yet (ryanofsky)

Thread notes: ryanofsky provided a Code review ACK while expressing Concept -0 with a non-blocking design objection preferring to keep error/warning levels.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ryanofsky (contributor)objection, approachremoving error and warning log levels loses tags helpful for distinguishing actual problems from potential problemsopen · v1, not yet re-checkedansweredno2026-03-06: 'Keep the error and warning levels instead of removing them, because `[error]` `[warning]` tags shown in log output are helpful for distinguishing actual problems from potential problems'

Support:

  • ryanofsky (contributor) (ACK, 2026-03-06): helps decrease the gap between way the macros are actually used and the way they were originally intended to be used

Participants:

  • ryanofsky (contributor, objection, 1 statements 2026-03-06 to 2026-03-06): Concept -0 preferring to keep error and warning levels and add LogCritical, though gave Code review ACK

Dependencies

Depends on: #34038

Files

26 lines under test/bench/ci.

  • src/node/blockstorage.cpp +24/-24
  • src/test/logging_tests.cpp +12/-8
  • doc/developer-notes.md +9/-9
  • src/logging.cpp +9/-9
  • src/util/log.h +6/-4
  • test/functional/feature_config_args.py +2/-2
  • test/functional/feature_coinstatsindex_compatibility.py +1/-1

Card

The short text the ranking pass compares across PRs.

This PR combines the separate warning and error logging levels into a single alert level accessed via LogAlert. It addresses ambiguity and misuse around whether an issue warrants LogWarning or LogError when the primary distinction was whether the node was shutting down. The author notes that node operators can already see whether the node halted, making two alert levels redundant. This change is marked draft pending PR 34038, which refactors how log levels are exposed through configuration options.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-18T03:25, confidence high, input hash 653fea7627416a71
thread read through 2026-03-06 (2 statements), head 4ce720ba87
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Needs rebase · draft

Categories

Utilities (logging, arguments, libraries)

P3 · cleanup (ranking pass: Moved down: cosmetic log-level consolidation that is gated on the #34038 decision.)

The change simplifies the logging API across the codebase by replacing confusing and frequently misused LogError and LogWarning macros with a single LogAlert macro. As noted in issue #30348, having distinct levels whose only difference was whether the node shut down caused recurring ambiguity, making this a worthwhile internal cleanup.

Membership: Modifies core logging utilities in src/logging.cpp and src/util/log.h to combine Warning and Error log levels into Alert.

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