From 52a01b2cf2085399a3ede5480fcd1f74c5331b42 Mon Sep 17 00:00:00 2001 From: Philip Miller Date: Thu, 6 Sep 2018 02:47:48 -0400 Subject: [PATCH 1/4] add argument to rotating file sink for rotate_on_open when true, the log file will be rotated when it is opened so the newly constructed file will start off being empty --- include/spdlog/sinks/rotating_file_sink.h | 12 +++++++----- tests/file_log.cpp | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h index 7ed2385a..bd6582cd 100644 --- a/include/spdlog/sinks/rotating_file_sink.h +++ b/include/spdlog/sinks/rotating_file_sink.h @@ -27,12 +27,14 @@ template class rotating_file_sink SPDLOG_FINAL : public base_sink { public: - rotating_file_sink(filename_t base_filename, std::size_t max_size, std::size_t max_files) + rotating_file_sink(filename_t base_filename, std::size_t max_size, std::size_t max_files, bool rotate_on_open=false) : base_filename_(std::move(base_filename)) , max_size_(max_size) , max_files_(max_files) { file_helper_.open(calc_filename(base_filename_, 0)); + if (rotate_on_open) + rotate_(); current_size_ = file_helper_.size(); // expensive. called only once } @@ -130,15 +132,15 @@ using rotating_file_sink_st = rotating_file_sink; template inline std::shared_ptr rotating_logger_mt( - const std::string &logger_name, const filename_t &filename, size_t max_file_size, size_t max_files) + const std::string &logger_name, const filename_t &filename, size_t max_file_size, size_t max_files, bool rotate_on_open=false) { - return Factory::template create(logger_name, filename, max_file_size, max_files); + return Factory::template create(logger_name, filename, max_file_size, max_files, rotate_on_open); } template inline std::shared_ptr rotating_logger_st( - const std::string &logger_name, const filename_t &filename, size_t max_file_size, size_t max_files) + const std::string &logger_name, const filename_t &filename, size_t max_file_size, size_t max_files, bool rotate_on_open = false) { - return Factory::template create(logger_name, filename, max_file_size, max_files); + return Factory::template create(logger_name, filename, max_file_size, max_files, rotate_on_open); } } // namespace spdlog diff --git a/tests/file_log.cpp b/tests/file_log.cpp index ace4c065..dbaab390 100644 --- a/tests/file_log.cpp +++ b/tests/file_log.cpp @@ -60,7 +60,7 @@ TEST_CASE("rotating_file_logger2", "[rotating_logger]]") prepare_logdir(); size_t max_size = 1024 * 10; std::string basename = "logs/rotating_log"; - auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 1); + auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 1, true); for (int i = 0; i < 10; ++i) logger->info("Test message {}", i); From 3925f8fa16d3e08f26485e177456c44ca599e8ed Mon Sep 17 00:00:00 2001 From: Philip Miller Date: Thu, 24 Jan 2019 00:06:15 -0500 Subject: [PATCH 2/4] streamline constructor logic and improve test for rotate_on_open=true --- include/spdlog/sinks/rotating_file_sink.h | 20 ++++++++++++++++---- tests/test_file_logging.cpp | 19 ++++++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h index 0f057ad9..b2f52226 100644 --- a/include/spdlog/sinks/rotating_file_sink.h +++ b/include/spdlog/sinks/rotating_file_sink.h @@ -36,9 +36,14 @@ public: , max_size_(max_size) , max_files_(max_files) { - file_helper_.open(calc_filename(base_filename_, 0)); if (rotate_on_open) - rotate_(); + { + rotate_(false); + } + else + { + file_helper_.open(calc_filename(base_filename_, 0)); + } current_size_ = file_helper_.size(); // expensive. called only once } @@ -90,7 +95,7 @@ private: // log.1.txt -> log.2.txt // log.2.txt -> log.3.txt // log.3.txt -> delete - void rotate_() + void rotate_(bool reopen=true) { using details::os::filename_to_str; file_helper_.close(); @@ -118,7 +123,14 @@ private: } } } - file_helper_.reopen(true); + if (reopen) + { + file_helper_.reopen(true); + } + else + { + file_helper_.open(base_filename_, true); + } } // delete the target if exists, and rename the src file to target diff --git a/tests/test_file_logging.cpp b/tests/test_file_logging.cpp index 463f77e3..7df61f9e 100644 --- a/tests/test_file_logging.cpp +++ b/tests/test_file_logging.cpp @@ -38,7 +38,7 @@ TEST_CASE("flush_on", "[flush_on]]") REQUIRE(count_lines(filename) == 3); } -TEST_CASE("rotating_file_logger1", "[rotating_logger]]") +TEST_CASE("rotating_file_logger", "[rotating_logger]]") { prepare_logdir(); size_t max_size = 1024 * 10; @@ -55,12 +55,25 @@ TEST_CASE("rotating_file_logger1", "[rotating_logger]]") REQUIRE(count_lines(filename) == 10); } -TEST_CASE("rotating_file_logger2", "[rotating_logger]]") +TEST_CASE("rotating_file_logger2", "[rotating_logger2]]") { prepare_logdir(); size_t max_size = 1024 * 10; std::string basename = "logs/rotating_log"; - auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 1, true); + + { + // make an initial logger to create the first output file + auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 2, true); + for (int i = 0; i < 10; ++i) + { + logger->info("Test message {}", i); + } + // drop causes the logger destructor to be called, which is required so the + // next logger can rename the first output file. + spdlog::drop(logger->name()); + } + + auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 2, true); for (int i = 0; i < 10; ++i) { logger->info("Test message {}", i); From e41b92c55a543a7ab4090a679a55c9797ecb501f Mon Sep 17 00:00:00 2001 From: Philip Miller Date: Thu, 24 Jan 2019 00:11:06 -0500 Subject: [PATCH 3/4] fix inadvertent rename to original test function --- tests/test_file_logging.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_file_logging.cpp b/tests/test_file_logging.cpp index 7df61f9e..dd0c8999 100644 --- a/tests/test_file_logging.cpp +++ b/tests/test_file_logging.cpp @@ -38,7 +38,7 @@ TEST_CASE("flush_on", "[flush_on]]") REQUIRE(count_lines(filename) == 3); } -TEST_CASE("rotating_file_logger", "[rotating_logger]]") +TEST_CASE("rotating_file_logger1", "[rotating_logger]]") { prepare_logdir(); size_t max_size = 1024 * 10; @@ -55,7 +55,7 @@ TEST_CASE("rotating_file_logger", "[rotating_logger]]") REQUIRE(count_lines(filename) == 10); } -TEST_CASE("rotating_file_logger2", "[rotating_logger2]]") +TEST_CASE("rotating_file_logger2", "[rotating_logger]]") { prepare_logdir(); size_t max_size = 1024 * 10; From 4f65fcd7b18b9d1aac29191a9f2821934712d843 Mon Sep 17 00:00:00 2001 From: Philip Miller Date: Thu, 24 Jan 2019 09:19:52 -0500 Subject: [PATCH 4/4] remove minor optimization for the sake of simplicity --- include/spdlog/sinks/rotating_file_sink.h | 24 +++++++---------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h index b2f52226..ae0f70f6 100644 --- a/include/spdlog/sinks/rotating_file_sink.h +++ b/include/spdlog/sinks/rotating_file_sink.h @@ -36,15 +36,12 @@ public: , max_size_(max_size) , max_files_(max_files) { - if (rotate_on_open) - { - rotate_(false); - } - else - { - file_helper_.open(calc_filename(base_filename_, 0)); - } + file_helper_.open(calc_filename(base_filename_, 0)); current_size_ = file_helper_.size(); // expensive. called only once + if (rotate_on_open && current_size_ > 0) + { + rotate_(); + } } // calc filename according to index and file extension if exists. @@ -95,7 +92,7 @@ private: // log.1.txt -> log.2.txt // log.2.txt -> log.3.txt // log.3.txt -> delete - void rotate_(bool reopen=true) + void rotate_() { using details::os::filename_to_str; file_helper_.close(); @@ -123,14 +120,7 @@ private: } } } - if (reopen) - { - file_helper_.reopen(true); - } - else - { - file_helper_.open(base_filename_, true); - } + file_helper_.reopen(true); } // delete the target if exists, and rename the src file to target