Merge bitcoin/bitcoin#25306: logging: add LogPrintfCategory to log unconditionally with category

ecff20d logging: use LogPrintfCategory rather than a manual category (Jon Atack)
eb8aab7 logging: add LogPrintfCategory to log unconditionally with category (Jon Atack)

Pull request description:

  These are the next two commits from #25203.

  - Add `LogPrintfCategory` to log unconditionally while prefixing the output with the passed category name. Add documentation and a unit test, and update the `lint-logs.py` and `lint-format-strings.py` scripts.

  - Replace the log messages that manually print a category, with `LogPrintfCategory`. In upcoming commits, it will likely be used in many other cases, such as to replace `LogPrintf` where it makes sense.

ACKs for top commit:
  klementtan:
    Code Review ACK ecff20d
  laanwj:
    Code review ACK ecff20d
  brunoerg:
    ACK ecff20d

Tree-SHA512: ad3a82835254f7606efcd14b88f3d9072f1eb9b25db1321ed38ef6a4ec60efd555d78f5e19d93736f2f8500251d06f8beee9d694a153f24bf5cce3590a2a45a5

Author: laanwj

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<WorkQueue<HTTPClosure>>(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++) {

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<Sock> Session::StreamAccept()

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,

src/logging.h

@@ -199,20 +199,26 @@ 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)) {      \
             LogPrintLevel_(category, BCLog::Level::None, __VA_ARGS__); \
         }                                                              \
     } while (0)
 
+// Log conditionally, prefixing the output with the passed category name and severity level.
 #define LogPrintLevel(category, level, ...)               \
     do {                                                  \
         if (LogAcceptCategory((category), (level))) {     \

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<std::string> 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());
 }
 

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 {

test/lint/lint-format-strings.py

@@ -22,6 +22,7 @@
     'LogConnectFailure,1',
     'LogPrint,1',
     'LogPrintf,0',
+    'LogPrintfCategory,1',
     'LogPrintLevel,2',
     'printf,0',
     'snprintf,2',

test/lint/lint-logs.py

@@ -16,12 +16,12 @@
 
 
 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: