#34729 Reduce log noise
https://github.com/bitcoin/bitcoin/pull/34729 · · +85/-54 in 13 files, 7 commits · labels: none
Goal
- Prevent misleading high-severity errors and log spam during routine network and proxy connection failures
This pull request introduces the LogWarnThenDebug macro in util/log.h to issue an initial warning on potentially misconfigured conditions and demote subsequent occurrences to categorized debug logs. It also audits and adjusts log levels across netbase, coinstats, txindex, and validation to reduce exaggerated LogError occurrences introduced by earlier refactors.
Problem: Node operators encounter excessive log spam and misleading high-severity errors for routine network connection failures or intermittent proxy issues.
Category: P2P (#57 of 65)
P4 · cleanup
- P4 because it reduces log noise on connection failures but does not affect protocol logic or DoS resistance
- The impact is limited to cleaner logs for node operators
Changes in netbase.cpp reduce log noise when connections fail or proxies disconnect, but have no effect on P2P protocol logic, relay correctness, or DoS resistance.
Membership: Modifies logging behavior in netbase.cpp for SOCKS5 proxy handshakes and socket connection attempts.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 0
Category: Utilities (logging, arguments, libraries) (#47 of 66)
P3 · cleanup
- P3 because it provides a reusable rate-limited logging pattern across multiple modules
- It eliminates misleading error-level reporting across the codebase
Generalizes a rate-limited logging pattern previously hardcoded in PCP/NAT-PMP into util/log.h and fixes misleading LogError calls across several modules.
Membership: Adds the LogWarnThenDebug macro in util/log.h and tests in logging_tests.cpp.
Factors: security/stability 0, bug 1, performance 0, user value 1, leverage 1
Reviewability: Ready
- Ready to review
The PR is rebased against master, passes CI, and has no pending requests on the author.
Author status: Active; rebased and pushed updates addressing reviewer comments.
Open concerns:
- Demoting error and warning messages to debug removes tags that help distinguish normal connection events from failure states.
Resolved concerns:
- Added unit test coverage for LogWarnThenDebug in logging_tests.
- Clarified why certain socket and proxy failures warrant LogWarnThenDebug versus standard debug logging.
Agreement: Positive w/ caveats
- General support for reducing log noise and adding the helper (sedited, l0rinc)
- Nonblocking objection: demoting error levels makes diagnosing failures harder (ryanofsky)
Positive, but ryanofsky notes demoting error levels makes diagnosing failures harder
Multiple contributors support the noise reduction and helper utility, but ryanofsky raised a standing caveat regarding the loss of explicit error tags.
- sedited and l0rinc provided ACKs approving the approach and implementation.
- ryanofsky gave Concept/Approach -0 noting that losing error/warning tags makes logs harder to interpret, but considered it acceptable to merge if others agree.
Review verdicts (DrahtBot): 0 (+4)
- Stale ACK: kevkevinpal, sedited, l0rinc, ryanofsky
Files
25 lines under test/bench/ci.
- src/netbase.cpp +28/-28
- src/test/logging_tests.cpp +23/-0
- src/common/pcp.cpp +4/-15
- src/util/log.h +12/-0
- src/index/txindex.cpp +3/-3
- doc/developer-notes.md +5/-0
- src/bitcoin-tx.cpp +3/-1
- src/validation.cpp +2/-2
- src/kernel/coinstats.cpp +1/-1
- src/script/signingprovider.cpp +1/-1
- src/script/signingprovider.h +1/-1
- src/wallet/scriptpubkeyman.h +1/-1
- src/wallet/test/wallet_tests.cpp +1/-1
Card
This pull request adds a LogWarnThenDebug logging helper in util/log.h and uses it alongside standard debug levels to demote exaggerated LogError messages across SOCKS5 networking, coinstats, and txindex. The change directly targets log file bloat and false alarms in debug.log for operators while preserving the initial warning for systematic misconfigurations. Reviewers broadly support the noise reduction, though ryanofsky expressed reservations that demoting messages makes diagnosing intermittent connection failures harder. The PR has several approvals, passes CI, and has no unresolved technical blockers.