Implemented daily sink rotation #661

include/spdlog/details/os-inl.h

@@ -164,6 +164,11 @@ SPDLOG_INLINE int remove(const filename_t &filename) SPDLOG_NOEXCEPT
 #endif
 }
 
+SPDLOG_INLINE int remove_if_exists(const filename_t &filename) SPDLOG_NOEXCEPT
+{
+    return file_exists(filename) ? remove(filename) : 0;
+}
+
 SPDLOG_INLINE int rename(const filename_t &filename1, const filename_t &filename2) SPDLOG_NOEXCEPT
 {
 #if defined(_WIN32) && defined(SPDLOG_WCHAR_FILENAMES)
@@ -173,7 +178,7 @@ SPDLOG_INLINE int rename(const filename_t &filename1, const filename_t &filename
 #endif
 }
 
-// Return if file exists
+// Return true if file exists
 SPDLOG_INLINE bool file_exists(const filename_t &filename) SPDLOG_NOEXCEPT
 {
 #ifdef _WIN32

include/spdlog/details/os.h

@@ -43,11 +43,16 @@ void prevent_child_fd(FILE *f);
 // fopen_s on non windows for writing
 bool fopen_s(FILE **fp, const filename_t &filename, const filename_t &mode);
 
+// Remove filename. return 0 on success
 int remove(const filename_t &filename) SPDLOG_NOEXCEPT;
 
+// Remove file if exists. return 0 on success
+// Note: Non atomic (might return failure to delete if concurrently deleted by other process/thread)
+int remove_if_exists(const filename_t &filename) SPDLOG_NOEXCEPT;
+
 int rename(const filename_t &filename1, const filename_t &filename2) SPDLOG_NOEXCEPT;
 
-// Return if file exists
+// Return if file exists.
 bool file_exists(const filename_t &filename) SPDLOG_NOEXCEPT;
 
 // Return file size according to open FILE* object

include/spdlog/sinks/daily_file_sink.h

@@ -36,26 +36,38 @@ struct daily_filename_calculator
 };
 
 /*
- * Rotating file sink based on date. rotates at midnight
+ * Rotating file sink based on date.
+ * If truncate != false , the created file will be truncated.
+ * If max_files > 0, retain only the last max_files and delete previous.
  */
 template<typename Mutex, typename FileNameCalc = daily_filename_calculator>
 class daily_file_sink final : public base_sink<Mutex>
 {
 public:
     // create daily file sink which rotates on given time
-    daily_file_sink(filename_t base_filename, int rotation_hour, int rotation_minute, bool truncate = false)
+    daily_file_sink(filename_t base_filename, int rotation_hour, int rotation_minute, bool truncate = false, ushort max_files = 0)
         : base_filename_(std::move(base_filename))
         , rotation_h_(rotation_hour)
         , rotation_m_(rotation_minute)
         , truncate_(truncate)
+        , max_files_(max_files)
+        , filenames_q_()
     {
         if (rotation_hour < 0 || rotation_hour > 23 || rotation_minute < 0 || rotation_minute > 59)
         {
             SPDLOG_THROW(spdlog_ex("daily_file_sink: Invalid rotation time in ctor"));
         }
+
         auto now = log_clock::now();
-        file_helper_.open(FileNameCalc::calc_filename(base_filename_, now_tm(now)), truncate_);
+        auto filename = FileNameCalc::calc_filename(base_filename_, now_tm(now));
+        file_helper_.open(filename, truncate_);
         rotation_tp_ = next_rotation_tp_();
+
+        if (max_files_ > 0)
+        {
+            filenames_q_ = details::circular_q<filename_t>(static_cast<size_t>(max_files_));
+            filenames_q_.push_back(std::move(filename));
+        }
     }
 
     const filename_t &filename() const
@@ -71,14 +83,23 @@ protected:
 #else
         auto time = msg.time;
 #endif
-        if (time >= rotation_tp_)
+
+        bool should_rotate = time >= rotation_tp_;
+        if (should_rotate)
         {
-            file_helper_.open(FileNameCalc::calc_filename(base_filename_, now_tm(time)), truncate_);
+            auto filename = FileNameCalc::calc_filename(base_filename_, now_tm(time));
+            file_helper_.open(filename, truncate_);
             rotation_tp_ = next_rotation_tp_();
         }
         memory_buf_t formatted;
         base_sink<Mutex>::formatter_->format(msg, formatted);
         file_helper_.write(formatted);
+
+        // Do the cleaning ony at the end because it might throw on failure.
+        if (should_rotate && max_files_ > 0)
+        {
+            delete_old_();
+        }
     }
 
     void flush_() override
@@ -108,12 +129,36 @@ private:
         return {rotation_time + std::chrono::hours(24)};
     }
 
