Merge bitcoin/bitcoin#25614: Severity-based logging, step 2

958048057087e6562b474f9028316c00ec03c2e4 Update debug logging section in the developer notes (Jon Atack)
1abaa31aa3d833caf2290d6c90f57f7f79d146c0 Update -debug and -debugexclude help docs for severity level logging (Jon Atack)
45f92821621a60891044f57c7a7bc4ab4c7d8a01 Create BCLog::Level::Trace log severity level (Jon Atack)
2a8712db4fb5d06f1a525a79bb0f793cb733aaa6 Unit test coverage for -loglevel configuration option (klementtan)
eb7bee5f84d41e35cb4296e01bea2aa5ac80a856 Create -loglevel configuration option (klementtan)
98a1f9c68744074f29fa5fa67514218b5ee9edc4 Unit test coverage for log severity levels (klementtan)
9c7507bf76e79da99766a69df939520ea0a125d1 Create BCLog::Logger::LogLevelsString() helper function (klementtan)
8fe3457dbb4146952b92fb9509bbe4e97dc1f05b Update LogAcceptCategory() and unit tests with log severity levels (klementtan)
c2797cfc602c5cdd899a7c11b37bb5711cebff38 Add BCLog::Logger::SetLogLevel()/SetCategoryLogLevel() for string inputs (klementtan)
f6c0cc03509255ffa4dfd6e2822fce840dd0b181 Add BCLog::Logger::m_category_log_levels data member and getter/setter (Jon Atack)
2978b387bffc226fb1eaca4d30f24a0deedb2a36 Add BCLog::Logger::m_log_level data member and getter/setter (Jon Atack)
f1379aeca9d3a8c4d3528de4d0af8298cb42fee4 Simplify BCLog::Level enum class and LogLevelToStr() function (Jon Atack)

Pull request description:

  This is an updated version of https://github.com/bitcoin/bitcoin/pull/25287 and the next steps in parent PR #25203 implementing, with Klement Tan, user-configurable, per-category severity log levels based on an idea by John Newbery and refined in GitHub discussions by Wladimir Van der Laan and Marco Falke.

  - simplify the `BCLog::Level` enum class and the `LogLevelToStr()` function and add documentation
  - update the logging logic to filter logs by log level both globally and per-category
  - add a hidden `-loglevel` help-debug config option to allow testing setting the global or per-category severity level on startup for logging categories enabled with the `-debug` configuration option or the logging RPC (Klement Tan)
  - add a `trace` log severity level selectable by the user; the plan is for the current debug messages to become trace, LogPrint ones to become debug, and LogPrintf ones to become info, warning, or error

  ```
  $ ./src/bitcoind -help-debug | grep -A10 loglevel
    -loglevel=<level>|<category>:<level>
         Set the global or per-category severity level for logging categories
         enabled with the -debug configuration option or the logging RPC:
         info, debug, trace (default=info); warning and error levels are
         always logged. If <category>:<level> is supplied, the setting
         will override the global one and may be specified multiple times
         to set multiple category-specific levels. <category> can be:
         addrman, bench, blockstorage, cmpctblock, coindb, estimatefee,
         http, i2p, ipc, leveldb, libevent, lock, mempool, mempoolrej,
         net, proxy, prune, qt, rand, reindex, rpc, selectcoins, tor,
         util, validation, walletdb, zmq.
  ```

  See the individual commit messages for details.

ACKs for top commit:
  jonatack:
    One final push per `git range-diff a5d5569 ce3c4c9 9580480` (should be trivial to re-ACK) to ensure this pull changes no default behavior in any way for users or the tests/CI in order to be completely v24 compatible, to update the unit test setup in general, and to update the debug logging section in the developer notes.
  klementtan:
    reACK https://github.com/bitcoin/bitcoin/commit/958048057087e6562b474f9028316c00ec03c2e4
  1440000bytes:
    reACK https://github.com/bitcoin/bitcoin/pull/25614/commits/958048057087e6562b474f9028316c00ec03c2e4
  vasild:
    ACK 958048057087e6562b474f9028316c00ec03c2e4
  dunxen:
    reACK 9580480
  brunoerg:
    reACK 958048057087e6562b474f9028316c00ec03c2e4

Tree-SHA512: 476a638e0581f40b5d058a9992691722e8b546471ec85e07cbc990798d1197fbffbd02e1b3d081b4978404e07a428378cdc8e159c0004b81f58be7fb01b7cba0

3 files changed, 119 insertions, 11 deletions

diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp
index 80684ac205..9da1ee11f9 100644
--- a/src/test/i2p_tests.cpp
+++ b/src/test/i2p_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <i2p.h>
+#include <logging.h>
 #include <netaddress.h>
 #include <test/util/logging.h>
 #include <test/util/net.h>
