log: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order - #25202
Merged
Merged
Conversation
laanwj
force-pushed
the
2022-05-logging-improved
branch
from
May 24, 2022 18:42
ff55e92 to
cdd3d8d
Compare
jonatack
reviewed
May 24, 2022
laanwj
force-pushed
the
2022-05-logging-improved
branch
from
May 24, 2022 19:21
cdd3d8d to
d2a8887
Compare
6 tasks
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. 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. |
maflcko
reviewed
May 25, 2022
maflcko
approved these changes
May 25, 2022
Messages with level `WARN` or higher should be logged even when the category is not provided with `-debug=`, to make sure important warnings are not lost.
Map libevent's severity to our own severity level for logging.
This is more consistent with the other functions, as well as with the logging output itself. If we want to make this change, we should do it before it's all over the place.
laanwj
force-pushed
the
2022-05-logging-improved
branch
from
May 25, 2022 09:33
d2a8887 to
c4e7717
Compare
Member
Author
|
Re-pushed d2a8887→c4e7717:
|
laanwj
changed the title
jonatack
reviewed
May 25, 2022
| // Please do not do this in normal code | ||
| void Logv(const char * format, va_list ap) override { | ||
| if (!LogAcceptCategory(BCLog::LEVELDB)) { | ||
| if (!LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug)) { |
Member
There was a problem hiding this comment.
Member
Author
There was a problem hiding this comment.
I agree, but as you say, this was the case before this PR, and is not introduced with category/severity based logging. There's probably tons of source files that, besides these ones, need logging.h included.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log messages from leveldb and libevent libraries in the severity+level based log format introduced in #24464.
Example of messages before:
Example of messages after:
The first commit changes it so that messages with level Warning and Error are always logged independent of the
-debugsetting. I think this is useful to make sure warnings and errors, which tend to be important, are not lost. In the future this should be made more configurable.Last commit changes LogPrintLevel argument order to category, severity: This is more consistent with the other functions, as well as with the logging output itself. If we want to make this change, we should do it before it's all over the place.