+    // Delete the file N rotations ago.
+    // Throw spdlog_ex on failure to delete the old file.
+    void delete_old_()
+    {
+        using details::os::filename_to_str;
+        using details::os::remove_if_exists;
+
+        filename_t current_file = filename();
+        if (filenames_q_.full())
+        {
+            filename_t old_filename;
+            filenames_q_.pop_front(old_filename);
+            bool ok = remove_if_exists(old_filename) == 0;
+            if (!ok)
+            {
+                filenames_q_.push_back(std::move(current_file));
+                throw spdlog_ex("Failed removing daily file " + filename_to_str(old_filename), errno);
+            }
+        }
+        filenames_q_.push_back(std::move(current_file));
+    }
+
     filename_t base_filename_;
     int rotation_h_;
     int rotation_m_;
     log_clock::time_point rotation_tp_;
     details::file_helper file_helper_;
     bool truncate_;
+    ushort max_files_;
+    details::circular_q<filename_t> filenames_q_;
 };
 
 using daily_file_sink_mt = daily_file_sink<std::mutex>;

tests/CMakeLists.txt

@@ -10,6 +10,7 @@ endif()
 set(SPDLOG_UTESTS_SOURCES
     test_file_helper.cpp
     test_file_logging.cpp
+	test_daily_logger.cpp
     test_misc.cpp
     test_pattern_formatter.cpp
     test_async.cpp

tests/test_daily_logger.cpp

@@ -0,0 +1,153 @@
+/*
+ * This content is released under the MIT License as specified in https://raw.githubusercontent.com/gabime/spdlog/master/LICENSE
+ */
+#include "includes.h"
+
+TEST_CASE("daily_logger with dateonly calculator", "[daily_logger]")
+{
+    using sink_type = spdlog::sinks::daily_file_sink<std::mutex, spdlog::sinks::daily_filename_calculator>;
+
+    prepare_logdir();
+
+    // calculate filename (time based)
+    std::string basename = "logs/daily_dateonly";
+    std::tm tm = spdlog::details::os::localtime();
+    spdlog::memory_buf_t w;
+    fmt::format_to(w, "{}_{:04d}-{:02d}-{:02d}", basename, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday);
+
+    auto logger = spdlog::create<sink_type>("logger", basename, 0, 0);
+    for (int i = 0; i < 10; ++i)
+    {
+
+        logger->info("Test message {}", i);
+    }
+    logger->flush();
+
+    auto filename = fmt::to_string(w);
+    REQUIRE(count_lines(filename) == 10);
+}
+
+struct custom_daily_file_name_calculator
+{
+    static spdlog::filename_t calc_filename(const spdlog::filename_t &basename, const tm &now_tm)
+    {
+        spdlog::memory_buf_t w;
+        fmt::format_to(w, "{}{:04d}{:02d}{:02d}", basename, now_tm.tm_year + 1900, now_tm.tm_mon + 1, now_tm.tm_mday);
+        return fmt::to_string(w);
+    }
+};
+
+TEST_CASE("daily_logger with custom calculator", "[daily_logger]")
+{
+    using sink_type = spdlog::sinks::daily_file_sink<std::mutex, custom_daily_file_name_calculator>;
+
+    prepare_logdir();
+
+    // calculate filename (time based)
+    std::string basename = "logs/daily_dateonly";
+    std::tm tm = spdlog::details::os::localtime();
+    spdlog::memory_buf_t w;
+    fmt::format_to(w, "{}{:04d}{:02d}{:02d}", basename, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday);
+
+    auto logger = spdlog::create<sink_type>("logger", basename, 0, 0);
+    for (int i = 0; i < 10; ++i)
+    {
+        logger->info("Test message {}", i);
+    }
+
+    logger->
+
+        flush();
+
+    auto filename = fmt::to_string(w);
+    REQUIRE(count_lines(filename) == 10);
+}
+
+/*
+ * File name calculations
+ */
+
+TEST_CASE("rotating_file_sink::calc_filename1", "[rotating_file_sink]]")
+{
+    auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated.txt", 3);
+    REQUIRE(filename == "rotated.3.txt");
+}
+
+TEST_CASE("rotating_file_sink::calc_filename2", "[rotating_file_sink]]")
+{
+    auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated", 3);
+    REQUIRE(filename == "rotated.3");
+}
+
+TEST_CASE("rotating_file_sink::calc_filename3", "[rotating_file_sink]]")
+{
+    auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated.txt", 0);
+    REQUIRE(filename == "rotated.txt");
+}
+
+// regex supported only from gcc 4.9 and above
+#if defined(_MSC_VER) || !(__GNUC__ <= 4 && __GNUC_MINOR__ < 9)
+
+#include <regex>
+
+TEST_CASE("daily_file_sink::daily_filename_calculator", "[daily_file_sink]]")
+{
+    // daily_YYYY-MM-DD_hh-mm.txt
+    auto filename = spdlog::sinks::daily_filename_calculator::calc_filename("daily.txt", spdlog::details::os::localtime());
+    // date regex based on https://www.regular-expressions.info/dates.html
+    std::regex re(R"(^daily_(19|20)\d\d-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])\.txt$)");
+    std::smatch match;
+    REQUIRE(std::regex_match(filename, match, re));
+}
+#endif
+
+/* Test removal of old files */
+static spdlog::details::log_msg create_msg(std::chrono::seconds offset)
+{
+    using spdlog::log_clock;
+    spdlog::details::log_msg msg{"test", spdlog::level::info, "Hello Message"};
+    msg.time = log_clock::now() + offset;
+    return msg;
+}
+
+static void test_rotate(int days_to_run, ushort max_days, ushort expected_n_files)
+{
+    using spdlog::log_clock;
+    using spdlog::details::log_msg;
+    using spdlog::sinks::daily_file_sink_st;
+    using namespace spdlog::details;
+
+    prepare_logdir();
+
+    std::string basename = "logs/daily_rotate.txt";
+    daily_file_sink_st sink{basename, 2, 30, true, max_days};
+
+    // simulate messages with 24 intervals
+
+    for (int i = 0; i < days_to_run; i++)
+    {
+        auto offset = std::chrono::seconds{24 * 3600 * i};
+        sink.log(create_msg(offset));
+    }
+
+    REQUIRE(count_files("logs") == static_cast<size_t>(expected_n_files));
+}
+
+TEST_CASE("daily_logger rotate", "[daily_file_sink]")
+{
+
+    int days_to_run = 1;
+    test_rotate(days_to_run, 0, 1);
+    test_rotate(days_to_run, 1, 1);
+    test_rotate(days_to_run, 3, 1);
+    test_rotate(days_to_run, 10, 1);
+
+    days_to_run = 10;
+    test_rotate(days_to_run, 0, 10);
+    test_rotate(days_to_run, 1, 1);
+    test_rotate(days_to_run, 3, 3);
+    test_rotate(days_to_run, 9, 9);
+    test_rotate(days_to_run, 10, 10);
+    test_rotate(days_to_run, 11, 10);
+    test_rotate(days_to_run, 20, 10);
+}

tests/test_file_logging.cpp

@@ -93,94 +93,3 @@ TEST_CASE("rotating_file_logger2", "[rotating_logger]]")
     auto filename1 = basename + ".1";
     REQUIRE(get_filesize(filename1) <= max_size);
 }
