feat: Do not print critical errors in stdout (#2468)

Author: mathbunnyru

benchmarks/util/log/LoggerBenchmark.cpp

@@ -45,7 +45,7 @@ struct BenchmarkLoggingInitializer {
     static std::shared_ptr<spdlog::sinks::sink>
     createFileSink(LogService::FileLoggingParams const& params)
     {
-        return LogService::createFileSink(params);
+        return LogService::createFileSink(params, kLOG_FORMAT);
     }
 
     static Logger
@@ -107,7 +107,6 @@ benchmarkConcurrentFileLogging(benchmark::State& state)
                 auto logger = std::make_shared<spdlog::async_logger>(
                     channel, fileSink, spdlog::thread_pool(), spdlog::async_overflow_policy::block
                 );
-                logger->set_pattern(kLOG_FORMAT);
                 spdlog::register_logger(logger);
                 Logger const threadLogger = BenchmarkLoggingInitializer::getLogger(std::move(logger));
 

src/util/log/Logger.cpp

@@ -31,7 +31,10 @@
 #include <spdlog/async.h>
 #include <spdlog/async_logger.h>
 #include <spdlog/common.h>
+#include <spdlog/details/log_msg.h>
+#include <spdlog/formatter.h>
 #include <spdlog/logger.h>
+#include <spdlog/pattern_formatter.h>
 #include <spdlog/sinks/rotating_file_sink.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
 #include <spdlog/spdlog.h>
@@ -41,6 +44,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <filesystem>
+#include <iostream>
 #include <memory>
 #include <optional>
 #include <string>
@@ -120,26 +124,63 @@ getSeverityLevel(std::string_view logLevel)
     std::unreachable();
 }
 
+/**
+ * @brief Custom formatter that filters out critical messages
+ *
+ * This formatter only processes and formats messages with severity level less than critical.
+ * Critical messages will be handled separately.
+ */
+class NonCriticalFormatter : public spdlog::formatter {
+public:
+    NonCriticalFormatter(std::unique_ptr<spdlog::formatter> wrappedFormatter)
+        : wrapped_formatter_(std::move(wrappedFormatter))
+    {
+    }
+
+    void
+    format(spdlog::details::log_msg const& msg, spdlog::memory_buf_t& dest) override
+    {
+        // Only format messages with severity less than critical
+        if (msg.level != spdlog::level::critical) {
+            wrapped_formatter_->format(msg, dest);
+        }
+    }
+
+    std::unique_ptr<formatter>
+    clone() const override
+    {
+        return std::make_unique<NonCriticalFormatter>(wrapped_formatter_->clone());
+    }
+
+private:
+    std::unique_ptr<spdlog::formatter> wrapped_formatter_;
+};
+
 /**
  * @brief Initializes console logging.
  *
  * @param logToConsole A boolean indicating whether to log to console.
+ * @param format A string representing the log format.
  * @return Vector of sinks for console logging.
  */
 static std::vector<spdlog::sink_ptr>
-createConsoleSinks(bool logToConsole)
+createConsoleSinks(bool logToConsole, std::string const& format)
 {
     std::vector<spdlog::sink_ptr> sinks;
 
     if (logToConsole) {
         auto consoleSink = std::make_shared<spdlog::sinks::stdout_color_sink_mt>();
         consoleSink->set_level(spdlog::level::trace);
+        consoleSink->set_formatter(
+            std::make_unique<NonCriticalFormatter>(std::make_unique<spdlog::pattern_formatter>(format))
+        );
         sinks.push_back(std::move(consoleSink));
     }
 
     // Always add stderr sink for fatal logs
     auto stderrSink = std::make_shared<spdlog::sinks::stderr_color_sink_mt>();
     stderrSink->set_level(spdlog::level::critical);
+    stderrSink->set_formatter(std::make_unique<spdlog::pattern_formatter>(format));
     sinks.push_back(std::move(stderrSink));
 
     return sinks;
@@ -153,7 +194,7 @@ createConsoleSinks(bool logToConsole)
  * @return File sink for logging.
  */
 spdlog::sink_ptr
-LogService::createFileSink(FileLoggingParams const& params)
+LogService::createFileSink(FileLoggingParams const& params, std::string const& format)
 {
     std::filesystem::path const dirPath(params.logDir);
     // the below are taken from user in MB, but spdlog needs it to be in bytes
@@ -163,6 +204,7 @@ LogService::createFileSink(FileLoggingParams const& params)
         (dirPath / "clio.log").string(), rotationSize, params.dirMaxFiles
     );
     fileSink->set_level(spdlog::level::trace);
+    fileSink->set_formatter(std::make_unique<spdlog::pattern_formatter>(format));
 
     return fileSink;
 }
@@ -229,11 +271,13 @@ LogService::init(config::ClioConfigDefinition const& config)
     data.isAsync = config.get<bool>("log.is_async");
     data.defaultSeverity = getSeverityLevel(config.get<std::string>("log.level"));
 
+    std::string const format = config.get<std::string>("log.format");
+
     if (data.isAsync) {
         spdlog::init_thread_pool(8192, 1);
     }
 
-    data.allSinks = createConsoleSinks(config.get<bool>("log.enable_console"));
+    data.allSinks = createConsoleSinks(config.get<bool>("log.enable_console"), format);
 
     if (auto const logDir = config.maybeValue<std::string>("log.directory"); logDir.has_value()) {
         std::filesystem::path const dirPath{logDir.value()};
@@ -250,7 +294,7 @@ LogService::init(config::ClioConfigDefinition const& config)
             .rotationSizeMB = config.get<uint32_t>("log.rotation_size"),
             .dirMaxFiles = config.get<uint32_t>("log.directory_max_files"),
         };
-        data.allSinks.push_back(createFileSink(params));
+        data.allSinks.push_back(createFileSink(params, format));
     }
 
     // get min severity per channel, can be overridden using the `log.channels` array
