From eb8aab759fb15824a5dd3004e689d0eb5b884a32 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 25 May 2022 18:26:54 +0200 Subject: logging: add LogPrintfCategory to log unconditionally with category prefixing the output with the passed category name. - add documentation - add a unit test - update lint-logs.py - update lint-format-strings.py --- src/logging.h | 8 +++++++- src/test/logging_tests.cpp | 5 ++++- test/lint/lint-format-strings.py | 1 + test/lint/lint-logs.py | 4 ++-- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/logging.h b/src/logging.h index 8a896b6b33..50869ad89a 100644 --- a/src/logging.h +++ b/src/logging.h @@ -199,13 +199,18 @@ static inline void LogPrintf_(const std::string& logging_function, const std::st } } - #define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) +// Log unconditionally. #define LogPrintf(...) LogPrintLevel_(BCLog::LogFlags::NONE, BCLog::Level::None, __VA_ARGS__) +// Log unconditionally, prefixing the output with the passed category name. +#define LogPrintfCategory(category, ...) LogPrintLevel_(category, BCLog::Level::None, __VA_ARGS__) + // Use a macro instead of a function for conditional logging to prevent // evaluating arguments when logging for the category is not enabled. + +// Log conditionally, prefixing the output with the passed category name. #define LogPrint(category, ...) \ do { \ if (LogAcceptCategory((category), BCLog::Level::Debug)) { \ @@ -213,6 +218,7 @@ static inline void LogPrintf_(const std::string& logging_function, const std::st } \ } while (0) +// Log conditionally, prefixing the output with the passed category name and severity level. #define LogPrintLevel(category, level, ...) \ do { \ if (LogAcceptCategory((category), (level))) { \ diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 3f6a605945..5a5e3b3f1f 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -103,6 +103,7 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup) LogPrintLevel(BCLog::NET, BCLog::Level::Info, "foo8: %s\n", "bar8"); LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "foo9: %s\n", "bar9"); LogPrintLevel(BCLog::NET, BCLog::Level::Error, "foo10: %s\n", "bar10"); + LogPrintfCategory(BCLog::VALIDATION, "foo11: %s\n", "bar11"); std::ifstream file{tmp_log_path}; std::vector log_lines; for (std::string log; std::getline(file, log);) { @@ -114,7 +115,9 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup) "[net:debug] foo7: bar7", "[net:info] foo8: bar8", "[net:warning] foo9: bar9", - "[net:error] foo10: bar10"}; + "[net:error] foo10: bar10", + "[validation] foo11: bar11", + }; BOOST_CHECK_EQUAL_COLLECTIONS(log_lines.begin(), log_lines.end(), expected.begin(), expected.end()); } diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 412cf86791..522023e328 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -22,6 +22,7 @@ FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [ 'LogConnectFailure,1', 'LogPrint,1', 'LogPrintf,0', + 'LogPrintfCategory,1', 'LogPrintLevel,2', 'printf,0', 'snprintf,2', diff --git a/test/lint/lint-logs.py b/test/lint/lint-logs.py index de53729b4e..aaf697467d 100755 --- a/test/lint/lint-logs.py +++ b/test/lint/lint-logs.py @@ -16,12 +16,12 @@ from subprocess import check_output def main(): - logs_list = check_output(["git", "grep", "--extended-regexp", r"(LogPrintLevel|LogPrintf?)\(", "--", "*.cpp"], universal_newlines=True, encoding="utf8").splitlines() + logs_list = check_output(["git", "grep", "--extended-regexp", r"(LogPrintLevel|LogPrintfCategory|LogPrintf?)\(", "--", "*.cpp"], universal_newlines=True, encoding="utf8").splitlines() unterminated_logs = [line for line in logs_list if not re.search(r'(\\n"|/\* Continued \*/)', line)] if unterminated_logs != []: - print("All calls to LogPrintf(), LogPrint(), LogPrintLevel(), and WalletLogPrintf() should be terminated with \"\\n\".") + print("All calls to LogPrintf(), LogPrintfCategory(), LogPrint(), LogPrintLevel(), and WalletLogPrintf() should be terminated with \"\\n\".") print("") for line in unterminated_logs: -- cgit v1.2.3 From ecff20db286e2f5d3afe32cfaae72de69d34d23c Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 25 May 2022 18:43:13 +0200 Subject: logging: use LogPrintfCategory rather than a manual category Here we update only the log messages that manually print a category. In upcoming commits, LogPrintCategory will likely be used in many other cases, such as to replace `LogPrintf` where it makes sense. --- src/httpserver.cpp | 4 ++-- src/i2p.cpp | 4 ++-- src/init.cpp | 4 ++-- src/torcontrol.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 44937dc523..be42da92a7 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -400,7 +400,7 @@ bool InitHTTPServer() LogPrint(BCLog::HTTP, "Initialized HTTP server\n"); int workQueueDepth = std::max((long)gArgs.GetIntArg("-rpcworkqueue", DEFAULT_HTTP_WORKQUEUE), 1L); - LogPrintf("HTTP: creating work queue of depth %d\n", workQueueDepth); + LogPrintfCategory(BCLog::HTTP, "creating work queue of depth %d\n", workQueueDepth); g_work_queue = std::make_unique>(workQueueDepth); // transfer ownership to eventBase/HTTP via .release() @@ -424,7 +424,7 @@ void StartHTTPServer() { LogPrint(BCLog::HTTP, "Starting HTTP server\n"); int rpcThreads = std::max((long)gArgs.GetIntArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L); - LogPrintf("HTTP: starting %d worker threads\n", rpcThreads); + LogPrintfCategory(BCLog::HTTP, "starting %d worker threads\n", rpcThreads); g_thread_http = std::thread(ThreadHTTP, eventBase); for (int i = 0; i < rpcThreads; i++) { diff --git a/src/i2p.cpp b/src/i2p.cpp index 9b8f83caef..eccb048bb3 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -376,8 +376,8 @@ void Session::CreateIfNotCreatedAlready() m_session_id = session_id; m_control_sock = std::move(sock); - LogPrintf("I2P: SAM session created: session id=%s, my address=%s\n", m_session_id, - m_my_addr.ToString()); + LogPrintfCategory(BCLog::I2P, "SAM session created: session id=%s, my address=%s\n", + m_session_id, m_my_addr.ToString()); } std::unique_ptr Session::StreamAccept() diff --git a/src/init.cpp b/src/init.cpp index d0fd6074b1..bc29a0ede6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1506,8 +1506,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) uiInterface.InitMessage(_("Verifying blocks…").translated); auto check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) { - LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", - MIN_BLOCKS_TO_KEEP); + LogPrintfCategory(BCLog::PRUNE, "pruned datadir may not have more than %d blocks; only checking available blocks\n", + MIN_BLOCKS_TO_KEEP); } maybe_verify_error = VerifyLoadedChainstate(chainman, fReset, diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 82b8529d76..d6e792a55f 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -421,7 +421,7 @@ void TorController::add_onion_cb(TorControlConnection& _conn, const TorControlRe return; } service = LookupNumeric(std::string(service_id+".onion"), Params().GetDefaultPort()); - LogPrintf("tor: Got service ID %s, advertising service %s\n", service_id, service.ToString()); + LogPrintfCategory(BCLog::TOR, "Got service ID %s, advertising service %s\n", service_id, service.ToString()); if (WriteBinaryFile(GetPrivateKeyFile(), private_key)) { LogPrint(BCLog::TOR, "Cached service private key to %s\n", fs::PathToString(GetPrivateKeyFile())); } else { -- cgit v1.2.3