logging: Add severity level to logs.
ClosedPublic
Actions

Authored by PiRK on Wed, Nov 27, 21:14.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC5f2fd5768759: logging: Add severity level to logs.

Summary

Overview: This PR introduces a new macro, LogPrintLevel, that allows developers to add logs with the severity level. Additionally, it will also print the log category if it is specified.

Sample log:

2022-03-04T16:41:15Z [opencon] [net:debug] trying connection XX.XX.XXX.XXX:YYYYY lastseen=2.7hrs

This is a backport of core#24464 and core#29419
https://github.com/bitcoin/bitcoin/pull/24464/commits/a8290649a6578df120a71c9493acdf071e010d96
https://github.com/bitcoin/bitcoin/pull/29419/commits/d3b3af90343b7671231afd7dff87e87ff86d31d7 (dedup category names and improve logging.cpp)
https://github.com/bitcoin/bitcoin/pull/24464/commits/e11cdc930375eaec8d737e116138b2f2018c099f

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Wed, Nov 27, 21:14

Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Nov 27, 21:14

PiRK requested review of this revision.Wed, Nov 27, 21:14

teamcity edited the summary of this revision. (Show Details)Wed, Nov 27, 21:14

PiRK added inline comments.Wed, Nov 27, 21:17

src/test/logging_tests.cpp
56–62 ↗	(On Diff #51155)	I have no clue about why we need to do this when Core does not need it. I checked the entire git history for logging.*, and didn't find anything that would explain why the logs are properly flushed for Core but not for Bitcoin ABC. Without this, all these tests fail because tmp_log_path is still empty when we read it to look for the log lines.

PiRK added inline comments.Wed, Nov 27, 21:19

src/logging.cpp
110 ↗	(On Diff #51155)	Unrelated to this diff: This one is unused, so if we ever need more log categories we could reuse the `(1 << 8)` slot

Harbormaster completed remote builds in B31356: Diff 51155.Wed, Nov 27, 21:43

PiRK added a child revision: D17222: log: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order.Thu, Nov 28, 12:41

Fabien accepted this revision.Fri, Nov 29, 08:46

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/logging.cpp
131 ↗	(On Diff #51155)	While you're at it you could remove this category and make it Net with loglevel Debug. It's better to do it in a followup

This revision is now accepted and ready to land.Fri, Nov 29, 08:46

PiRK added inline comments.Fri, Nov 29, 09:22

src/logging.cpp
131 ↗	(On Diff #51155)	OK. But I think if we do it now it will still print all log message for the Net category no matter the level.
131 ↗	(On Diff #51155)	OK but for now there is still no way to define the logging level per-category or globally, we need additional backports first (core#25614 for instance). So doing it right now would bloat the log files again.