Conversation
This generalises the functionality introduced for PCP/NAT-PMP logging in bitcoin#34549.
This mostly reduces log noise by using BCLog::NET debug logging where possible (with `LogWarnThenDebug()` used to ensure the first log message still appears for things that may be due to system misconfiguration). Also drops some unnecessary trailing newlines.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
See #30348 for further background; I think the changes here should be able to stand on their own merits. I haven't addressed cases where there's ambiguity between LogWarning and LogError here, but if there are other current cases where things are at Warning/Error and should be Info, or at Warning/Error/Info and should be Debug, happy to add them to this PR. |
kevkevinpal
left a comment
kevkevinpal
left a comment
There was a problem hiding this comment.
ACK c665935
This is a good improvement on the amount of functional code dedicated to logging. Also, the LogWarnThenDebug log will be useful in future cases to help avoid log noise
Added two comments about potentially adding BCLog::VALIDATION to the two validation logs touched
| : coin->nHeight; | ||
| } else { | ||
| LogInfo("ERROR: %s: Missing input %d in transaction \'%s\'\n", __func__, i, tx.GetHash().GetHex()); | ||
| LogWarning("%s: Missing input %d in transaction \'%s\'", __func__, i, tx.GetHash().GetHex()); |
There was a problem hiding this comment.
| LogWarning("%s: Missing input %d in transaction \'%s\'", __func__, i, tx.GetHash().GetHex()); | |
| LogWarning(BCLog::VALIDATION, %s: Missing input %d in transaction \'%s\'", __func__, i, tx.GetHash().GetHex()); |
| } | ||
| if (!state.IsValid()) { | ||
| LogInfo("Block validation error: %s", state.ToString()); | ||
| LogWarning("Block validation error: %s", state.ToString()); |
There was a problem hiding this comment.
| LogWarning("Block validation error: %s", state.ToString()); | |
| LogWarning(BCLog::VALIDATION, "Block validation error: %s", state.ToString()); |
3 participants
This set of changes reduces some of the exaggerated logging that was introduced in #29236 when "error()" logging was upgraded from
LogPrintf(akaLogInfo) toLogError. It also introducesLogWarnThenDebug()which duplicates the behaviour of #34549 which issues a warning the first time the log line is hit, but downgrades that to categorized debug messages afterwards. This function is used for failures that are likely caused by system configuration problems (so likely degrade the node's functionality and warrant admin attention) but are triggered by remote activity and thus could occur frequently.