diff --git a/src/util/config/ConfigDefinition.cpp b/src/util/config/ConfigDefinition.cpp index a29c616e5..490c4ffef 100644 --- a/src/util/config/ConfigDefinition.cpp +++ b/src/util/config/ConfigDefinition.cpp @@ -420,6 +420,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)}, diff --git a/src/util/config/ConfigDescription.hpp b/src/util/config/ConfigDescription.hpp index 435a2945f..a66b021ef 100644 --- a/src/util/config/ConfigDescription.hpp +++ b/src/util/config/ConfigDescription.hpp @@ -357,6 +357,10 @@ Documentation can be found at: #include #include +#include #include #include #include @@ -184,12 +185,20 @@ 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); + std::shared_ptr fileSink; + + if (params.rotation.has_value()) { + // rotation sizes are taken from user in MB, but spdlog needs bytes + auto const rotationSize = mbToBytes(params.rotation->sizeMB); + fileSink = std::make_shared( + (dirPath / "clio.log").string(), rotationSize, params.rotation->maxFiles + ); + } else { + fileSink = std::make_shared( + (dirPath / "clio.log").string(), /*truncate=*/false + ); + } - auto fileSink = std::make_shared( - (dirPath / "clio.log").string(), rotationSize, params.dirMaxFiles - ); fileSink->set_level(spdlog::level::trace); fileSink->set_formatter(std::make_unique(format)); @@ -336,10 +345,17 @@ LogService::getSinks(config::ClioConfigDefinition const& config) } } + std::optional rotation = std::nullopt; + if (config.get("log.rotate")) { + rotation = RotationParams{ + .sizeMB = config.get("log.rotation_size"), + .maxFiles = config.get("log.directory_max_files"), + }; + } + FileLoggingParams const params{ .logDir = logDir.value(), - .rotationSizeMB = config.get("log.rotation_size"), - .dirMaxFiles = config.get("log.directory_max_files"), + .rotation = rotation, }; allSinks.push_back(createFileSink(params, format)); } diff --git a/src/util/log/Logger.hpp b/src/util/log/Logger.hpp index 7cb177e7d..15ff809cc 100644 --- a/src/util/log/Logger.hpp +++ b/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; @@ -386,13 +388,16 @@ class LogService : public LogServiceState { */ [[nodiscard]] static std:: expected>, std::string> - getSinks(config::ClioConfigDefinition const& config); + tSinks(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 rotation; ///< nullopt when rotation is disabled }; friend struct ::BenchmarkLoggingInitializer; diff --git a/tests/unit/util/log/LogServiceInitTests.cpp b/tests/unit/util/log/LogServiceInitTests.cpp index 23af73e82..b73086aa6 100644 --- a/tests/unit/util/log/LogServiceInitTests.cpp +++ b/tests/unit/util/log/LogServiceInitTests.cpp @@ -66,6 +66,8 @@ struct LogServiceInitTests : virtual public LoggerFixture { {"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,22 @@ 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("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("log.rotate")); +} + diff --git a/tests/unit/util/log/LoggerTests.cpp b/tests/unit/util/log/LoggerTests.cpp index 658fc1d82..57195d06f 100644 --- a/tests/unit/util/log/LoggerTests.cpp +++ b/tests/unit/util/log/LoggerTests.cpp @@ -1,14 +1,26 @@ #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 #include #include #include #include +#include #include #include using namespace util; +using util::config::Array; +using util::config::ConfigFileJson; +using util::config::ConfigType; +using util::config::ConfigValue; namespace { size_t @@ -91,3 +103,135 @@ 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() / "clio_log_rotation_tests"; + + 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) +{ + // rotation_size=1 (MB) is the minimum allowed value; writing > 1 MB triggers rotation. + 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); +}