feat: Optional log rotation (#3016)

This PR adds an option to disable log rotation.

benchmarks/util/log/LoggerBenchmark.cpp

@@ -11,6 +11,7 @@
 #include <barrier>
 #include <chrono>
 #include <cstddef>
+#include <cstdint>
 #include <filesystem>
 #include <memory>
 #include <string>
@@ -24,9 +25,15 @@ static constexpr auto kLOG_FORMAT = "%Y-%m-%d %H:%M:%S.%f %^%3!l:%n%$ - %v";
 
 struct BenchmarkLoggingInitializer {
     [[nodiscard]] static std::shared_ptr<spdlog::sinks::sink>
-    createFileSink(LogService::FileLoggingParams const& params)
+    createFileSink(std::string const& logDir, uint32_t sizeMB, uint32_t maxFiles)
     {
-        return LogService::createFileSink(params, kLOG_FORMAT);
+        return LogService::createFileSink(
+            LogService::FileLoggingParams{
+                .logDir = logDir,
+                .rotation = LogService::RotationParams{.sizeMB = sizeMB, .maxFiles = maxFiles},
+            },
+            kLOG_FORMAT
+        );
     }
 
     static Logger
@@ -68,11 +75,7 @@ benchmarkConcurrentFileLogging(benchmark::State& state)
         static constexpr size_t kTHREAD_COUNT = 1;
         spdlog::init_thread_pool(kQUEUE_SIZE, kTHREAD_COUNT);
 
-        auto fileSink = BenchmarkLoggingInitializer::createFileSink({
-            .logDir = logDir,
-            .rotationSizeMB = 5,
-            .dirMaxFiles = 25,
-        });
+        auto fileSink = BenchmarkLoggingInitializer::createFileSink(logDir, 5, 25);
 
         std::vector<std::thread> threads;
         threads.reserve(numThreads);

docs/config-description.md

@@ -561,6 +561,14 @@ Documentation can be found at: <https://github.com/gabime/spdlog/wiki/Custom-for
 - **Constraints**: The minimum value is `1`. The maximum value is `4294967295`.
 - **Description**: The maximum number of log files in the directory.
 
+### log.rotate
+
+- **Required**: True
+- **Type**: boolean
+- **Default value**: `True`
+- **Constraints**: None
+- **Description**: Enables or disables log file rotation. When disabled, a single log file is used without size-based rotation. Useful when rotation is managed externally (e.g., via logrotate).
+
 ### log.tag_style
 
 - **Required**: True

src/util/config/ConfigDefinition.cpp

@@ -422,6 +422,8 @@ getClioConfig()
          {"log.directory_max_files",
           ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(gValidateUint32)},
 
+         {"log.rotate", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
+
          {"log.tag_style",
           ConfigValue{ConfigType::String}.defaultValue("none").withConstraint(gValidateLogTag)},
 

src/util/config/ConfigDescription.hpp

@@ -357,6 +357,10 @@ Documentation can be found at: <https://github.com/gabime/spdlog/wiki/Custom-for
                     "file starts."},
         KV{.key = "log.directory_max_files",
            .value = "The maximum number of log files in the directory."},
+        KV{.key = "log.rotate",
+           .value = "Enables or disables log file rotation. When disabled, a single log file is "
+                    "used without size-based rotation. Useful when rotation is managed externally "
+                    "(e.g., via logrotate)."},
         KV{.key = "log.tag_style",
            .value = "Log tags are unique identifiers for log messages. `uint`/`int` starts logging "
                     "from 0 and increments, "

src/util/log/Logger.cpp

@@ -17,6 +17,7 @@
 #include <spdlog/formatter.h>
 #include <spdlog/logger.h>
 #include <spdlog/pattern_formatter.h>
+#include <spdlog/sinks/basic_file_sink.h>
 #include <spdlog/sinks/rotating_file_sink.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
 #include <spdlog/spdlog.h>
@@ -184,12 +185,18 @@ spdlog::sink_ptr
 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
-    auto const rotationSize = mbToBytes(params.rotationSizeMB);
+    auto fileSink = [&]() -> std::shared_ptr<spdlog::sinks::sink> {
+        auto const logPath = (dirPath / "clio.log").string();
+        if (params.rotation.has_value()) {
+            // rotation sizes are taken from user in MB, but spdlog needs bytes
+            auto const rotationSize = mbToBytes(params.rotation->sizeMB);
+            return std::make_shared<spdlog::sinks::rotating_file_sink_mt>(
+                logPath, rotationSize, params.rotation->maxFiles
+            );
+        }
+        return std::make_shared<spdlog::sinks::basic_file_sink_mt>(logPath, /*truncate=*/false);
+    }();
 
-    auto fileSink = std::make_shared<spdlog::sinks::rotating_file_sink_mt>(
-        (dirPath / "clio.log").string(), rotationSize, params.dirMaxFiles
-    );
     fileSink->set_level(spdlog::level::trace);
     fileSink->set_formatter(std::make_unique<spdlog::pattern_formatter>(format));
 
@@ -336,10 +343,17 @@ LogService::getSinks(config::ClioConfigDefinition const& config)
             }
         }
 
+        std::optional<RotationParams> rotation = std::nullopt;
+        if (config.get<bool>("log.rotate")) {
+            rotation = RotationParams{
+                .sizeMB = config.get<uint32_t>("log.rotation_size"),
+                .maxFiles = config.get<uint32_t>("log.directory_max_files"),
+            };
+        }
+
         FileLoggingParams const params{
             .logDir = logDir.value(),
-            .rotationSizeMB = config.get<uint32_t>("log.rotation_size"),
-            .dirMaxFiles = config.get<uint32_t>("log.directory_max_files"),
+            .rotation = rotation,
         };
         allSinks.push_back(createFileSink(params, format));
     }

