Merge bitcoin/bitcoin#29798: Logging cleanup

a7432dd6ed logging: clarify -debug and -debugexclude descriptions (Anthony Towns)
74dd33cb0a rpc: make logging method reject "0" category and correct the help text (Vasil Dimov)
8c6f3bf163 logging, refactor: minor encapsulation improvement and use BCLog::NONE instead of 0 (Vasil Dimov)
160706aa38 logging, refactor: make category special cases explicit (Ryan Ofsky)

Pull request description:

  * Move special cases from `LOG_CATEGORIES_BY_STR` to `GetLogCategory()` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1547990373)).

  * Remove `"none"` and `"0"` from RPC `logging` help because that help text was wrong. `"none"` resulted in an error and `"0"` was ignored itself (contrary to what the help text suggested).

  * Remove unused `LOG_CATEGORIES_BY_STR[""]` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1548018694)).

  This is a followup to https://github.com/bitcoin/bitcoin/pull/29419, addressing leftover suggestions + more.

ACKs for top commit:
  LarryRuane:
    ACK a7432dd6ed
  ryanofsky:
    Code review ACK a7432dd6ed. Only changes since last review are removing dead if statement and adding AJ's suggested -debug and -debugexclude help improvements, which look accurate and much more clear.

Tree-SHA512: 41b997b06fccdb4c1d31f57d4752c83caa744cb3280276a337ef4a9b7012a04eb945071db6b8fad24c6a6cf8761f2f800fe6d8f3d8836f5b39c25e4f11c85bf0
This commit is contained in:
Ryan Ofsky 2024-08-04 20:53:52 -04:00
commit 55d19945ef
No known key found for this signature in database
GPG key ID: 46800E30FC748A66
4 changed files with 14 additions and 20 deletions

View file

@ -27,9 +27,9 @@ void AddLoggingArgs(ArgsManager& argsman)
{
argsman.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file (default: %s). Relative paths will be prefixed by a net-specific datadir location. Pass -nodebuglogfile to disable writing the log to a file.", DEFAULT_DEBUGLOGFILE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
argsman.AddArg("-debug=<category>", "Output debug and trace logging (default: -nodebug, supplying <category> is optional). "
"If <category> is not supplied or if <category> = 1, output all debug and trace logging. <category> can be: " + LogInstance().LogCategoriesString() + ". This option can be specified multiple times to output multiple categories.",
"If <category> is not supplied or if <category> is 1 or \"all\", output all debug 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.",
ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
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.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
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);
argsman.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-loglevel=<level>|<category>:<level>", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC. Possible values are %s (default=%s). The following levels are always logged: error, warning, info. 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: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);

View file

@ -168,8 +168,6 @@ bool BCLog::Logger::DefaultShrinkDebugFile() const
}
static const std::map<std::string, BCLog::LogFlags, std::less<>> LOG_CATEGORIES_BY_STR{
{"0", BCLog::NONE},
{"", BCLog::NONE},
{"net", BCLog::NET},
{"tor", BCLog::TOR},
{"mempool", BCLog::MEMPOOL},
@ -201,8 +199,6 @@ static const std::map<std::string, BCLog::LogFlags, std::less<>> LOG_CATEGORIES_
{"txreconciliation", BCLog::TXRECONCILIATION},
{"scan", BCLog::SCAN},
{"txpackages", BCLog::TXPACKAGES},
{"1", BCLog::ALL},
{"all", BCLog::ALL},
};
static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_FLAG{
@ -210,11 +206,8 @@ static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_
[](const auto& in) {
std::unordered_map<BCLog::LogFlags, std::string> out;
for (const auto& [k, v] : in) {
switch (v) {
case BCLog::NONE: out.emplace(BCLog::NONE, ""); break;
case BCLog::ALL: out.emplace(BCLog::ALL, "all"); break;
default: out.emplace(v, k);
}
const bool inserted{out.emplace(v, k).second};
assert(inserted);
}
return out;
}(LOG_CATEGORIES_BY_STR)
@ -222,7 +215,7 @@ static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_
bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str)
{
if (str.empty()) {
if (str.empty() || str == "1" || str == "all") {
flag = BCLog::ALL;
return true;
}
@ -251,8 +244,11 @@ std::string BCLog::Logger::LogLevelToStr(BCLog::Level level)
assert(false);
}
std::string LogCategoryToStr(BCLog::LogFlags category)
static std::string LogCategoryToStr(BCLog::LogFlags category)
{
if (category == BCLog::ALL) {
return "all";
}
auto it = LOG_CATEGORIES_BY_FLAG.find(category);
assert(it != LOG_CATEGORIES_BY_FLAG.end());
return it->second;
@ -278,10 +274,9 @@ static std::optional<BCLog::Level> GetLogLevel(std::string_view level_str)
std::vector<LogCategory> BCLog::Logger::LogCategoriesList() const
{
std::vector<LogCategory> ret;
ret.reserve(LOG_CATEGORIES_BY_STR.size());
for (const auto& [category, flag] : LOG_CATEGORIES_BY_STR) {
if (flag != BCLog::NONE && flag != BCLog::ALL) {
ret.push_back(LogCategory{.category = category, .active = WillLogCategory(flag)});
}
ret.push_back(LogCategory{.category = category, .active = WillLogCategory(flag)});
}
return ret;
}

View file

@ -119,7 +119,7 @@ namespace BCLog {
std::atomic<Level> m_log_level{DEFAULT_LOG_LEVEL};
/** Log categories bitfield. */
std::atomic<uint32_t> m_categories{0};
std::atomic<uint32_t> m_categories{BCLog::NONE};
void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, std::string_view source_file, int source_line, std::string_view logging_function, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const;
@ -132,6 +132,8 @@ namespace BCLog {
void LogPrintStr_(std::string_view str, std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags category, BCLog::Level level)
EXCLUSIVE_LOCKS_REQUIRED(m_cs);
std::string GetLogPrefix(LogFlags category, Level level) const;
public:
bool m_print_to_console = false;
bool m_print_to_file = false;
@ -145,8 +147,6 @@ namespace BCLog {
fs::path m_file_path;
std::atomic<bool> m_reopen_file{false};
std::string GetLogPrefix(LogFlags category, Level level) const;
/** Send a string to the log output */
void LogPrintStr(std::string_view str, std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags category, BCLog::Level level)
EXCLUSIVE_LOCKS_REQUIRED(!m_cs);

View file

@ -221,7 +221,6 @@ static RPCHelpMan 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"
" - \"none\", \"0\" : even if other logging categories are specified, ignore all of them.\n"
,
{
{"include", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to add to debug logging",