logging: Move message formatting to util/log.h

With this change, callers can use util/log.h to emit log messages and do not need to
include the full logging implementation in logging.h.

There's a potential performance impact with this change from an extra
`strprintf` call in log statements where `Logger::WillLogCategoryLevel` returns
true but `Logger::Enabled` returns false. This happens when bitcoind is run
with `-noprinttoconsole -nodebuglogfile` options.

For background, log macro arguments are supposed to be evaluated when
`Logger::WillLogCategoryLevel` returns true, even if log output is not enabled.
Changing this behavior would be reasonable but needs consideration in a
separate PR since not evaluating arguments in log statements has the potential
to change non-logging behavior.

The extra `strprintf` call could have been avoided by expanding this change and
making the `ShouldLog()` function return a tri-state DO_LOG / DO_NOT_LOG /
DO_NOT_LOG_ONLY_EVALUATE_ARGS value instead of a bool, but this complexity did
not seem warranted.

Review with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

src/logging.cpp

@@ -602,3 +602,16 @@ bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::stri
     m_category_log_levels[flag] = level.value();
     return true;
 }
+
+bool util::log::ShouldLog(Category category, Level level)
+{
+    return LogInstance().WillLogCategoryLevel(static_cast<BCLog::LogFlags>(category), level);
+}
+
+void util::log::Log(util::log::Entry entry)
+{
+    BCLog::Logger& logger{LogInstance()};
+    if (logger.Enabled()) {
+        logger.LogPrintStr(std::move(entry.message), std::move(entry.source_loc), static_cast<BCLog::LogFlags>(entry.category), entry.level, entry.should_ratelimit);
+    }
+}

src/logging.h

@@ -9,8 +9,6 @@
 #include <crypto/siphash.h>
 #include <logging/categories.h> // IWYU pragma: export
 #include <threadsafety.h>
-#include <tinyformat.h>
-#include <util/check.h>
 #include <util/fs.h>
 #include <util/log.h> // IWYU pragma: export
 #include <util/string.h>
@@ -22,11 +20,8 @@
 #include <functional>
 #include <list>
 #include <memory>
-#include <mutex>
-#include <source_location>
 #include <string>
 #include <unordered_map>
-#include <unordered_set>
 #include <vector>
 
 static const bool DEFAULT_LOGTIMEMICROS = false;
@@ -304,18 +299,4 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
 /** Return true if str parses as a log category and set the flag */
 bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
 
-template <typename... Args>
-inline void LogPrintFormatInternal(SourceLocation&& source_loc, BCLog::LogFlags flag, BCLog::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
-{
-    if (LogInstance().Enabled()) {
-        std::string log_msg;
-        try {
-            log_msg = tfm::format(fmt, args...);
-        } catch (tinyformat::format_error& fmterr) {
-            log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
-        }
-        LogInstance().LogPrintStr(log_msg, std::move(source_loc), flag, level, should_ratelimit);
-    }
-}
-
 #endif // BITCOIN_LOGGING_H

src/util/log.h

@@ -5,8 +5,13 @@
 #ifndef BITCOIN_UTIL_LOG_H
 #define BITCOIN_UTIL_LOG_H
 
+#include <logging/categories.h> // IWYU pragma: export
+#include <tinyformat.h>
+#include <util/check.h>
+
 #include <cstdint>
 #include <source_location>
+#include <string>
 #include <string_view>
 
 /// Like std::source_location, but allowing to override the function name.
@@ -30,6 +35,9 @@ private:
 };
 
 namespace util::log {
+/** Opaque to util::log; interpreted by consumers (e.g., BCLog::LogFlags). */
+using Category = uint64_t;
+
 enum class Level {
     Trace = 0, // High-volume or detailed logging for development/debugging
     Debug,     // Reasonably noisy logging, but still usable in production
@@ -37,6 +45,21 @@ enum class Level {
     Warning,
     Error,
 };
+
+struct Entry {
+    Category category;
+    Level level;
+    bool should_ratelimit{false}; //!< Hint for consumers if this entry should be ratelimited
+    SourceLocation source_loc;
+    std::string message;
+};
+
+/** Return whether messages with specified category and level should be logged. Applications using
+ * the logging library need to provide this. */
+bool ShouldLog(Category category, Level level);
+
+/** Send message to be logged. Applications using the logging library need to provide this. */
+void Log(Entry entry);
 } // namespace util::log
 
 namespace BCLog {
@@ -44,6 +67,23 @@ namespace BCLog {
 using Level = util::log::Level;
 } // namespace BCLog
 
+template <typename... Args>
+inline void LogPrintFormatInternal(SourceLocation&& source_loc, BCLog::LogFlags flag, BCLog::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
+{
+    std::string log_msg;
+    try {
+        log_msg = tfm::format(fmt, args...);
+    } catch (tinyformat::format_error& fmterr) {
+        log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
+    }
+    util::log::Log(util::log::Entry{
+        .category = flag,
+        .level = level,
+        .should_ratelimit = should_ratelimit,
+        .source_loc = std::move(source_loc),
+        .message = std::move(log_msg)});
+}
+
 // Allow __func__ to be used in any context without warnings:
 // NOLINTNEXTLINE(bugprone-lambda-function-name)
 #define LogPrintLevel_(category, level, should_ratelimit, ...) LogPrintFormatInternal(SourceLocation{__func__}, category, level, should_ratelimit, __VA_ARGS__)
@@ -64,7 +104,7 @@ using Level = util::log::Level;
 // developers or power users who are aware that -debug may cause excessive disk usage due to logging.
 #define detail_LogIfCategoryAndLevelEnabled(category, level, ...)      \
     do {                                                               \
-        if (LogAcceptCategory((category), (level))) {                  \
+        if (util::log::ShouldLog((category), (level))) {               \
             bool rate_limit{level >= BCLog::Level::Info};              \
             Assume(!rate_limit); /*Only called with the levels below*/ \
             LogPrintLevel_(category, level, rate_limit, __VA_ARGS__);  \