I think this RPC would be easier to use and reason about if we split it into a read and a write method, where the write method makes a single change per call:
getlogconfig -> see current log levels (under a levels key, so we can add other runtime log options later without breaking anything)setloglevel <level> -> set global log levelsetloglevel <level> <category> -> set category level
For example, consider bitcoin rpc loglevel zmq=info trace. It's not obvious (and not documented) what the log level of zmq is going to be after this call: info or trace? If every call makes just one change, the user sorts priorities, and we keep the interface easier to understand while limiting implementation complexity and test surface (no more OBJ_NAMED_PARAMS, dynamically generated args, or client conversion entry). Furthermore, it makes it easier to upgrade the RPC later if we need to.
It also aligns much better with the new OpenRPC spec. Params are described independently, so the schema can't express precedence or mutual exclusion between them. With setloglevel, every request that's valid according to the schema has exactly one meaning, and the signature doesn't change whenever a category is added. Splitting read and write also makes side effects obvious from the method name, and lets -rpcwhitelist grant read-only access. The main downside is that the schema no longer lists the valid categories, at least until RPCArg supports enums.
Being able to set multiple levels in a single command is convenient on the CLI, but I think that's a CLI concern rather than an RPC one: RPC clients can just make multiple calls (or use a JSON-RPC batch), and I don't think atomicity is meaningful for logging.
<details>
<summary>git diff on 828a0287fa</summary>
diff --git a/doc/release-notes-35387.md b/doc/release-notes-35387.md
index 131a12f5b5..43cae071cb 100644
--- a/doc/release-notes-35387.md
+++ b/doc/release-notes-35387.md
@@ -13,15 +13,14 @@ Logging
`-loglevel=<category>:debug` being a synonym for `-debug=<category>`, and
`-loglevel=<category>:info` being a synonym for `-debugexclude=<category>`.
-- A new `loglevel` RPC has been added, which provides a superset of
- functionality of the `logging` RPC, and allows enabling `trace` logs as well
- as `debug` logs. Examples:
+- New `getlogconfig` and `setloglevel` RPCs have been added, which provide a
+ superset of functionality of the `logging` RPC, and allow enabling `trace`
+ logs as well as `debug` logs. Examples:
```sh
- bitcoin rpc loglevel # See current log levels
- bitcoin rpc loglevel trace # Set global log level
- bitcoin rpc loglevel debug # Set global log level
- bitcoin rpc loglevel info # Set global log level
- bitcoin rpc loglevel libevent=info # Set per-category log level
- bitcoin rpc loglevel trace net=debug libevent=info # Set global and category levels
+ bitcoin rpc getlogconfig # See current logging configuration
+ bitcoin rpc setloglevel trace # Set global log level
+ bitcoin rpc setloglevel debug # Set global log level
+ bitcoin rpc setloglevel info # Set global log level
+ bitcoin rpc setloglevel info libevent # Set per-category log level
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 98c2557a18..a28543fb3b 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -334,7 +334,6 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "psbtbumpfee", 1, "psbt_version"},
{ "logging", 0, "include" },
{ "logging", 1, "exclude" },
- { "loglevel", 1, "categories" },
{ "disconnectnode", 1, "nodeid" },
{ "gethdkeys", 0, "active_only" },
{ "gethdkeys", 0, "options" },
diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp
index 6de4562495..4c45ed129a 100644
--- a/src/rpc/node.cpp
+++ b/src/rpc/node.cpp
@@ -220,72 +220,80 @@ static void UpdateLogCategories(std::vector<std::pair<BCLog::LogFlags, BCLog::Le
}
}
-static RPCMethod loglevel()
+static UniValue LogLevelsToJSON()
{
- std::vector<RPCArg> category_args;
- for (const auto& cat : LogInstance().LogCategoriesList()) {
category_args.emplace_back(cat.category, RPCArg::Type::STR, RPCArg::Optional::OMITTED,
"log level for the \"" + cat.category + "\" category");
- return RPCMethod{"loglevel",
"Gets and sets per-category log levels.\n"
"When called without arguments, returns all log categories with their current log level.\n"
"When called with arguments, sets the log level for specified categories,\n"
"then returns the updated state of all categories.\n"
"The valid log levels are: " + LogInstance().LogLevelsString() + "\n"
,
{
{"all", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Log level to set for all categories."},
{"categories", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "Per-category log levels.", std::move(category_args)},
},
- return levels;
+}
- +static RPCResult LogLevelsResult(std::string key_name)
+{
- return RPCResult{
RPCResult::Type::OBJ_DYN, std::move(key_name), "keys are the logging categories, values are their current log levels",
{
{RPCResult::Type::STR, "category", "current log level"},
}};
+}- +static RPCMethod getlogconfig()
+{
- return RPCMethod{"getlogconfig",
"Returns the current logging configuration.\n",
{},
RPCResult{
RPCResult::Type::OBJ_DYN, "", "keys are the logging categories, values are their current log levels",
HelpExampleCli("loglevel", "")
+ HelpExampleCli("loglevel", "debug")
+ HelpExampleCli("-named loglevel", "info net=debug")
+ HelpExampleRpc("loglevel", "\"debug\"")
HelpExampleCli("getlogconfig", "")
+ HelpExampleRpc("getlogconfig", "")
},
[](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
{
- std::vector<std::pair<BCLog::LogFlags, BCLog::Level>> changes;
- // Optional positional "level" param.
- if (!request.params[0].isNull()) {
const std::string level_str = request.params[0].get_str();
const auto level = BCLog::Logger::GetLogLevel(level_str);
if (!level || *level > BCLog::Level::Info) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown log level \"" + level_str + "\". Valid values: " + LogInstance().LogLevelsString());
}
changes.emplace_back(BCLog::ALL, *level);
- }
- UniValue result(UniValue::VOBJ);
- result.pushKV("levels", LogLevelsToJSON());
- return result;
+},
- };
+}
- // Named "categories" params: applied in the order they appear in the request.
- // Category names are validated by OBJ_NAMED_PARAMS, so GetLogCategory always succeeds here.
- if (!request.params[1].isNull()) {
const UniValue& cats = request.params[1].get_obj();
for (const std::string& cat : cats.getKeys()) {
const std::string level_str = cats[cat].get_str();
const auto level = BCLog::Logger::GetLogLevel(level_str);
if (!level || *level > BCLog::Level::Info) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown log level \"" + level_str + "\". Valid values: " + LogInstance().LogLevelsString());
}
changes.emplace_back(*BCLog::Logger::GetLogCategory(cat), *level);
}
+static RPCMethod setloglevel()
+{
- return RPCMethod{"setloglevel",
"Sets the log level for a single logging category, or for all categories.\n"
"Setting the log level for all categories also resets all per-category log levels.\n"
"Returns the updated log levels of all categories.\n"
"The valid log levels are: " + LogInstance().LogLevelsString() + "\n"
"The valid logging categories are: " + LogInstance().LogCategoriesString() + "\n"
,
{
{"level", RPCArg::Type::STR, RPCArg::Optional::NO, "Log level to set."},
{"category", RPCArg::Type::STR, RPCArg::Default{"all"}, "Category to set the log level for, or \"all\" for all categories."},
},
LogLevelsResult(""),
RPCExamples{
HelpExampleCli("setloglevel", "debug")
+ HelpExampleCli("setloglevel", "trace net")
+ HelpExampleRpc("setloglevel", "\"trace\", \"net\"")
},
[](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
+{- const auto level{BCLog::Logger::GetLogLevel(self.Argstd::string_view("level"))};
- if (!level || *level > BCLog::Level::Info) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("unknown log level \"%s\". Valid values: %s", self.Arg<std::string_view>("level"), LogInstance().LogLevelsString()));
}
- return LogLevelsToJSON();
},
};
}
@@ -301,7 +309,7 @@ static RPCMethod logging()
"The valid logging categories are: " + LogInstance().LogCategoriesString() + "\n"
"In addition, the following are available as category names with special meanings:\n"
" - "all", "1" : represent all logging categories.\n"
"See also: the \"getlogconfig\" and \"setloglevel\" RPCs, which provide a superset of functionality, allowing trace logs to be enabled in addition to debug logs.\n"
,
{
{"include", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to add to debug logging",
@@ -489,8 +497,9 @@ void RegisterNodeRPCCommands(CRPCTable& t)
{
static const CRPCCommand commands[]{
{"control", &getmemoryinfo},
{"control", &getlogconfig},
{"control", &logging},
{"control", &setloglevel},
{"util", &getindexinfo},
{"hidden", &setmocktime},
{"hidden", &mockscheduler},
diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp
index c81156a164..548e08f5bf 100644
--- a/src/test/fuzz/rpc.cpp
+++ b/src/test/fuzz/rpc.cpp
@@ -135,6 +135,7 @@ const std::vectorstd::string RPC_COMMANDS_SAFE_FOR_FUZZING{
"getdescriptorinfo",
"getdifficulty",
"getindexinfo",- "getlogconfig",
"getmemoryinfo",
"getmempoolancestors",
"getmempooldescendants",
@@ -163,7 +164,6 @@ const std::vectorstd::string RPC_COMMANDS_SAFE_FOR_FUZZING{
"joinpsbts",
"listbanned",
"logging",
- "loglevel",
"mockscheduler",
"ping",
"preciousblock",
@@ -174,6 +174,7 @@ const std::vectorstd::string RPC_COMMANDS_SAFE_FOR_FUZZING{
"scantxoutset",
"sendmsgtopeer", // when no peers are connected, no p2p message is sent
"sendrawtransaction",
- "setloglevel",
"setmocktime",
"setnetworkactive",
"signmessagewithprivkey",
diff --git a/test/functional/feature_logging.py b/test/functional/feature_logging.py
index 9d6ea2be35..1f41f3e0b1 100755
--- a/test/functional/feature_logging.py
+++ b/test/functional/feature_logging.py
@@ -9,7 +9,10 @@ import os
from test_framework.test_framework import BitcoinTestFramework
from test_framework.p2p import P2PInterface
from test_framework.test_node import ErrorMatch
-from test_framework.util import assert_raises_rpc_error
+from test_framework.util import (
- assert_equal,
- assert_raises_rpc_error,
+)
class LoggingTest(BitcoinTestFramework):
@@ -160,66 +163,59 @@ class LoggingTest(BitcoinTestFramework):
p2p.wait_for_verack()
self.nodes[0].disconnect_p2ps()
# Read-only call returns all categories with level strings
levels = self.nodes[0].loglevel()
assert isinstance(levels, dict)
assert all(v in ('trace', 'debug', 'info') for v in levels.values())
# All categories should be at 'info' (disabled) with our clean baseline
assert all(v == 'info' for v in levels.values()), f"Expected all info, got: {levels}"
# Set a single category to trace
result = self.nodes[0].loglevel(net='trace')
assert result['net'] == 'trace'
assert result['http'] == 'info'
result = self.nodes[0].setloglevel('trace', 'net')
assert_equal(result, self.nodes[0].getlogconfig()['levels'])
assert_equal(result['net'], 'trace')
assert_equal(result['http'], 'info')
# Cross-check with logging RPC: net should be active (trace < info), http inactive
assert self.nodes[0].logging()['net']
assert not self.nodes[0].logging()['http']
# Set a single category to debug
result = self.nodes[0].setloglevel(level='debug', category='net')
assert_equal(result['net'], 'debug')
# Set a category back to info (disable it)
result = self.nodes[0].setloglevel('info', 'net')
assert_equal(result['net'], 'info')
assert not self.nodes[0].logging()['net']
# Set all categories to debug, the category defaults to "all"
result = self.nodes[0].setloglevel('debug')
assert all(v == 'debug' for v in result.values()), f"Expected all debug, got: {result}"
assert self.nodes[0].logging()['net']
assert self.nodes[0].logging()['http']
# "all" named argument combined with per-category override
result = self.nodes[0].loglevel(all='info', net='trace')
assert result['net'] == 'trace'
assert result['http'] == 'info'
# Set all categories to trace, then override net back to info
result = self.nodes[0].loglevel(all='trace', net='info')
assert result['net'] == 'info'
assert result['http'] == 'trace'
# Setting the level for all categories resets per-category levels
self.nodes[0].setloglevel('trace', 'net')
result = self.nodes[0].setloglevel('info', 'all')
assert all(v == 'info' for v in result.values()), f"Expected all info, got: {result}"
# Invalid category raises an error
assert_raises_rpc_error(-8, "Unknown named parameter notacategory", self.nodes[0].loglevel, **{'notacategory': 'debug'})
assert_raises_rpc_error(-8, "unknown logging category notacategory", self.nodes[0].setloglevel, 'debug', 'notacategory')
# Invalid level raises an error
assert_raises_rpc_error(-8, "unknown log level", self.nodes[0].setloglevel, 'verbose', 'net')
# getlogconfig and logging RPCs stay consistent
self.nodes[0].setloglevel('info')
self.nodes[0].setloglevel('debug', 'rpc')
logging_result = self.nodes[0].logging()
if name == 'main':
LoggingTest(file).main()
</details>