@@ -269,9 +313,6 @@ LogService::init(config::ClioConfigDefinition const& config)
 
     spdlog::set_default_logger(spdlog::get("General"));
 
-    std::string const format = config.get<std::string>("log.format");
-    spdlog::set_pattern(format);
-
     LOG(LogService::info()) << "Default log level = " << toString(data.defaultSeverity);
     return {};
 }

src/util/log/Logger.hpp

@@ -346,7 +346,7 @@ class LogService {
 
     [[nodiscard]]
     static std::shared_ptr<spdlog::sinks::sink>
-    createFileSink(FileLoggingParams const& params);
+    createFileSink(FileLoggingParams const& params, std::string const& format);
 };
 
 };  // namespace util

tests/common/util/LoggerFixtures.cpp

@@ -22,6 +22,7 @@
 #include "util/log/Logger.hpp"
 
 #include <spdlog/common.h>
+#include <spdlog/pattern_formatter.h>
 #include <spdlog/sinks/ostream_sink.h>
 #include <spdlog/spdlog.h>
 
@@ -35,6 +36,7 @@ LoggerFixture::LoggerFixture()
 
     // Create ostream sink for testing
     auto ostreamSink = std::make_shared<spdlog::sinks::ostream_sink_mt>(stream_);
+    ostreamSink->set_formatter(std::make_unique<spdlog::pattern_formatter>("%^%3!l:%n%$ - %v"));
 
     // Create loggers for each channel
     std::ranges::for_each(util::Logger::kCHANNELS, [&ostreamSink](char const* channel) {
@@ -50,8 +52,6 @@ LoggerFixture::LoggerFixture()
     spdlog::register_logger(traceLogger);
 
     spdlog::set_default_logger(spdlog::get("General"));
-
-    spdlog::set_pattern("%^%3!l:%n%$ - %v");
 }
 
 NoLoggerFixture::NoLoggerFixture()

tests/unit/LogServiceInitTests.cpp

@@ -32,6 +32,7 @@
 #include <fmt/format.h>
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
+#include <spdlog/pattern_formatter.h>
 #include <spdlog/sinks/ostream_sink.h>
 #include <spdlog/spdlog.h>
 
@@ -84,6 +85,7 @@ struct LogServiceInitTests : virtual public ::testing::Test {
     replaceSinks()
     {
         auto ostreamSink = std::make_shared<spdlog::sinks::ostream_sink_mt>(stream_);
+        ostreamSink->set_formatter(std::make_unique<spdlog::pattern_formatter>("%^%3!l:%n%$ - %v"));
 
         for (auto const& channel : Logger::kCHANNELS) {
             auto logger = spdlog::get(channel);
@@ -93,8 +95,6 @@ struct LogServiceInitTests : virtual public ::testing::Test {
             logger->sinks().clear();
             logger->sinks().push_back(ostreamSink);
         }
-
-        spdlog::set_pattern("%^%3!l:%n%$ - %v");
     }
 
 private: