From 071206ef5929f34ab45a6b2a62107946ce62d5b5 Mon Sep 17 00:00:00 2001 From: Tamas Florin Date: Thu, 4 Jun 2020 13:38:21 +0300 Subject: [PATCH 1/2] Add support for custom filename calculator in rotating_file_sink. --- include/spdlog/sinks/rotating_file_sink-inl.h | 131 ------------------ include/spdlog/sinks/rotating_file_sink.h | 103 ++++++++++++-- src/file_sinks.cpp | 4 - tests/CMakeLists.txt | 1 + tests/test_daily_logger.cpp | 22 --- tests/test_rotating_logger.cpp | 77 ++++++++++ 6 files changed, 169 insertions(+), 169 deletions(-) delete mode 100644 include/spdlog/sinks/rotating_file_sink-inl.h create mode 100644 tests/test_rotating_logger.cpp diff --git a/include/spdlog/sinks/rotating_file_sink-inl.h b/include/spdlog/sinks/rotating_file_sink-inl.h deleted file mode 100644 index d715ebf3..00000000 --- a/include/spdlog/sinks/rotating_file_sink-inl.h +++ /dev/null @@ -1,131 +0,0 @@ -// Copyright(c) 2015-present, Gabi Melman & spdlog contributors. -// Distributed under the MIT License (http://opensource.org/licenses/MIT) - -#pragma once - -#ifndef SPDLOG_HEADER_ONLY -#include -#endif - -#include - -#include -#include -#include - -#include -#include -#include -#include -#include -#include - -namespace spdlog { -namespace sinks { - -template -SPDLOG_INLINE rotating_file_sink::rotating_file_sink( - filename_t base_filename, std::size_t max_size, std::size_t max_files, bool rotate_on_open) - : base_filename_(std::move(base_filename)) - , max_size_(max_size) - , max_files_(max_files) -{ - 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. -// e.g. calc_filename("logs/mylog.txt, 3) => "logs/mylog.3.txt". -template -SPDLOG_INLINE filename_t rotating_file_sink::calc_filename(const filename_t &filename, std::size_t index) -{ - if (index == 0u) - { - return filename; - } - - filename_t basename, ext; - std::tie(basename, ext) = details::file_helper::split_by_extension(filename); - return fmt::format(SPDLOG_FILENAME_T("{}.{}{}"), basename, index, ext); -} - -template -SPDLOG_INLINE filename_t rotating_file_sink::filename() -{ - std::lock_guard lock(base_sink::mutex_); - return file_helper_.filename(); -} - -template -SPDLOG_INLINE void rotating_file_sink::sink_it_(const details::log_msg &msg) -{ - memory_buf_t formatted; - base_sink::formatter_->format(msg, formatted); - current_size_ += formatted.size(); - if (current_size_ > max_size_) - { - rotate_(); - current_size_ = formatted.size(); - } - file_helper_.write(formatted); -} - -template -SPDLOG_INLINE void rotating_file_sink::flush_() -{ - file_helper_.flush(); -} - -// Rotate files: -// log.txt -> log.1.txt -// log.1.txt -> log.2.txt -// log.2.txt -> log.3.txt -// log.3.txt -> delete -template -SPDLOG_INLINE void rotating_file_sink::rotate_() -{ - using details::os::filename_to_str; - using details::os::path_exists; - file_helper_.close(); - for (auto i = max_files_; i > 0; --i) - { - filename_t src = calc_filename(base_filename_, i - 1); - if (!path_exists(src)) - { - continue; - } - filename_t target = calc_filename(base_filename_, i); - - if (!rename_file_(src, target)) - { - // if failed try again after a small delay. - // this is a workaround to a windows issue, where very high rotation - // rates can cause the rename to fail with permission denied (because of antivirus?). - details::os::sleep_for_millis(100); - if (!rename_file_(src, target)) - { - file_helper_.reopen(true); // truncate the log file anyway to prevent it to grow beyond its limit! - current_size_ = 0; - throw_spdlog_ex("rotating_file_sink: failed renaming " + filename_to_str(src) + " to " + filename_to_str(target), errno); - } - } - } - file_helper_.reopen(true); -} - -// delete the target if exists, and rename the src file to target -// return true on success, false otherwise. -template -SPDLOG_INLINE bool rotating_file_sink::rename_file_(const filename_t &src_filename, const filename_t &target_filename) -{ - // try to delete the target file in case it already exists. - (void)details::os::remove(target_filename); - return details::os::rename(src_filename, target_filename) == 0; -} - -} // namespace sinks -} // namespace spdlog diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h index e1e85a7d..35c1e50a 100644 --- a/include/spdlog/sinks/rotating_file_sink.h +++ b/include/spdlog/sinks/rotating_file_sink.h @@ -15,20 +15,68 @@ namespace spdlog { namespace sinks { +/* + * Generator of rotating log file names in format basename.index.extension + * e.g. calc_filename("logs/mylog.txt, 3) => "logs/mylog.3.txt". + */ +struct rotating_filename_calculator +{ + // Create filename for the form basename.index.extension + static filename_t calc_filename(const filename_t &filename, std::size_t index) + { + if (index == 0u) + { + return filename; + } + + filename_t basename, ext; + std::tie(basename, ext) = details::file_helper::split_by_extension(filename); + return fmt::format(SPDLOG_FILENAME_T("{}.{}{}"), basename, index, ext); + } +}; + // // Rotating file sink based on size // -template +template class rotating_file_sink final : public base_sink { public: - rotating_file_sink(filename_t base_filename, std::size_t max_size, std::size_t max_files, bool rotate_on_open = false); - static filename_t calc_filename(const filename_t &filename, std::size_t index); - filename_t filename(); + 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(FileNameCalc::calc_filename(base_filename_, 0)); + current_size_ = file_helper_.size(); // expensive. called only once + if (rotate_on_open && current_size_ > 0) + { + rotate_(); + } + } + filename_t filename() + { + std::lock_guard lock(base_sink::mutex_); + return file_helper_.filename(); + } protected: - void sink_it_(const details::log_msg &msg) override; - void flush_() override; + void sink_it_(const details::log_msg &msg) override + { + memory_buf_t formatted; + base_sink::formatter_->format(msg, formatted); + current_size_ += formatted.size(); + if (current_size_ > max_size_) + { + rotate_(); + current_size_ = formatted.size(); + } + file_helper_.write(formatted); + } + void flush_() override + { + file_helper_.flush(); + } private: // Rotate files: @@ -36,11 +84,45 @@ private: // log.1.txt -> log.2.txt // log.2.txt -> log.3.txt // log.3.txt -> delete - void rotate_(); + void rotate_() + { + using details::os::filename_to_str; + using details::os::path_exists; + file_helper_.close(); + for (auto i = max_files_; i > 0; --i) + { + filename_t src = FileNameCalc::calc_filename(base_filename_, i - 1); + if (!path_exists(src)) + { + continue; + } + filename_t target = FileNameCalc::calc_filename(base_filename_, i); + + if (!rename_file_(src, target)) + { + // if failed try again after a small delay. + // this is a workaround to a windows issue, where very high rotation + // rates can cause the rename to fail with permission denied (because of antivirus?). + details::os::sleep_for_millis(100); + if (!rename_file_(src, target)) + { + file_helper_.reopen(true); // truncate the log file anyway to prevent it to grow beyond its limit! + current_size_ = 0; + throw_spdlog_ex("rotating_file_sink: failed renaming " + filename_to_str(src) + " to " + filename_to_str(target), errno); + } + } + } + file_helper_.reopen(true); + } // delete the target if exists, and rename the src file to target // return true on success, false otherwise. - bool rename_file_(const filename_t &src_filename, const filename_t &target_filename); + bool rename_file_(const filename_t &src_filename, const filename_t &target_filename) + { + // try to delete the target file in case it already exists. + (void)details::os::remove(target_filename); + return details::os::rename(src_filename, target_filename) == 0; + } filename_t base_filename_; std::size_t max_size_; @@ -49,6 +131,7 @@ private: details::file_helper file_helper_; }; + using rotating_file_sink_mt = rotating_file_sink; using rotating_file_sink_st = rotating_file_sink; @@ -72,7 +155,3 @@ inline std::shared_ptr rotating_logger_st( return Factory::template create(logger_name, filename, max_file_size, max_files, rotate_on_open); } } // namespace spdlog - -#ifdef SPDLOG_HEADER_ONLY -#include "rotating_file_sink-inl.h" -#endif diff --git a/src/file_sinks.cpp b/src/file_sinks.cpp index 5fd7e98a..3d70e837 100644 --- a/src/file_sinks.cpp +++ b/src/file_sinks.cpp @@ -14,7 +14,3 @@ template class SPDLOG_API spdlog::sinks::basic_file_sink; template class SPDLOG_API spdlog::sinks::basic_file_sink; - -#include -template class SPDLOG_API spdlog::sinks::rotating_file_sink; -template class SPDLOG_API spdlog::sinks::rotating_file_sink; \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2dd4958b..f57a98b8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,6 +18,7 @@ set(SPDLOG_UTESTS_SOURCES test_file_helper.cpp test_file_logging.cpp test_daily_logger.cpp + test_rotating_logger.cpp test_misc.cpp test_eventlog.cpp test_pattern_formatter.cpp diff --git a/tests/test_daily_logger.cpp b/tests/test_daily_logger.cpp index a3e70e68..3ccdfe65 100644 --- a/tests/test_daily_logger.cpp +++ b/tests/test_daily_logger.cpp @@ -61,28 +61,6 @@ TEST_CASE("daily_logger with custom calculator", "[daily_logger]") require_message_count(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) diff --git a/tests/test_rotating_logger.cpp b/tests/test_rotating_logger.cpp new file mode 100644 index 00000000..f4c7ee0b --- /dev/null +++ b/tests/test_rotating_logger.cpp @@ -0,0 +1,77 @@ +/* + * This content is released under the MIT License as specified in https://raw.githubusercontent.com/gabime/spdlog/master/LICENSE + */ +#include "includes.h" + +TEST_CASE("rotating_looger with default calculator", "[rotating_logger]") +{ + using sink_type = spdlog::sinks::rotating_file_sink; + + prepare_logdir(); + + std::string filename = "test_logs/rotating_default"; + + auto logger = spdlog::create("logger", filename, 5 * 1024, 1); + for (int i = 0; i < 10; ++i) + { + logger->info("Hello World!", i); + } + logger->flush(); + + require_message_count(filename, 10); +} + +struct custom_rotating_file_name_calculator +{ + static spdlog::filename_t calc_filename(const spdlog::filename_t &filename, std::size_t) + { + spdlog::filename_t basename, ext; + std::tie(basename, ext) = spdlog::details::file_helper::split_by_extension(filename); + return fmt::format(SPDLOG_FILENAME_T("{}_custom{}"), basename, ext); + } +}; + +TEST_CASE("rotating_logger with custom calculator", "[rotating_logger]") +{ + using sink_type = spdlog::sinks::rotating_file_sink; + + prepare_logdir(); + + // calculate filename + std::string basename = "test_logs/rotating"; + spdlog::memory_buf_t w; + fmt::format_to(w, "{}_custom", basename); + + auto logger = spdlog::create("logger", basename, 5 * 1024, 1); + for (int i = 0; i < 10; ++i) + { + logger->info("Hello World!"); + } + + logger->flush(); + + auto filename = fmt::to_string(w); + require_message_count(filename, 10); +} + +/* + * File name calculations + */ + +TEST_CASE("rotating_file_sink::calc_filename1", "[rotating_file_sink]]") +{ + auto filename = spdlog::sinks::rotating_filename_calculator::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_filename_calculator::calc_filename("rotated", 3); + REQUIRE(filename == "rotated.3"); +} + +TEST_CASE("rotating_file_sink::calc_filename3", "[rotating_file_sink]]") +{ + auto filename = spdlog::sinks::rotating_filename_calculator::calc_filename("rotated.txt", 0); + REQUIRE(filename == "rotated.txt"); +} From d5aa8db36f977b6ba90003c13a9475eccdc80033 Mon Sep 17 00:00:00 2001 From: Tamas Florin Date: Sat, 6 Jun 2020 21:08:03 +0300 Subject: [PATCH 2/2] Add missing os include for rotating_file_sink. --- include/spdlog/sinks/rotating_file_sink.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h index 35c1e50a..4a016f44 100644 --- a/include/spdlog/sinks/rotating_file_sink.h +++ b/include/spdlog/sinks/rotating_file_sink.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include