@@ -19,6 +20,8 @@ BOOST_FIXTURE_TEST_SUITE(i2p_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(unlimited_recv)
 {
+    const auto prev_log_level{LogInstance().LogLevel()};
+    LogInstance().SetLogLevel(BCLog::Level::Trace);
     auto CreateSockOrig = CreateSock;
 
     // Mock CreateSock() to create MockSock.
@@ -39,6 +42,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv)
     }
 
     CreateSock = CreateSockOrig;
+    LogInstance().SetLogLevel(prev_log_level);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
index 5a5e3b3f1f..a6f3a62c71 100644
--- a/src/test/logging_tests.cpp
+++ b/src/test/logging_tests.cpp
@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <init/common.h>
 #include <logging.h>
 #include <logging/timer.h>
 #include <test/util/setup_common.h>
@@ -10,6 +11,7 @@
 #include <chrono>
 #include <fstream>
 #include <iostream>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -17,6 +19,12 @@
 
 BOOST_FIXTURE_TEST_SUITE(logging_tests, BasicTestingSetup)
 
+static void ResetLogger()
+{
+    LogInstance().SetLogLevel(BCLog::DEFAULT_LOG_LEVEL);
+    LogInstance().SetCategoryLogLevel({});
+}
+
 struct LogSetup : public BasicTestingSetup {
     fs::path prev_log_path;
     fs::path tmp_log_path;
@@ -25,6 +33,8 @@ struct LogSetup : public BasicTestingSetup {
     bool prev_log_timestamps;
     bool prev_log_threadnames;
     bool prev_log_sourcelocations;
+    std::unordered_map<BCLog::LogFlags, BCLog::Level> prev_category_levels;
+    BCLog::Level prev_log_level;
 
     LogSetup() : prev_log_path{LogInstance().m_file_path},
                  tmp_log_path{m_args.GetDataDirBase() / "tmp_debug.log"},
@@ -32,14 +42,21 @@ struct LogSetup : public BasicTestingSetup {
                  prev_print_to_file{LogInstance().m_print_to_file},
                  prev_log_timestamps{LogInstance().m_log_timestamps},
                  prev_log_threadnames{LogInstance().m_log_threadnames},
-                 prev_log_sourcelocations{LogInstance().m_log_sourcelocations}
+                 prev_log_sourcelocations{LogInstance().m_log_sourcelocations},
+                 prev_category_levels{LogInstance().CategoryLevels()},
+                 prev_log_level{LogInstance().LogLevel()}
     {
         LogInstance().m_file_path = tmp_log_path;
         LogInstance().m_reopen_file = true;
         LogInstance().m_print_to_file = true;
         LogInstance().m_log_timestamps = false;
         LogInstance().m_log_threadnames = false;
-        LogInstance().m_log_sourcelocations = true;
+
+        // Prevent tests from failing when the line number of the logs changes.
+        LogInstance().m_log_sourcelocations = false;
+
+        LogInstance().SetLogLevel(BCLog::Level::Debug);
+        LogInstance().SetCategoryLogLevel({});
     }
 
     ~LogSetup()
@@ -51,6 +68,8 @@ struct LogSetup : public BasicTestingSetup {
         LogInstance().m_log_timestamps = prev_log_timestamps;
         LogInstance().m_log_threadnames = prev_log_threadnames;
         LogInstance().m_log_sourcelocations = prev_log_sourcelocations;
+        LogInstance().SetLogLevel(prev_log_level);
+        LogInstance().SetCategoryLogLevel(prev_category_levels);
     }
 };
 
@@ -74,6 +93,7 @@ BOOST_AUTO_TEST_CASE(logging_timer)
 
 BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup)
 {
+    LogInstance().m_log_sourcelocations = true;
     LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s", "bar1\n");
     LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::None, "foo2: %s", "bar2\n");
     LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo3: %s", "bar3\n");
@@ -94,9 +114,6 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup)
 
 BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup)
 {
-    // Prevent tests from failing when the line number of the following log calls changes.
-    LogInstance().m_log_sourcelocations = false;
-
     LogPrintf("foo5: %s\n", "bar5");
     LogPrint(BCLog::NET, "foo6: %s\n", "bar6");
     LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "foo7: %s\n", "bar7");
@@ -123,16 +140,14 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup)
 
 BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup)
 {
-    // Prevent tests from failing when the line number of the following log calls changes.
-    LogInstance().m_log_sourcelocations = false;
     LogInstance().EnableCategory(BCLog::LogFlags::ALL);
-    const auto concated_categery_names = LogInstance().LogCategoriesString();
+    const auto concatenated_category_names = LogInstance().LogCategoriesString();
     std::vector<std::pair<BCLog::LogFlags, std::string>> expected_category_names;
-    const auto category_names = SplitString(concated_categery_names, ',');
+    const auto category_names = SplitString(concatenated_category_names, ',');
     for (const auto& category_name : category_names) {
-        BCLog::LogFlags category = BCLog::NONE;
+        BCLog::LogFlags category;
         const auto trimmed_category_name = TrimString(category_name);
-        BOOST_TEST(GetLogCategory(category, trimmed_category_name));
+        BOOST_REQUIRE(GetLogCategory(category, trimmed_category_name));
         expected_category_names.emplace_back(category, trimmed_category_name);
     }
 
@@ -153,4 +168,92 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup)
     BOOST_CHECK_EQUAL_COLLECTIONS(log_lines.begin(), log_lines.end(), expected.begin(), expected.end());
 }
 
+BOOST_FIXTURE_TEST_CASE(logging_SeverityLevels, LogSetup)
+{
+    LogInstance().EnableCategory(BCLog::LogFlags::ALL);
+
+    LogInstance().SetLogLevel(BCLog::Level::Debug);
+    LogInstance().SetCategoryLogLevel(/*category_str=*/"net", /*level_str=*/"info");
+
+    // Global log level
+    LogPrintLevel(BCLog::HTTP, BCLog::Level::Info, "foo1: %s\n", "bar1");
+    LogPrintLevel(BCLog::MEMPOOL, BCLog::Level::Trace, "foo2: %s. This log level is lower than the global one.\n", "bar2");
+    LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Warning, "foo3: %s\n", "bar3");
+    LogPrintLevel(BCLog::RPC, BCLog::Level::Error, "foo4: %s\n", "bar4");
+
+    // Category-specific log level
+    LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "foo5: %s\n", "bar5");
+    LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "foo6: %s. This log level is the same as the global one but lower than the category-specific one, which takes precedence. \n", "bar6");
+    LogPrintLevel(BCLog::NET, BCLog::Level::Error, "foo7: %s\n", "bar7");
+
+    std::vector<std::string> expected = {
+        "[http:info] foo1: bar1",
+        "[validation:warning] foo3: bar3",
+        "[rpc:error] foo4: bar4",
+        "[net:warning] foo5: bar5",
+        "[net:error] foo7: bar7",
+    };
+    std::ifstream file{tmp_log_path};
+    std::vector<std::string> log_lines;
+    for (std::string log; std::getline(file, log);) {
+        log_lines.push_back(log);
+    }
+    BOOST_CHECK_EQUAL_COLLECTIONS(log_lines.begin(), log_lines.end(), expected.begin(), expected.end());
+}
+
+BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup)
+{
+    // Set global log level
+    {
+        ResetLogger();
+        ArgsManager args;
+        args.AddArg("-loglevel", "...", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
+        const char* argv_test[] = {"bitcoind", "-loglevel=debug"};
+        std::string err;
+        BOOST_REQUIRE(args.ParseParameters(2, argv_test, err));
+        init::SetLoggingLevel(args);
+        BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::Level::Debug);
+    }
+
+    // Set category-specific log level
+    {
+        ResetLogger();
+        ArgsManager args;
+        args.AddArg("-loglevel", "...", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
+        const char* argv_test[] = {"bitcoind", "-loglevel=net:trace"};
+        std::string err;
+        BOOST_REQUIRE(args.ParseParameters(2, argv_test, err));
+        init::SetLoggingLevel(args);
+        BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::DEFAULT_LOG_LEVEL);
+
+        const auto& category_levels{LogInstance().CategoryLevels()};
+        const auto net_it{category_levels.find(BCLog::LogFlags::NET)};
+        BOOST_REQUIRE(net_it != category_levels.end());
+        BOOST_CHECK_EQUAL(net_it->second, BCLog::Level::Trace);
+    }
+
+    // Set both global log level and category-specific log level
+    {
+        ResetLogger();
+        ArgsManager args;
+        args.AddArg("-loglevel", "...", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
+        const char* argv_test[] = {"bitcoind", "-loglevel=debug", "-loglevel=net:trace", "-loglevel=http:info"};
+        std::string err;
+        BOOST_REQUIRE(args.ParseParameters(4, argv_test, err));
+        init::SetLoggingLevel(args);
+        BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::Level::Debug);
+
+        const auto& category_levels{LogInstance().CategoryLevels()};
+        BOOST_CHECK_EQUAL(category_levels.size(), 2);
+
+        const auto net_it{category_levels.find(BCLog::LogFlags::NET)};
+        BOOST_CHECK(net_it != category_levels.end());
+        BOOST_CHECK_EQUAL(net_it->second, BCLog::Level::Trace);
+
+        const auto http_it{category_levels.find(BCLog::LogFlags::HTTP)};
+        BOOST_CHECK(http_it != category_levels.end());
+        BOOST_CHECK_EQUAL(http_it->second, BCLog::Level::Info);
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 8a0b03c9d1..c3989a12fe 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -108,6 +108,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
             "-logsourcelocations",
             "-logtimemicros",
             "-logthreadnames",
+            "-loglevel=trace",
             "-debug",
             "-debugexclude=libevent",
             "-debugexclude=leveldb",