From 494cc8bace26a49f9cd38a3af49e32c8d6c537bd Mon Sep 17 00:00:00 2001
From: gabime <gmelman1@gmail.com>
Date: Sun, 15 Sep 2019 18:34:29 +0300
Subject: [PATCH] Implemented daily sink rotation #661
---
include/spdlog/details/os-inl.h | 7 +-
include/spdlog/details/os.h | 7 +-
include/spdlog/sinks/daily_file_sink.h | 55 ++++++++-
tests/CMakeLists.txt | 1 +
tests/test_daily_logger.cpp | 153 +++++++++++++++++++++++++
tests/test_file_logging.cpp | 91 ---------------
tests/utils.cpp | 26 +++++
tests/utils.h | 2 +
8 files changed, 244 insertions(+), 98 deletions(-)
create mode 100644 tests/test_daily_logger.cpp
diff --git a/include/spdlog/details/os-inl.h b/include/spdlog/details/os-inl.h
index b40a5391..b4f43956 100644
--- a/include/spdlog/details/os-inl.h
+++ b/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
diff --git a/include/spdlog/details/os.h b/include/spdlog/details/os.h
index 46f523f9..75cc7c9c 100644
--- a/include/spdlog/details/os.h
+++ b/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
diff --git a/include/spdlog/sinks/daily_file_sink.h b/include/spdlog/sinks/daily_file_sink.h
index 6f80af40..5b15302c 100644
--- a/include/spdlog/sinks/daily_file_sink.h
+++ b/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>;
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 6a1fe138..1cf8f805 100644
--- a/tests/CMakeLists.txt
+++ b/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
diff --git a/tests/test_daily_logger.cpp b/tests/test_daily_logger.cpp
new file mode 100644
index 00000000..afa1c02f
--- /dev/null
+++ b/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);
+}
\ No newline at end of file
diff --git a/tests/test_file_logging.cpp b/tests/test_file_logging.cpp
index 357d4b87..7563cf65 100644
--- a/tests/test_file_logging.cpp
+++ b/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
diff --git a/tests/utils.cpp b/tests/utils.cpp
index 6bb15872..0b07e08c 100644
--- a/tests/utils.cpp
+++ b/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
diff --git a/tests/utils.h b/tests/utils.h
index e788b507..de553a9c 100644
--- a/tests/utils.h
+++ b/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);