logging: API improvements #34038

pull ajtowns wants to merge 8 commits into bitcoin:master from ajtowns:202512-log-niceties changing 18 files +350 −203
  1. ajtowns commented at 5:56 am on December 10, 2025: contributor

    A few changes:

    • Provide STDLOCK(m_cs) instead of StdLockGuard g(m_cs) for the logging mutexes, so that we can get better warnings from clang if annotations have been missed
    • Allow writing LogInfo(BCLog::NO_RATE_LIMIT, "log message") to avoid rate limiting, rather than having to supply a false argument to LogPrintLevel_(...)
    • Replace the -loglevel parameter with a new -trace parameter that works the same as -debug
    • Change the logging RPC to allow enabling trace debugging, and to differentiate trace vs debug levels
  2. DrahtBot commented at 5:56 am on December 10, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34038.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Approach ACK stickies-v

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #33646 (log: check fclose() results and report safely in logging.cpp by cedwies)
    • #33215 (Fix compatibility with -debuglogfile command-line option by hebasto)
    • #17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
    • #17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
    • #17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
    • #17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. ajtowns force-pushed on Dec 10, 2025
  4. DrahtBot added the label CI failed on Dec 10, 2025
  5. DrahtBot commented at 6:01 am on December 10, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20088899137/job/57632110650 LLM reason (✨ experimental): Lint failure: the RPC code uses a fatal assert (rpc_assert), triggering the linter to fail.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  6. ajtowns commented at 6:04 am on December 10, 2025: contributor
    The reason for the locking change for ShrinkDebugFile is that the following commit is enough for clang to figure out that the LogWarning("couldn't shrink") message would take the lock, and thus that ShrinkDebugFile should be annotated with !m_cs. But thinking about it, it seemed like it would make more sense to hold the lock and pause any attempted logging until the shrinking was finished anyway, so that’s what I did. Because we only shrink on startup, before calling StartLogging, I don’t think it has much real impact.
  7. ajtowns force-pushed on Dec 10, 2025
  8. in src/rpc/node.cpp:247 in 1df56912a2 outdated
    243@@ -235,12 +244,26 @@ static RPCHelpMan logging()
    244                         {
    245                             {"exclude_category", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "the valid logging category"},
    246                         }},
    247+                    {"trace", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to add to trace logging",
    


    stickies-v commented at 11:19 am on December 10, 2025:
    Since we’re doing a potentially breaking change (with -deprecatedrpc) here already, what do you think about renaming “include” to “debug”? Also, the RPC documentation wrt evaluation order needs to be updated a bit to reflect the debug -> trace -> exclude evaluation order.

    ajtowns commented at 9:00 pm on December 10, 2025:
    Seems like a fine idea, but not sure how you’d provide compatibility for include=["net"] with deprecaterpc?

    stickies-v commented at 10:26 pm on December 10, 2025:

    I think this should work?

     0diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
     1index 0693b60557..25c313c552 100644
     2--- a/src/rpc/client.cpp
     3+++ b/src/rpc/client.cpp
     4@@ -325,7 +325,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     5     { "psbtbumpfee", 1, "replaceable"},
     6     { "psbtbumpfee", 1, "outputs"},
     7     { "psbtbumpfee", 1, "original_change_index"},
     8-    { "logging", 0, "include" },
     9+    { "logging", 0, "debug" },
    10     { "logging", 1, "exclude" },
    11     { "logging", 2, "trace" },
    12     { "disconnectnode", 1, "nodeid" },
    13diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp
    14index abca5e69bd..06714bde37 100644
    15--- a/src/rpc/node.cpp
    16+++ b/src/rpc/node.cpp
    17@@ -225,20 +225,22 @@ static void EnableOrDisableLogCategories(UniValue cats, BCLog::Level new_level)
    18 
    19 static RPCHelpMan logging()
    20 {
    21+    const bool use_deprecated = IsDeprecatedRPCEnabled("logging");
    22+    const std::string debug_category_name{use_deprecated ? "include" : "debug"};
    23     return RPCHelpMan{"logging",
    24             "Gets and sets the logging configuration.\n"
    25             "When called without an argument, returns the the status of the supported logging categories.\n"
    26             "When called with arguments, adds or removes categories from debug or trace logging and return the lists above.\n"
    27-            "The arguments are evaluated in order \"include\", \"trace\", \"exclude\".\n"
    28-            "If an item is both included/traced and excluded, it will thus end up being excluded.\n"
    29+            "The arguments are evaluated in order \"" + debug_category_name + "\", \"trace\", \"exclude\".\n"
    30+            "If an item is debug and/or trace but also excluded, it will thus end up being excluded.\n"
    31             "The valid logging categories are: " + LogInstance().LogCategoriesString() + "\n"
    32             "In addition, the following are available as category names with special meanings:\n"
    33             "  - \"all\",  \"1\" : represent all logging categories.\n"
    34             ,
    35                 {
    36-                    {"include", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to add to debug logging",
    37+                    {debug_category_name, RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to add to debug logging",
    38                         {
    39-                            {"include_category", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "the valid logging category"},
    40+                            {"debug_category", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "the debug logging category"},
    41                         }},
    42                     {"exclude", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to remove from debug logging",
    43                         {
    44@@ -249,14 +251,14 @@ static RPCHelpMan logging()
    45                             {"trace_category", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "the valid logging category"},
    46                         }},
    47                 },
    48-                {
    49-                    RPCResult{"If -deprecatedrpc=logging is set",
    50+                {   use_deprecated ?
    51+                    RPCResult{
    52                         RPCResult::Type::OBJ_DYN, "", "keys are the logging categories, and values indicates its status",
    53                         {
    54                             {RPCResult::Type::BOOL, "category", "if being debug logged or not. false:inactive, true:active"},
    55                         }
    56-                    },
    57-                    RPCResult{"Otherwise",
    58+                    } :
    59+                    RPCResult{
    60                         RPCResult::Type::OBJ, "", "",
    61                         {{
    62                             {RPCResult::Type::ARR, "excluded", "excluded categories", {{RPCResult::Type::STR, "", ""}}},
    63@@ -289,7 +291,6 @@ static RPCHelpMan logging()
    64         UpdateHTTPServerLogging(LogInstance().WillLogCategory(BCLog::LIBEVENT));
    65     }
    66 
    67-    bool use_deprecated = IsDeprecatedRPCEnabled("logging");
    68     UniValue result(UniValue::VOBJ);
    69     if (use_deprecated) {
    70         for (const auto& info : LogInstance().LogCategoriesInfo()) {
    71diff --git a/test/functional/rpc_misc.py b/test/functional/rpc_misc.py
    72index fa1de774b7..4dfe13ec99 100755
    73--- a/test/functional/rpc_misc.py
    74+++ b/test/functional/rpc_misc.py
    75@@ -80,10 +80,12 @@ class RpcMiscTest(BitcoinTestFramework):
    76         assert('qt' in node.logging()['trace'])
    77         node.logging(exclude=['qt'])
    78         assert('qt' in node.logging()['excluded'])
    79-        node.logging(include=['qt'])
    80+        node.logging(debug=['qt'])
    81         assert('qt' in node.logging()['debug'])
    82         node.logging(trace=['qt'])
    83         assert('qt' in node.logging()['trace'])
    84+        # "include" is now deprecated in favour of "debug"
    85+        assert_raises_rpc_error(-8, "Unknown named parameter include", node.logging, include="net")
    86 
    87         # Test logging RPC returns the logging categories in alphabetical order.
    88         logging = node.logging()
    89@@ -125,6 +127,10 @@ class RpcMiscTest(BitcoinTestFramework):
    90         # Specifying an unknown index name returns an empty result
    91         assert_equal(node.getindexinfo("foo"), {})
    92 
    93+        # Test that deprecatedrpc="logging" maintains behaviour
    94+        self.restart_node(0, ["-deprecatedrpc=logging"])
    95+        node.logging(include=["net"])
    96+        assert_raises_rpc_error(-8, "Unknown named parameter debug", node.logging, debug="net")
    97 
    98 if __name__ == '__main__':
    99     RpcMiscTest(__file__).main()
    

    ajtowns commented at 3:47 am on December 11, 2025:
    Nice. Left the RPCResult text including both forms of output, as that makes it a little more self-documenting that people can use the deprecatedrpc option if desired. Also left include in the client side so that bitcoin-cli still works with old bitcoinds or bitcoinds running deprecatedrpc=logging.

    stickies-v commented at 10:57 am on December 11, 2025:

    Also left include in the client side so that bitcoin-cli still works with old bitcoinds or bitcoinds

    I’m not sure that’s feasible without also adding a -deprecatedrpc option to cli (unless that already exists), which I don’t think is worth it? Afaik we don’t support disjoint bitcoind and bitcoin-cli versions, and I think bitcoin-cli is more for manual usage anyway at which point just typing “debug” instead of “include” is trivial? The diff I added passes all tests (locally) and also adds functional tests to ensure both the current and the deprecated approach work fine over RPC (i.e. without cli).


    ajtowns commented at 11:56 pm on December 12, 2025:

    Changed it to just support both include or debug as aliases indefinitely. (Can remove the include= alias when the deprecatedrpc option is removed)

    I don’t think it makes sense to ship a bitcoind/bitcoin-cli pair that don’t work together under some configuration param (ie bitcoin-cli -named include="['net']" doesn’t work because cli won’t parse it as json, and debug=["net"] won’t work because you’re running with deprecatedrpc=logging)

  9. fanquake added the label Needs release note on Dec 10, 2025
  10. stickies-v commented at 11:37 am on December 10, 2025: contributor
    Concept ACK. Both the new BCLog::NO_RATE_LIMIT and -loglevel changes make for a more natural and ergonomic interface for both developers and users, and the changes required are limited enough. The breaking RPC change is probably the biggest downside/cost of this PR.
  11. ajtowns force-pushed on Dec 10, 2025
  12. ajtowns force-pushed on Dec 10, 2025
  13. DrahtBot removed the label CI failed on Dec 11, 2025
  14. ajtowns force-pushed on Dec 11, 2025
  15. ajtowns force-pushed on Dec 11, 2025
  16. DrahtBot added the label CI failed on Dec 11, 2025
  17. ajtowns force-pushed on Dec 11, 2025
  18. ajtowns force-pushed on Dec 11, 2025
  19. ajtowns force-pushed on Dec 11, 2025
  20. DrahtBot removed the label CI failed on Dec 11, 2025
  21. DrahtBot added the label Needs rebase on Dec 14, 2025
  22. ajtowns force-pushed on Dec 15, 2025
  23. DrahtBot removed the label Needs rebase on Dec 15, 2025
  24. abo-L commented at 1:52 pm on December 15, 2025: none
    bc1qhhml75wgm3a3303tuxyvhr74jr4jmhhk6qy5um
  25. DrahtBot added the label Needs rebase on Dec 18, 2025
  26. threadsafety: Add STDLOCK() and improve annotation checking for StdMutex
    StdLockGuard did not ensure that the lock it was taking was not already
    held. Add a macro and an annotated StdMutex::CheckNotHeld() function
    to correct that. Renames StdLockGuard to StdMutex::Guard, though that
    should only be used through the STDLOCK macro in order to get better
    thread safety checking.
    7e81a427d7
  27. logging: Protect ShrinkDebugFile by m_cs
    We should not be logging while shrinking the debug file, so make sure
    that's true by using our mutex.
    f3dbad4ec9
  28. logging: Provide BCLog::NO_RATE_LIMIT to avoid rate limits 480c501b18
  29. logging: Rename LogPrintLevel_ into detail_ namespace
    After the previous commit, LogPrintLevel_ is only used to implement
    other macros.
    543742dd42
  30. logging: Move GetLogCategory into Logger class 08a624736a
  31. logging: replace -loglevel arg with -trace
    Previously, trace logging for a category was enabled by saying "-debug=net
    -loglevel=net:trace" and saying "-loglevel=net:trace" (without "-debug"
    or "-debug=net") would silently do nothing. Simplify this so that
    "-debug=net" enables debug logging for net and "-trace=net" enables
    both debug and trace logging for net.
    
    Note that "-nodebug" and "-notrace" only disable previous "-debug=xxx" and
    "-trace=yyy" arguments, so "-debug=mempool -trace=net -nodebug" will cancel
    "-debug=mempool", but will still include both debug and trace logs for net.
    ae20b168ac
  32. rpc: Update logging RPC to support trace level debugging
    Also changes the output format, to distinguish debug/trace
    levels. Provides `-deprecatedrpc=logging` if old ouptut format is needed.
    c6635583e4
  33. ajtowns force-pushed on Dec 20, 2025
  34. [doc] Release notes for user-visible logging changes f565bbce26
  35. DrahtBot added the label CI failed on Dec 20, 2025
  36. DrahtBot removed the label Needs rebase on Dec 20, 2025
  37. DrahtBot removed the label CI failed on Dec 20, 2025
  38. ajtowns commented at 3:06 am on December 21, 2025: contributor
    Rebased on top of other logging changes, added a release note
  39. in src/threadsafety.h:71 in f565bbce26
    66@@ -65,15 +67,19 @@ class LOCKABLE StdMutex : public std::mutex
    67     //! with the ! operator, to indicate that a mutex should not be held.
    68     const StdMutex& operator!() const { return *this; }
    69 #endif // __clang__
    70-};
    71 
    72-// StdLockGuard provides an annotated version of std::lock_guard for us,
    73-// and should only be used when sync.h Mutex/LOCK/etc are not usable.
    


    stickies-v commented at 8:05 am on January 2, 2026:

    and should only be used when sync.h Mutex/LOCK/etc are not usable.

    I think this part of the docstring is still applicable after this PR, should we keep it?

  40. in src/logging.h:360 in f565bbce26
    354@@ -351,8 +355,32 @@ namespace BCLog {
    355         static std::string LogLevelToStr(BCLog::Level level);
    356 
    357         bool DefaultShrinkDebugFile() const;
    358-    };
    359 
    360+        template <bool should_ratelimit=true, typename... Args>
    361+        inline void LogPrintFormatInternal(SourceLocation&& source_loc, BCLog::LogFlags flag, BCLog::Level level, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
    


    stickies-v commented at 8:36 am on January 2, 2026:
    nit: can we multi-line these long signatures?
  41. in src/logging.h:383 in 08a624736a outdated
    377@@ -378,6 +378,9 @@ namespace BCLog {
    378         {
    379             LogPrintFormatInternal<false>(std::move(source_loc), flag, level, std::move(fmt), args...);
    380         }
    381+
    382+        /** Return true if str parses as a log category and set the flag */
    383+        static bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
    


    stickies-v commented at 8:54 am on January 2, 2026:

    in commit 08a624736a: would be nice to get rid of the out-parameter

      0diff --git a/src/logging.cpp b/src/logging.cpp
      1index 494f513865..5765d8f322 100644
      2--- a/src/logging.cpp
      3+++ b/src/logging.cpp
      4@@ -127,10 +127,11 @@ void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
      5 
      6 bool BCLog::Logger::EnableCategory(std::string_view str)
      7 {
      8-    BCLog::LogFlags flag;
      9-    if (!GetLogCategory(flag, str)) return false;
     10-    EnableCategory(flag);
     11-    return true;
     12+    if (auto flag{GetLogCategory(str)}) {
     13+        EnableCategory(*flag);
     14+        return true;
     15+    }
     16+    return false;
     17 }
     18 
     19 void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
     20@@ -140,10 +141,11 @@ void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
     21 
     22 bool BCLog::Logger::DisableCategory(std::string_view str)
     23 {
     24-    BCLog::LogFlags flag;
     25-    if (!GetLogCategory(flag, str)) return false;
     26-    DisableCategory(flag);
     27-    return true;
     28+    if (auto flag{GetLogCategory(str)}) {
     29+        DisableCategory(*flag);
     30+        return true;
     31+    }
     32+    return false;
     33 }
     34 
     35 bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const
     36@@ -216,18 +218,16 @@ static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_
     37     }(LOG_CATEGORIES_BY_STR)
     38 };
     39 
     40-bool BCLog::Logger::GetLogCategory(BCLog::LogFlags& flag, std::string_view str)
     41+std::optional<BCLog::LogFlags> BCLog::Logger::GetLogCategory(std::string_view str)
     42 {
     43     if (str.empty() || str == "1" || str == "all") {
     44-        flag = BCLog::ALL;
     45-        return true;
     46+        return BCLog::ALL;
     47     }
     48     auto it = LOG_CATEGORIES_BY_STR.find(str);
     49     if (it != LOG_CATEGORIES_BY_STR.end()) {
     50-        flag = it->second;
     51-        return true;
     52+        return it->second;
     53     }
     54-    return false;
     55+    return std::nullopt;
     56 }
     57 
     58 std::string BCLog::Logger::LogLevelToStr(BCLog::Level level)
     59@@ -592,13 +592,13 @@ bool BCLog::Logger::SetLogLevel(std::string_view level_str)
     60 
     61 bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::string_view level_str)
     62 {
     63-    BCLog::LogFlags flag;
     64-    if (!GetLogCategory(flag, category_str)) return false;
     65+    auto flag{GetLogCategory(category_str)};
     66+    if (!flag) return false;
     67 
     68     const auto level = GetLogLevel(level_str);
     69     if (!level.has_value() || level.value() > MAX_USER_SETABLE_SEVERITY_LEVEL) return false;
     70 
     71     STDLOCK(m_cs);
     72-    m_category_log_levels[flag] = level.value();
     73+    m_category_log_levels[flag.value()] = level.value();
     74     return true;
     75 }
     76diff --git a/src/logging.h b/src/logging.h
     77index 18c759daa4..d9c02e9e69 100644
     78--- a/src/logging.h
     79+++ b/src/logging.h
     80@@ -380,7 +380,7 @@ namespace BCLog {
     81         }
     82 
     83         /** Return true if str parses as a log category and set the flag */
     84-        static bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
     85+        static std::optional<BCLog::LogFlags> GetLogCategory(std::string_view str);
     86     };
     87 } // namespace BCLog
     88 
     89diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
     90index 0e29fa314d..4de66bcc7a 100644
     91--- a/src/test/logging_tests.cpp
     92+++ b/src/test/logging_tests.cpp
     93@@ -164,10 +164,10 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup)
     94     std::vector<std::pair<BCLog::LogFlags, std::string>> expected_category_names;
     95     const auto category_names = SplitString(concatenated_category_names, ',');
     96     for (const auto& category_name : category_names) {
     97-        BCLog::LogFlags category;
     98         const auto trimmed_category_name = TrimString(category_name);
     99-        BOOST_REQUIRE(BCLog::Logger::GetLogCategory(category, trimmed_category_name));
    100-        expected_category_names.emplace_back(category, trimmed_category_name);
    101+        auto category{BCLog::Logger::GetLogCategory(trimmed_category_name)};
    102+        BOOST_REQUIRE(category);
    103+        expected_category_names.emplace_back(*category, trimmed_category_name);
    104     }
    105 
    106     std::vector<std::string> expected;
    
  42. in src/threadsafety.h:79 in f565bbce26
    82+    public:
    83+        explicit Guard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<StdMutex>(cs) {}
    84+        ~Guard() UNLOCK_FUNCTION() = default;
    85+    };
    86+
    87+    static inline StdMutex& CheckNotHeld(StdMutex& cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; }
    


    stickies-v commented at 9:07 am on January 2, 2026:
    nit: since this fn is public, the name could be a bit confusing since this doesn’t do any runtime checks, and the compile-time checks only work under specific circumstances. Maybe good to move this in detail_ and/or add a brief docstring to avoid misuse?
  43. in src/logging.cpp:123 in f565bbce26
    122@@ -123,6 +123,7 @@ void BCLog::Logger::DisableLogging()
    123 void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
    


    stickies-v commented at 10:16 am on January 2, 2026:
    Should we rename this to DebugCategory now for consistency?
  44. in doc/release-notes-34038.md:7 in f565bbce26
    0@@ -0,0 +1,14 @@
    1+
    2+Logging
    3+=======
    4+
    5+- A new `-trace` option has been added. The only trace logs currently
    6+  supported are detailed logs about wallet sql statements, which can
    7+  be accessed via `-trace=wallet`. The `-loglevel` configuration option
    


    stickies-v commented at 10:19 am on January 2, 2026:
    0  be accessed via `-trace=walletdb`. The `-loglevel` configuration option
    
  45. in src/logging.cpp:126 in f565bbce26
    122@@ -123,6 +123,7 @@ void BCLog::Logger::DisableLogging()
    123 void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
    124 {
    125     m_categories |= flag;
    126+    SetCategoryLogLevel(flag, Level::Debug);
    


    stickies-v commented at 10:39 am on January 2, 2026:
    Shouldn’t DisableCategory unset this, too?
  46. in src/logging.h:341 in f565bbce26
    340+        void EnableCategory(LogFlags flag) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    341+        bool EnableCategory(std::string_view str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    342+        void TraceCategory(LogFlags flag) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    343+        bool TraceCategory(std::string_view str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    344+        void DisableCategory(LogFlags flag) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    345+        bool DisableCategory(std::string_view str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    


    stickies-v commented at 12:40 pm on January 2, 2026:

    I’m a bit unsure about the merit of these functions. I think just using SetCategoryLogLevel is a bit more straightforward, which can also be simplified now that we don’t have -loglevel anymore. I think the string overloads could also be removed, and move input validation/parsing to the edge rather than in logging.

      0diff --git a/src/bench/logging.cpp b/src/bench/logging.cpp
      1index 85e33b035e..c68206fd9b 100644
      2--- a/src/bench/logging.cpp
      3+++ b/src/bench/logging.cpp
      4@@ -18,7 +18,7 @@
      5 static void Logging(benchmark::Bench& bench, const std::vector<const char*>& extra_args, const std::function<void()>& log)
      6 {
      7     // Reset any enabled logging categories from a previous benchmark run.
      8-    LogInstance().DisableCategory(BCLog::LogFlags::ALL);
      9+    LogInstance().SetCategoryLogLevel(BCLog::ALL, BCLog::Level::Info);
     10 
     11     TestingSetup test_setup{
     12         ChainType::REGTEST,
     13diff --git a/src/init/common.cpp b/src/init/common.cpp
     14index eb1b06adf5..99908581ef 100644
     15--- a/src/init/common.cpp
     16+++ b/src/init/common.cpp
     17@@ -69,9 +69,11 @@ util::Result<void> SetLoggingCategories(const ArgsManager& args)
     18     const auto categories_to_process = (last_negated == categories.rend()) ? categories : std::ranges::subrange(last_negated.base(), categories.end());
     19 
     20     for (const auto& cat : categories_to_process) {
     21-        if (!LogInstance().EnableCategory(cat)) {
     22+        BCLog::LogFlags flags;
     23+        if (!BCLog::Logger::GetLogCategory(flags, cat)) {
     24             return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debug", cat)};
     25         }
     26+        LogInstance().SetCategoryLogLevel(flags, BCLog::Level::Debug);
     27     }
     28 
     29     // Special-case: Disregard any debugging categories appearing before -trace=0/none
     30@@ -81,16 +83,20 @@ util::Result<void> SetLoggingCategories(const ArgsManager& args)
     31 
     32     const auto trace_categories_to_process = (trace_negated == trace_categories.rend()) ? trace_categories : std::ranges::subrange(trace_negated.base(), trace_categories.end());
     33     for (const auto& cat : trace_categories_to_process) {
     34-        if (!LogInstance().TraceCategory(cat)) {
     35+        BCLog::LogFlags flags;
     36+        if (!BCLog::Logger::GetLogCategory(flags, cat)) {
     37             return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-trace", cat)};
     38         }
     39+        LogInstance().SetCategoryLogLevel(flags, BCLog::Level::Trace);
     40     }
     41 
     42     // Now remove the logging categories which were explicitly excluded
     43     for (const std::string& cat : args.GetArgs("-debugexclude")) {
     44-        if (!LogInstance().DisableCategory(cat)) {
     45+        BCLog::LogFlags flags;
     46+        if (!BCLog::Logger::GetLogCategory(flags, cat)) {
     47             return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat)};
     48         }
     49+        LogInstance().SetCategoryLogLevel(flags, BCLog::Level::Info);
     50     }
     51     return {};
     52 }
     53diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
     54index 15812baf00..253f101c4e 100644
     55--- a/src/kernel/bitcoinkernel.cpp
     56+++ b/src/kernel/bitcoinkernel.cpp
     57@@ -735,12 +735,12 @@ void btck_logging_set_level_category(btck_LogCategory category, btck_LogLevel le
     58 
     59 void btck_logging_enable_category(btck_LogCategory category)
     60 {
     61-    LogInstance().EnableCategory(get_bclog_flag(category));
     62+    LogInstance().SetCategoryLogLevel(get_bclog_flag(category), BCLog::Level::Debug);
     63 }
     64 
     65 void btck_logging_disable_category(btck_LogCategory category)
     66 {
     67-    LogInstance().DisableCategory(get_bclog_flag(category));
     68+    LogInstance().SetCategoryLogLevel(get_bclog_flag(category), BCLog::Level::Info);
     69 }
     70 
     71 void btck_logging_disable()
     72diff --git a/src/logging.cpp b/src/logging.cpp
     73index 870e666c69..6fa0e22db3 100644
     74--- a/src/logging.cpp
     75+++ b/src/logging.cpp
     76@@ -120,47 +120,6 @@ void BCLog::Logger::DisableLogging()
     77     StartLogging();
     78 }
     79 
     80-void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
     81-{
     82-    m_categories |= flag;
     83-    SetCategoryLogLevel(flag, Level::Debug);
     84-}
     85-
     86-bool BCLog::Logger::EnableCategory(std::string_view str)
     87-{
     88-    BCLog::LogFlags flag;
     89-    if (!GetLogCategory(flag, str)) return false;
     90-    EnableCategory(flag);
     91-    return true;
     92-}
     93-
     94-void BCLog::Logger::TraceCategory(BCLog::LogFlags flag)
     95-{
     96-    m_categories |= flag;
     97-    SetCategoryLogLevel(flag, Level::Trace);
     98-}
     99-
    100-bool BCLog::Logger::TraceCategory(std::string_view str)
    101-{
    102-    BCLog::LogFlags flag;
    103-    if (!GetLogCategory(flag, str)) return false;
    104-    TraceCategory(flag);
    105-    return true;
    106-}
    107-
    108-void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
    109-{
    110-    m_categories &= ~flag;
    111-}
    112-
    113-bool BCLog::Logger::DisableCategory(std::string_view str)
    114-{
    115-    BCLog::LogFlags flag;
    116-    if (!GetLogCategory(flag, str)) return false;
    117-    DisableCategory(flag);
    118-    return true;
    119-}
    120-
    121 bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const
    122 {
    123     return (m_categories.load(std::memory_order_relaxed) & category) != 0;
    124@@ -272,7 +231,7 @@ static std::string LogCategoryToStr(BCLog::LogFlags category)
    125     return it->second;
    126 }
    127 
    128-static std::optional<BCLog::Level> GetLogLevel(std::string_view level_str)
    129+std::optional<BCLog::Level> BCLog::Logger::GetLogLevel(std::string_view level_str)
    130 {
    131     if (level_str == "trace") {
    132         return BCLog::Level::Trace;
    133@@ -625,6 +584,14 @@ bool BCLog::Logger::SetLogLevel(std::string_view level_str)
    134 void BCLog::Logger::SetCategoryLogLevel(BCLog::LogFlags flag, BCLog::Level level)
    135 {
    136     STDLOCK(m_cs);
    137+    Assume(level <= BCLog::Level::Info);
    138+    if (level >= BCLog::Level::Info) {
    139+        m_categories &= ~flag;
    140+        m_category_log_levels.erase(flag);
    141+    } else {
    142+        m_categories |= flag;
    143+    }
    144+
    145     if (flag == LogFlags::ALL) {
    146         SetLogLevel(level);
    147         m_category_log_levels.clear();
    148@@ -632,15 +599,3 @@ void BCLog::Logger::SetCategoryLogLevel(BCLog::LogFlags flag, BCLog::Level level
    149         m_category_log_levels[flag] = level;
    150     }
    151 }
    152-
    153-bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::string_view level_str)
    154-{
    155-    BCLog::LogFlags flag;
    156-    if (!GetLogCategory(flag, category_str)) return false;
    157-
    158-    const auto level = GetLogLevel(level_str);
    159-    if (!level.has_value() || level.value() > MAX_USER_SETABLE_SEVERITY_LEVEL) return false;
    160-
    161-    SetCategoryLogLevel(flag, level.value());
    162-    return true;
    163-}
    164diff --git a/src/logging.h b/src/logging.h
    165index 46ac348ac4..c894288ab4 100644
    166--- a/src/logging.h
    167+++ b/src/logging.h
    168@@ -325,7 +325,6 @@ namespace BCLog {
    169             m_category_log_levels[category] = level;
    170         }
    171         void SetCategoryLogLevel(LogFlags flag, Level level) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    172-        bool SetCategoryLogLevel(std::string_view category_str, std::string_view level_str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    173 
    174         Level LogLevel() const { return m_log_level.load(); }
    175         void SetLogLevel(Level level) { m_log_level = level; }
    176@@ -333,13 +332,6 @@ namespace BCLog {
    177 
    178         CategoryMask GetCategoryMask() const { return m_categories.load(); }
    179 
    180-        void EnableCategory(LogFlags flag) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    181-        bool EnableCategory(std::string_view str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    182-        void TraceCategory(LogFlags flag) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    183-        bool TraceCategory(std::string_view str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    184-        void DisableCategory(LogFlags flag) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    185-        bool DisableCategory(std::string_view str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    186-
    187         bool WillLogCategory(LogFlags category) const;
    188         bool WillLogCategoryLevel(LogFlags category, Level level) const EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
    189 
    190@@ -378,6 +370,7 @@ namespace BCLog {
    191             LogPrintFormatInternal<false>(std::move(source_loc), flag, level, std::move(fmt), args...);
    192         }
    193 
    194+        static std::optional<BCLog::Level> GetLogLevel(std::string_view level_str);
    195         /** Return true if str parses as a log category and set the flag */
    196         static bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
    197     };
    198diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp
    199index 301911fa52..79e8df7bd3 100644
    200--- a/src/rpc/node.cpp
    201+++ b/src/rpc/node.cpp
    202@@ -202,24 +202,11 @@ static void EnableOrDisableLogCategories(UniValue cats, BCLog::Level new_level)
    203     for (unsigned int i = 0; i < cats.size(); ++i) {
    204         std::string cat = cats[i].get_str();
    205 
    206-        bool success{false};
    207-        switch (new_level) {
    208-        case BCLog::Level::Trace:
    209-            success = LogInstance().TraceCategory(cat);
    210-            break;
    211-        case BCLog::Level::Debug:
    212-            success = LogInstance().EnableCategory(cat);
    213-            break;
    214-        case BCLog::Level::Info:
    215-            success = LogInstance().DisableCategory(cat);
    216-            break;
    217-        default:
    218-            NONFATAL_UNREACHABLE();
    219-            break;
    220-        }
    221-        if (!success) {
    222+        BCLog::LogFlags flags;
    223+        if (!BCLog::Logger::GetLogCategory(flags, cat)) {
    224             throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown logging category " + cat);
    225         }
    226+        LogInstance().SetCategoryLogLevel(flags, new_level);
    227     }
    228 }
    229 
    230diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
    231index b06752c1e2..26fadcbdcc 100644
    232--- a/src/test/logging_tests.cpp
    233+++ b/src/test/logging_tests.cpp
    234@@ -95,8 +95,8 @@ struct LogSetup : public BasicTestingSetup {
    235         LogInstance().SetLogLevel(prev_log_level);
    236         LogInstance().SetCategoryLogLevel(prev_category_levels);
    237         LogInstance().SetRateLimiting(nullptr);
    238-        LogInstance().DisableCategory(BCLog::LogFlags::ALL);
    239-        LogInstance().EnableCategory(BCLog::LogFlags{prev_category_mask});
    240+        LogInstance().SetCategoryLogLevel(BCLog::LogFlags::ALL, BCLog::Level::Info);
    241+        LogInstance().SetCategoryLogLevel(BCLog::LogFlags{prev_category_mask}, BCLog::Level::Debug);
    242     }
    243 };
    244 
    245@@ -139,7 +139,7 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintStr, LogSetup)
    246 
    247 BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup)
    248 {
    249-    LogInstance().EnableCategory(BCLog::NET);
    250+    LogInstance().SetCategoryLogLevel(BCLog::NET, BCLog::Level::Debug);
    251     LogTrace(BCLog::NET, "foo6: %s", "bar6"); // not logged
    252     LogDebug(BCLog::NET, "foo7: %s", "bar7");
    253     LogInfo("foo8: %s", "bar8");
    254@@ -157,7 +157,7 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup)
    255 
    256 BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup)
    257 {
    258-    LogInstance().EnableCategory(BCLog::LogFlags::ALL);
    259+    LogInstance().SetCategoryLogLevel(BCLog::ALL, BCLog::Level::Debug);
    260     const auto concatenated_category_names = LogInstance().LogCategoriesString();
    261     std::vector<std::pair<BCLog::LogFlags, std::string>> expected_category_names;
    262     const auto category_names = SplitString(concatenated_category_names, ',');
    263@@ -184,8 +184,8 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup)
    264 BOOST_FIXTURE_TEST_CASE(logging_SeverityLevels, LogSetup)
    265 {
    266     LogInstance().SetLogLevel(BCLog::Level::Debug);
    267-    LogInstance().EnableCategory(BCLog::LogFlags::ALL);
    268-    LogInstance().SetCategoryLogLevel(/*category_str=*/"net", /*level_str=*/"info");
    269+    LogInstance().SetCategoryLogLevel(BCLog::LogFlags::ALL, BCLog::Level::Debug);
    270+    LogInstance().SetCategoryLogLevel(BCLog::LogFlags::NET, BCLog::Level::Info);
    271 
    272     // Global log level
    273     LogInfo("info_%s", 1);
    274@@ -431,8 +431,8 @@ BOOST_FIXTURE_TEST_CASE(logging_filesize_rate_limit, LogSetup)
    275     LogInstance().m_log_timestamps = false;
    276     LogInstance().m_log_sourcelocations = false;
    277     LogInstance().m_log_threadnames = false;
    278-    LogInstance().DisableCategory(BCLog::LogFlags::ALL);
    279-    LogInstance().EnableCategory(BCLog::LogFlags::HTTP);
    280+    LogInstance().SetCategoryLogLevel(BCLog::ALL, BCLog::Level::Info);
    281+    LogInstance().SetCategoryLogLevel(BCLog::HTTP, BCLog::Level::Debug);
    282 
    283     constexpr int64_t line_length{1024};
    284     constexpr int64_t num_lines{10};
    

    stickies-v commented at 8:34 am on January 3, 2026:
    Not implemented in my diff, but I think just using SetCategoryLevel would also make it quite straightforward to deduplicate the debug/trace parsing logic in SetLoggingCategories.
  47. in test/functional/feature_logging.py:99 in f565bbce26
     95+            self.restart_node(0, ['-trace=0', '-debug=http', '-debug=abc', disable_debug_opt, '-debug=rpc', '-debug=net'])
     96+            logging = self.nodes[0].logging()
     97+            assert 'http' in logging['excluded'] and 'http' not in (logging['debug'] + logging['trace'])
     98+            assert 'abc' not in (logging['excluded'] + logging['debug'] + logging['trace'])
     99+            assert 'rpc' in logging['debug'] and 'rpc' not in (logging['excluded'] + logging['trace'])
    100+            assert 'net' in logging['debug'] and 'rpc' not in (logging['excluded'] + logging['trace'])
    


    stickies-v commented at 8:07 am on January 3, 2026:
    0            assert 'net' in logging['debug'] and 'net' not in (logging['excluded'] + logging['trace'])
    
  48. in test/functional/feature_logging.py:115 in f565bbce26
    115-            assert logging['rpc']
    116-            assert logging['net']
    117+            assert 'http' in logging['excluded'] and 'http' not in (logging['debug'] + logging['trace'])
    118+            assert 'abc' not in (logging['excluded'] + logging['debug'] + logging['trace'])
    119+            assert 'rpc' in logging['trace'] and 'rpc' not in (logging['excluded'] + logging['debug'])
    120+            assert 'net' in logging['trace'] and 'rpc' not in (logging['excluded'] + logging['debug'])
    


    stickies-v commented at 8:08 am on January 3, 2026:
    0            assert 'net' in logging['trace'] and 'net' not in (logging['excluded'] + logging['debug'])
    
  49. in src/init/common.cpp:34 in f565bbce26
    33         ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    34-    argsman.AddArg("-debugexclude=<category>", "Exclude debug and trace logging for a category. Can be used in conjunction with -debug=1 to output debug and trace logging for all categories except the specified category. This option can be specified multiple times to exclude multiple categories. This takes priority over \"-debug\"", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    35+    argsman.AddArg("-trace=<category>", "Output trace logging (default: -notrace, supplying <category> is optional). "
    36+        "If <category> is not supplied or if <category> is 1 or \"all\", output all trace logging. If <category> is 0 or \"none\", any other categories are ignored. Other valid values for <category> are: " + LogInstance().LogCategoriesString() + ". This option can be specified multiple times to output multiple categories.",
    37+        ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    38+    argsman.AddArg("-debugexclude=<category>", "Exclude debug and trace logging for a category. Can be used in conjunction with -debug=1 to output debug and trace logging for all categories except the specified category. This option can be specified multiple times to exclude multiple categories. This takes priority over \"-debug\" and \"-trace\".", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    


    stickies-v commented at 8:15 am on January 3, 2026:

    nit

    0    argsman.AddArg("-debugexclude=<category>", "Exclude debug and trace logging for a category. Can be used in conjunction with -debug=1 or -trace=1 to output debug and trace logging for all categories except the specified category. This option can be specified multiple times to exclude multiple categories. This takes priority over \"-debug\" and \"-trace\".", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    
  50. in src/init/common.cpp:77 in f565bbce26
     96+            return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debug", cat)};
     97+        }
     98+    }
     99 
    100-        const auto categories_to_process = (last_negated == categories.rend()) ? categories : std::ranges::subrange(last_negated.base(), categories.end());
    101+    // Special-case: Disregard any debugging categories appearing before -trace=0/none
    


    stickies-v commented at 8:27 am on January 3, 2026:
    0    // Special-case: Disregard any trace categories appearing before -trace=0/none
    
  51. in src/test/logging_tests.cpp:83 in f565bbce26
    79@@ -79,10 +80,7 @@ struct LogSetup : public BasicTestingSetup {
    80         // Prevent tests from failing when the line number of the logs changes.
    81         LogInstance().m_log_sourcelocations = false;
    82 
    83-        LogInstance().SetLogLevel(BCLog::Level::Debug);
    84-        LogInstance().DisableCategory(BCLog::LogFlags::ALL);
    


    stickies-v commented at 8:39 am on January 3, 2026:
    nit: not moving LogInstance().DisableCategory(BCLog::LogFlags::ALL); means we’re no longer resetting the m_categories field, not a big issue but probably better to keep this? (also probably a good indication that the current interface is still a bit wonky)
  52. in src/test/logging_tests.cpp:216 in f565bbce26
    214     {
    215         ResetLogger();
    216         ArgsManager args;
    217-        args.AddArg("-loglevel", "...", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    218-        const char* argv_test[] = {"bitcoind", "-loglevel=debug"};
    219+        args.AddArg("-trace", "...", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
    


    stickies-v commented at 8:46 am on January 3, 2026:
    nit: this line can just be removed, we’re not passing any trace args
  53. in src/test/logging_tests.cpp:271 in f565bbce26
    285-        BOOST_CHECK_EQUAL(http_it->second, BCLog::Level::Info);
    286+        BOOST_CHECK_EQUAL(http_it->second, BCLog::Level::Trace);
    287+
    288+        const auto mempool_it{category_levels.find(BCLog::LogFlags::MEMPOOL)};
    289+        BOOST_CHECK(mempool_it != category_levels.end());
    290+        BOOST_CHECK_EQUAL(mempool_it->second, BCLog::Level::Debug);
    


    stickies-v commented at 8:53 am on January 3, 2026:

    I think it’s also useful to test here that non-set categories don’t show up? e.g.

    0BOOST_CHECK(!category_levels.contains(BCLog::LogFlags::REINDEX));
    
  54. in src/rpc/node.cpp:231 in f565bbce26
    226@@ -218,29 +227,43 @@ static RPCHelpMan logging()
    227 {
    228     return RPCHelpMan{"logging",
    229             "Gets and sets the logging configuration.\n"
    230-            "When called without an argument, returns the list of categories with status that are currently being debug logged or not.\n"
    231-            "When called with arguments, adds or removes categories from debug logging and return the lists above.\n"
    232-            "The arguments are evaluated in order \"include\", \"exclude\".\n"
    233-            "If an item is both included and excluded, it will thus end up being excluded.\n"
    234+            "When called without an argument, returns the status of the supported logging categories.\n"
    235+            "When called with arguments, adds or removes categories from debug or trace logging and returns the lists above.\n"
    


    stickies-v commented at 9:05 am on January 3, 2026:

    nit: “the lists above” is not really clear, would just duplicate from the line above to highlight that the result is the same:

    0            "When called with arguments, adds or removes categories from debug or trace logging and returns the status of the supported logging categories.\n"
    
  55. in test/functional/feature_logging.py:121 in f565bbce26
    121+
    122+            # Check that log values can be changed
    123+            logging = self.nodes[0].logging(["mempool"], ["net"], ["http"])
    124+            assert 'net' in logging['excluded'] and 'net' not in (logging['debug'] + logging['trace'])
    125+            assert 'mempool' in logging['debug'] and 'mempool' not in (logging['excluded'] + logging['trace'])
    126+            assert 'http' in logging['trace'] and 'http' not in (logging['excluded'] + logging['debug'])
    


    stickies-v commented at 9:11 am on January 3, 2026:

    Could be useful to test that previous values aren’t deleted?

    0
    1            # Check that previous settings without conflict are preserved
    2            assert 'rpc' in logging['trace'] and 'rpc' not in (logging['excluded'] + logging['debug'])
    
  56. in src/rpc/node.cpp:262 in f565bbce26
    268+                        }
    269+                    },
    270+                    RPCResult{"Otherwise",
    271+                        RPCResult::Type::OBJ, "", "",
    272+                        {{
    273+                            {RPCResult::Type::ARR, "excluded", "excluded categories", {{RPCResult::Type::STR, "", ""}}},
    


    stickies-v commented at 9:18 am on January 3, 2026:
    “exclude” makes sense when configuring categories, but wouldn’t “disabled” or “inactive” be a more appropriate name for the result?
  57. stickies-v commented at 9:27 am on January 3, 2026: contributor
    Approach ACK, reviewed code f565bbce26e52ffdbe3f8ecacd015d603d4eb05e

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-01-06 21:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me