src/util/log/Logger.hpp

@@ -28,6 +28,7 @@ class sink;  // NOLINT(readability-identifier-naming)
 struct BenchmarkLoggingInitializer;
 class LoggerFixture;
 struct LogServiceInitTests;
+struct LogFileRotationTests;
 
 namespace util {
 
@@ -229,6 +230,7 @@ class LogServiceState {
 protected:
     friend struct ::LogServiceInitTests;
     friend class ::LoggerFixture;
+    friend struct ::LogFileRotationTests;
     friend class Logger;
     friend class ::util::impl::OnAssert;
 
@@ -388,11 +390,14 @@ private:
         expected<std::vector<std::shared_ptr<spdlog::sinks::sink>>, std::string>
         getSinks(config::ClioConfigDefinition const& config);
 
+    struct RotationParams {
+        uint32_t sizeMB;
+        uint32_t maxFiles;
+    };
+
     struct FileLoggingParams {
         std::string logDir;
-
-        uint32_t rotationSizeMB;
-        uint32_t dirMaxFiles;
+        std::optional<RotationParams> rotation;  ///< nullopt when rotation is disabled
     };
 
     friend struct ::BenchmarkLoggingInitializer;

tests/unit/util/log/LogServiceInitTests.cpp

@@ -66,6 +66,8 @@ protected:
         {"log.directory_max_files",
          ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(config::gValidateUint32)},
 
+        {"log.rotate", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
+
         {"log.tag_style", ConfigValue{ConfigType::String}.defaultValue("none")},
     };
 
@@ -219,3 +221,21 @@ TEST_F(LogServiceInitTests, LogSizeAndHourRotationCannotBeZero)
         );
     }
 }
+
+TEST_F(LogServiceInitTests, RotateDefaultsToTrue)
+{
+    auto const parsingErrors = config_.parse(ConfigFileJson{boost::json::object{}});
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    EXPECT_TRUE(config_.get<bool>("log.rotate"));
+}
+
+TEST_F(LogServiceInitTests, RotationDisabledConfigParsesSuccessfully)
+{
+    auto const parsingErrors = config_.parse(
+        ConfigFileJson{boost::json::object{{"log", boost::json::object{{"rotate", false}}}}}
+    );
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    EXPECT_FALSE(config_.get<bool>("log.rotate"));
+}

tests/unit/util/log/LoggerTests.cpp

@@ -1,14 +1,30 @@
 #include "util/LoggerFixtures.hpp"
+#include "util/config/Array.hpp"
+#include "util/config/ConfigConstraints.hpp"
+#include "util/config/ConfigDefinition.hpp"
+#include "util/config/ConfigFileJson.hpp"
+#include "util/config/ConfigValue.hpp"
+#include "util/config/Types.hpp"
 #include "util/log/Logger.hpp"
 
+#include <boost/json/object.hpp>
+#include <boost/uuid/random_generator.hpp>
+#include <boost/uuid/uuid_io.hpp>
+#include <fmt/core.h>
+#include <fmt/format.h>
 #include <gtest/gtest.h>
 #include <spdlog/logger.h>
 #include <spdlog/spdlog.h>
 
 #include <cstddef>
+#include <filesystem>
 #include <memory>
 #include <string>
 using namespace util;
+using util::config::Array;
+using util::config::ConfigFileJson;
+using util::config::ConfigType;
+using util::config::ConfigValue;
 
 namespace {
 size_t
@@ -91,3 +107,138 @@ TEST_F(LoggerTest, ManyDynamicLoggers)
 
     ASSERT_EQ(loggersNum(), initialLoggers);
 }
