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 Passed

Unit

No Test Coverage

Build Status

Buildable 31356
Build 62210: Build Diff	ecash-lib-integration-tests · chronik-client-integration-tests · ecash-agora-integration-tests · lint-circular-dependencies · build-without-wallet · build-diff · build-clang-tidy · build-debug · build-clang · build-chronik-plugins · build-chronik
Build 62209: arc lint + arc unit

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	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	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	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	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	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.