-
-TEST_CASE("daily_logger with dateonly calculator", "[daily_logger_dateonly]]")
-{
-    using sink_type = spdlog::sinks::daily_file_sink<std::mutex, spdlog::sinks::daily_filename_calculator>;
-
-    prepare_logdir();
-    // calculate filename (time based)
-    std::string basename = "logs/daily_dateonly";
-    std::tm tm = spdlog::details::os::localtime();
-    spdlog::memory_buf_t w;
-    fmt::format_to(w, "{}_{:04d}-{:02d}-{:02d}", basename, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday);
-
-    auto logger = spdlog::create<sink_type>("logger", basename, 0, 0);
-    for (int i = 0; i < 10; ++i)
-    {
-
-        logger->info("Test message {}", i);
-    }
-    logger->flush();
-    auto filename = fmt::to_string(w);
-    REQUIRE(count_lines(filename) == 10);
-}
-
-struct custom_daily_file_name_calculator
-{
-    static spdlog::filename_t calc_filename(const spdlog::filename_t &basename, const tm &now_tm)
-    {
-        spdlog::memory_buf_t w;
-        fmt::format_to(w, "{}{:04d}{:02d}{:02d}", basename, now_tm.tm_year + 1900, now_tm.tm_mon + 1, now_tm.tm_mday);
-        return fmt::to_string(w);
-    }
-};
-
-TEST_CASE("daily_logger with custom calculator", "[daily_logger_custom]]")
-{
-    using sink_type = spdlog::sinks::daily_file_sink<std::mutex, custom_daily_file_name_calculator>;
-
-    prepare_logdir();
-    // calculate filename (time based)
-    std::string basename = "logs/daily_dateonly";
-    std::tm tm = spdlog::details::os::localtime();
-    spdlog::memory_buf_t w;
-    fmt::format_to(w, "{}{:04d}{:02d}{:02d}", basename, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday);
-
-    auto logger = spdlog::create<sink_type>("logger", basename, 0, 0);
-    for (int i = 0; i < 10; ++i)
-    {
-        logger->info("Test message {}", i);
-    }
-
-    logger->flush();
-    auto filename = fmt::to_string(w);
-    REQUIRE(count_lines(filename) == 10);
-}
-
-/*
- * File name calculations
- */
-
-TEST_CASE("rotating_file_sink::calc_filename1", "[rotating_file_sink]]")
-{
-    auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated.txt", 3);
-    REQUIRE(filename == "rotated.3.txt");
-}
-
-TEST_CASE("rotating_file_sink::calc_filename2", "[rotating_file_sink]]")
-{
-    auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated", 3);
-    REQUIRE(filename == "rotated.3");
-}
-
-TEST_CASE("rotating_file_sink::calc_filename3", "[rotating_file_sink]]")
-{
-    auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated.txt", 0);
-    REQUIRE(filename == "rotated.txt");
-}
-
-// regex supported only from gcc 4.9 and above
-#if defined(_MSC_VER) || !(__GNUC__ <= 4 && __GNUC_MINOR__ < 9)
-#include <regex>
-
-TEST_CASE("daily_file_sink::daily_filename_calculator", "[daily_file_sink]]")
-{
-    // daily_YYYY-MM-DD_hh-mm.txt
-    auto filename = spdlog::sinks::daily_filename_calculator::calc_filename("daily.txt", spdlog::details::os::localtime());
-    // date regex based on https://www.regular-expressions.info/dates.html
-    std::regex re(R"(^daily_(19|20)\d\d-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])\.txt$)");
-    std::smatch match;
-    REQUIRE(std::regex_match(filename, match, re));
-}
-#endif

tests/utils.cpp

@@ -1,4 +1,6 @@
 #include "includes.h"
+#include <sys/types.h>
+#include <dirent.h>
 
 void prepare_logdir()
 {
@@ -65,3 +67,27 @@ bool ends_with(std::string const &value, std::string const &ending)
     }
     return std::equal(ending.rbegin(), ending.rend(), value.rbegin());
 }
+
+#ifdef _WIN32
+std::size_t count_files(const std::string &folder) {}
+#else
+// Based on: https://stackoverflow.com/a/2802255/192001
+std::size_t count_files(const std::string &folder)
+{
+    size_t counter = 0;
+    DIR *dp = opendir(folder.c_str());
+    if (dp == nullptr)
+    {
+        throw std::runtime_error("Failed open folder " + folder);
+    }
+
+    struct dirent *ep;
+    while (ep = readdir(dp))
+    {
+        if (ep->d_name[0] != '.')
+            counter++;
+    }
+    (void)closedir(dp);
+    return counter;
+}
+#endif

tests/utils.h

@@ -5,6 +5,8 @@
 
 std::size_t count_lines(const std::string &filename);
 
+std::size_t count_files(const std::string &folder);
+
 void prepare_logdir();
 
 std::string file_contents(const std::string &filename);