+
+/**
+ * @brief Fixture for testing real log-file rotation behaviour.
+ *
+ * Unlike LoggerTest (which uses LoggerFixture's in-memory buffer), this fixture
+ * initialises the LogService with a real file sink and redirects all spdlog
+ * loggers to that sink so that written messages actually land on disk.
+ */
+struct LogFileRotationTests : ::testing::Test {
+    std::filesystem::path const tmpDir = std::filesystem::temp_directory_path() /
+        fmt::format("clio_log_rotation_tests_{}",
+                    boost::uuids::to_string(boost::uuids::random_generator{}()));
+
+    util::config::ClioConfigDefinition config{
+        {"log.channels.[].channel", Array{ConfigValue{ConfigType::String}}},
+        {"log.channels.[].level", Array{ConfigValue{ConfigType::String}}},
+
+        {"log.level", ConfigValue{ConfigType::String}.defaultValue("info")},
+
+        {"log.format",
+         ConfigValue{ConfigType::String}.defaultValue(R"(%Y-%m-%d %H:%M:%S.%f %^%3!l:%n%$ - %v)")},
+        {"log.is_async", ConfigValue{ConfigType::Boolean}.defaultValue(false)},
+
+        {"log.enable_console", ConfigValue{ConfigType::Boolean}.defaultValue(false)},
+
+        {"log.directory", ConfigValue{ConfigType::String}.optional()},
+
+        {"log.rotation_size",
+         ConfigValue{ConfigType::Integer}.defaultValue(2048).withConstraint(
+             util::config::gValidateUint32
+         )},
+
+        {"log.directory_max_files",
+         ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(
+             util::config::gValidateUint32
+         )},
+
+        {"log.rotate", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
+
+        {"log.tag_style", ConfigValue{ConfigType::String}.defaultValue("none")},
+    };
+
+    LogFileRotationTests()
+    {
+        std::filesystem::remove_all(tmpDir);
+        if (LogServiceState::initialized())
+            LogServiceState::reset();
+    }
+
+    ~LogFileRotationTests() override
+    {
+        if (LogService::initialized())
+            LogService::reset();
+        // Leave state initialised so that subsequent tests can call reset().
+        LogServiceState::init(false, Severity::FTL, {});
+        std::filesystem::remove_all(tmpDir);
+    }
+
+    /**
+     * @brief Initialises LogService with the current config and redirects all
+     * existing spdlog loggers to the newly created file sink.
+     *
+     * LogService::init() skips updating sinks on loggers that already exist in
+     * the spdlog registry. Calling replaceSinks() here ensures every logger
+     * writes to the file sink regardless of prior test state.
+     */
+    void
+    initFileLogging() const
+    {
+        ASSERT_TRUE(LogService::init(config));
+        LogServiceState::replaceSinks(LogServiceState::sinks_);
+    }
+
+    /** @brief Returns the number of regular files in tmpDir_. */
+    [[nodiscard]] std::size_t
+    countLogFiles() const
+    {
+        std::size_t count = 0;
+        for (auto const& entry : std::filesystem::directory_iterator(tmpDir)) {
+            if (entry.is_regular_file())
+                ++count;
+        }
+        return count;
+    }
+};
+
+TEST_F(LogFileRotationTests, RotationDisabledProducesSingleLogFile)
+{
+    auto const parsingErrors = config.parse(
+        ConfigFileJson{boost::json::object{
+            {"log",
+             boost::json::object{
+                 {"directory", tmpDir.string()},
+                 {"rotate", false},
+             }}
+        }}
+    );
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    initFileLogging();
+
+    // Write enough data to trigger rotation if it were enabled (> 1 MB).
+    // Writing at error level flushes immediately because flush_on(err) is set.
+    Logger const log{"General"};
+    std::string const bigMessage(1000, 'x');
+    for (int i = 0; i < 1100; ++i)
+        log.error() << bigMessage;
+
+    EXPECT_EQ(countLogFiles(), 1u);
+}
+
+TEST_F(LogFileRotationTests, RotationEnabledProducesMultipleLogFiles)
+{
+    auto const parsingErrors = config.parse(
+        ConfigFileJson{boost::json::object{
+            {"log",
+             boost::json::object{
+                 {"directory", tmpDir.string()},
+                 {"rotate", true},
+                 {"rotation_size", 1},
+                 {"directory_max_files", 2},
+             }}
+        }}
+    );
+    ASSERT_FALSE(parsingErrors.has_value());
+
+    initFileLogging();
+
+    Logger const log{"General"};
+    std::string const bigMessage(1000, 'x');
+    for (int i = 0; i < 1100; ++i)
+        log.error() << bigMessage;
+
+    EXPECT_GT(countLogFiles(), 1u);
+}