From d492c3646e478c8d84ece22a1d16dc8cab7895a8 Mon Sep 17 00:00:00 2001 From: Jakub Pyszczak Date: Thu, 30 Sep 2021 10:54:02 +0200 Subject: [PATCH] [EGD-7548] Crash dump rotate Added dumps rotation. Maximum count of the files set to 5. The oldest dumps are removed if there're more than 5 of them. Comparison is taken by the date. Unified log and crash dumps rotators. --- board/rt1051/CMakeLists.txt | 2 + .../rt1051/crashdump/crashdumpwriter_vfs.cpp | 17 ++-- .../rt1051/crashdump/crashdumpwriter_vfs.hpp | 14 +-- module-utils/CMakeLists.txt | 1 + module-utils/log/CMakeLists.txt | 2 +- module-utils/log/Logger.cpp | 45 +-------- module-utils/log/Logger.hpp | 8 +- module-utils/log/tests/CMakeLists.txt | 1 + module-utils/log/tests/test_logDumps.cpp | 6 +- module-utils/rotator/CMakeLists.txt | 15 +++ .../rotator/include/rotator/Rotator.hpp | 82 ++++++++++++++++ module-utils/rotator/tests/CMakeLists.txt | 9 ++ module-utils/rotator/tests/test_Rotator.cpp | 94 +++++++++++++++++++ 13 files changed, 228 insertions(+), 68 deletions(-) create mode 100644 module-utils/rotator/CMakeLists.txt create mode 100644 module-utils/rotator/include/rotator/Rotator.hpp create mode 100644 module-utils/rotator/tests/CMakeLists.txt create mode 100644 module-utils/rotator/tests/test_Rotator.cpp diff --git a/board/rt1051/CMakeLists.txt b/board/rt1051/CMakeLists.txt index 5ee3c2b387f1ef2a2e9c1f9af1eb9c3082b5013c..6a0bc7383b00bc313c9dcf8baed7bc7e38dfa8af 100644 --- a/board/rt1051/CMakeLists.txt +++ b/board/rt1051/CMakeLists.txt @@ -30,6 +30,8 @@ target_include_directories(board ) target_link_libraries(board + PRIVATE + utils-rotator PUBLIC fsl module-vfs diff --git a/board/rt1051/crashdump/crashdumpwriter_vfs.cpp b/board/rt1051/crashdump/crashdumpwriter_vfs.cpp index c41f987a338fa49f60706181971639dc98a155d9..84d34e569ecfebb45c6f84ba0dcb7240df6f53f2 100644 --- a/board/rt1051/crashdump/crashdumpwriter_vfs.cpp +++ b/board/rt1051/crashdump/crashdumpwriter_vfs.cpp @@ -8,21 +8,24 @@ #include "purefs/vfs_subsystem.hpp" #include +#include + namespace crashdump { + constexpr inline auto crashDumpFileName = "crashdump.hex"; + void CrashDumpWriterVFS::openDump() { - std::array crashDumpFileName{0}; - formatCrashDumpFileName(crashDumpFileName); + vfs = purefs::subsystem::vfs_core(); + const auto crashDumpFilePath = purefs::dir::getCrashDumpsPath() / crashDumpFileName; - const auto crashDumpFilePath = purefs::dir::getCrashDumpsPath() / crashDumpFileName.data(); - - vfs = purefs::subsystem::vfs_core(); + if (!rotator.rotateFile(crashDumpFilePath)) { + LOG_FATAL("Failed to rotate crash dumps."); + } dumpFd = vfs->open(crashDumpFilePath.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0666); if (dumpFd < 0) { - LOG_FATAL("Failed to open crash dump file [%s]. Won't be able to save crash info.", - crashDumpFilePath.c_str()); + LOG_FATAL("Failed to open crash dump file. Won't be able to save crash info."); } } diff --git a/board/rt1051/crashdump/crashdumpwriter_vfs.hpp b/board/rt1051/crashdump/crashdumpwriter_vfs.hpp index f1f55a6b801790d8f296dad49fed9aca3cc68b30..a2784049b63bb16f3a3ec1e4a410b8d361c2c942 100644 --- a/board/rt1051/crashdump/crashdumpwriter_vfs.hpp +++ b/board/rt1051/crashdump/crashdumpwriter_vfs.hpp @@ -4,6 +4,7 @@ #pragma once #include "crashdumpwriter.hpp" +#include #include #include @@ -16,11 +17,12 @@ namespace purefs::fs namespace crashdump { - constexpr inline auto CrashDumpFileNameFormat = "crashdump-%FT%TZ.hex"; - + constexpr inline auto maxRotationFilesCount = 5; class CrashDumpWriterVFS : public CrashDumpWriter { public: + CrashDumpWriterVFS() : rotator{".hex"} + {} void openDump() override; void saveDump() override; @@ -29,13 +31,7 @@ namespace crashdump void writeWords(const std::uint32_t *buff, std::size_t size) override; private: - template void formatCrashDumpFileName(std::array &name) - { - std::time_t now; - std::time(&now); - std::strftime(name.data(), name.size(), CrashDumpFileNameFormat, std::localtime(&now)); - } - + utils::Rotator rotator; int dumpFd{-1}; std::shared_ptr vfs; diff --git a/module-utils/CMakeLists.txt b/module-utils/CMakeLists.txt index 687ff506a5b6b8293448ae2793c898848e484565..8b596833cb93fb1aaa28feaf13f91aca21b12086 100644 --- a/module-utils/CMakeLists.txt +++ b/module-utils/CMakeLists.txt @@ -11,6 +11,7 @@ add_subdirectory(locale) add_subdirectory(log) add_subdirectory(math) add_subdirectory(phonenumber) +add_subdirectory(rotator) add_subdirectory(rrule) add_subdirectory(time) add_subdirectory(unicode) diff --git a/module-utils/log/CMakeLists.txt b/module-utils/log/CMakeLists.txt index c61a9cefc01476ad8cfe6ede90773b3aef5cdbc1..28fbde8a31cdeb82765d43059073b1df3f4c3151 100644 --- a/module-utils/log/CMakeLists.txt +++ b/module-utils/log/CMakeLists.txt @@ -17,10 +17,10 @@ target_link_libraries(log PRIVATE utility purefs-paths - PUBLIC module-os log-api + utils-rotator ) if (${ENABLE_TESTS}) diff --git a/module-utils/log/Logger.cpp b/module-utils/log/Logger.cpp index 62456f4e06b6c8292f71a3fbc7d086777cef7d51..b3d4047f64d40ba881bd2f5f51d211a0a3831888 100644 --- a/module-utils/log/Logger.cpp +++ b/module-utils/log/Logger.cpp @@ -12,21 +12,6 @@ namespace Log { - namespace - { - std::string getRotatedLogFileExtension(int count) - { - return ".log." + utils::to_string(count); - } - - std::filesystem::path getRotatedFilePath(const std::filesystem::path &source, int rotationCount) - { - auto path = source; - path.replace_extension(getRotatedLogFileExtension(rotationCount)); - return path; - } - } // namespace - std::map Logger::filtered = {{"ApplicationManager", logger_level::LOGINFO}, {"CellularMux", logger_level::LOGINFO}, {"ServiceCellular", logger_level::LOGINFO}, @@ -47,7 +32,7 @@ namespace Log return stream; } - Logger::Logger() : circularBuffer(circularBufferSize) + Logger::Logger() : circularBuffer{circularBufferSize}, rotator{".log"} {} void Logger::enableColors(bool enable) @@ -81,11 +66,10 @@ namespace Log return logs; } - void Logger::init(Application app, size_t fileSize, int filesCount) + void Logger::init(Application app, size_t fileSize) { application = std::move(app); maxFileSize = fileSize; - maxRotationIndex = filesCount - 1; #if LOG_USE_COLOR == 1 enableColors(true); #else @@ -158,7 +142,7 @@ namespace Log LOG_DEBUG("Max log file size exceeded. Rotating log files..."); { LockGuard lock(logFileMutex); - rotateLogFile(logPath); + rotator.rotateFile(logPath); } firstDump = true; } @@ -187,29 +171,6 @@ namespace Log return status; } - void Logger::rotateLogFile(const std::filesystem::path &logPath) - { - for (int i = currentRotationIndex; i > 0; --i) { - std::filesystem::path src = getRotatedFilePath(logPath, i); - if (i == maxRotationIndex) { - std::filesystem::remove(src); - continue; - } - std::filesystem::path dest = getRotatedFilePath(logPath, i + 1); - std::filesystem::rename(src, dest); - } - auto rotatedLogPath = getRotatedFilePath(logPath, 1); - std::filesystem::rename(logPath, rotatedLogPath); - onLogRotationFinished(); - } - - void Logger::onLogRotationFinished() noexcept - { - if (currentRotationIndex < maxRotationIndex) { - ++currentRotationIndex; - } - } - void Logger::addFileHeader(std::ofstream &file) const { file << application; diff --git a/module-utils/log/Logger.hpp b/module-utils/log/Logger.hpp index 7310fa4b1089d8bd5b49534edf5bf6dd48926614..e0f84e0cb70c17e2ae3a29acc8f353ea33538e17 100644 --- a/module-utils/log/Logger.hpp +++ b/module-utils/log/Logger.hpp @@ -7,6 +7,7 @@ #include #include "LoggerBuffer.hpp" #include "log_colors.hpp" +#include #include #include #include @@ -39,7 +40,7 @@ namespace Log return logger; } auto getLogs() -> std::string; - void init(Application app, size_t fileSize = MAX_LOG_FILE_SIZE, int filesCount = MAX_LOG_FILES_COUNT); + void init(Application app, size_t fileSize = MAX_LOG_FILE_SIZE); auto log(Device device, const char *fmt, va_list args) -> int; auto log(logger_level level, const char *file, int line, const char *function, const char *fmt, va_list args) -> int; @@ -73,8 +74,6 @@ namespace Log } void addFileHeader(std::ofstream &file) const; - void rotateLogFile(const std::filesystem::path &logPath); - void onLogRotationFinished() noexcept; cpp_freertos::MutexStandard mutex; cpp_freertos::MutexStandard logFileMutex; @@ -83,11 +82,10 @@ namespace Log char loggerBuffer[LOGGER_BUFFER_SIZE] = {0}; size_t loggerBufferCurrentPos = 0; size_t maxFileSize = MAX_LOG_FILE_SIZE; - int currentRotationIndex = 0; - int maxRotationIndex = 0; Application application; LoggerBuffer circularBuffer; + utils::Rotator rotator; static constexpr size_t circularBufferSize = 1000; static const char *levelNames[]; diff --git a/module-utils/log/tests/CMakeLists.txt b/module-utils/log/tests/CMakeLists.txt index 9d80451c95954092ce9c9e6ca7d65c8f76837d0d..4851b44531c4a1052bb2ae0f4750d729c7b90a0e 100644 --- a/module-utils/log/tests/CMakeLists.txt +++ b/module-utils/log/tests/CMakeLists.txt @@ -20,6 +20,7 @@ add_catch2_executable( LIBS module-utils module-vfs + log USE_FS ) diff --git a/module-utils/log/tests/test_logDumps.cpp b/module-utils/log/tests/test_logDumps.cpp index 3efaf67a459757e90116bef48ccded2bee92e9e8..a0c5c6f6015a84ffc12a5c942ff8d9b305baa999 100644 --- a/module-utils/log/tests/test_logDumps.cpp +++ b/module-utils/log/tests/test_logDumps.cpp @@ -38,7 +38,6 @@ TEST_CASE("Test if logs are dumped to a file without rotation") { auto app = Log::Application{"TestApp", "rev", "tag", "branch"}; constexpr auto MaxFileSize = 1024 * 1024; // 1 MB - constexpr auto MaxFilesCount = 3; constexpr auto TestLog = "12345678"; const std::filesystem::path logsDir = "./ut_logs"; const auto testLogFile = logsDir / "TestApp.log"; @@ -50,7 +49,7 @@ TEST_CASE("Test if logs are dumped to a file without rotation") std::filesystem::create_directory(logsDir); // Initialize the logger with test parameters. - Log::Logger::get().init(std::move(app), MaxFileSize, MaxFilesCount); + Log::Logger::get().init(std::move(app), MaxFileSize); REQUIRE(countFiles(logsDir) == 0); // Dump logs. @@ -72,7 +71,6 @@ TEST_CASE("Test if log files rotate") { auto app = Log::Application{"TestApp", "rev", "tag", "branch"}; constexpr auto MaxFileSize = 10; // 10 bytes - constexpr auto MaxFilesCount = 3; constexpr auto TestLog = "12345678"; const std::filesystem::path logsDir = "./ut_logs"; const auto testLogFile = logsDir / "TestApp.log"; @@ -84,7 +82,7 @@ TEST_CASE("Test if log files rotate") std::filesystem::create_directory(logsDir); // Initialize the logger with test parameters. - Log::Logger::get().init(std::move(app), MaxFileSize, MaxFilesCount); + Log::Logger::get().init(std::move(app), MaxFileSize); REQUIRE(countFiles(logsDir) == 0); // Dump logs. diff --git a/module-utils/rotator/CMakeLists.txt b/module-utils/rotator/CMakeLists.txt new file mode 100644 index 0000000000000000000000000000000000000000..ae67c443002b93b3265a7779be5bb1947002fab5 --- /dev/null +++ b/module-utils/rotator/CMakeLists.txt @@ -0,0 +1,15 @@ +add_library(utils-rotator INTERFACE) + +target_sources(utils-rotator + PUBLIC + include/rotator/Rotator.hpp +) + +target_include_directories(utils-rotator + INTERFACE + $ +) + +if (${ENABLE_TESTS}) + add_subdirectory(tests) +endif() diff --git a/module-utils/rotator/include/rotator/Rotator.hpp b/module-utils/rotator/include/rotator/Rotator.hpp new file mode 100644 index 0000000000000000000000000000000000000000..fa851aa4faa09ef11759386e9f36a280ecbcac97 --- /dev/null +++ b/module-utils/rotator/include/rotator/Rotator.hpp @@ -0,0 +1,82 @@ +// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. +// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md + +#pragma once + +#include +#include + +namespace utils +{ + template class Rotator + { + protected: + const std::string extension; + + std::size_t getRotatedFilesCount(const std::filesystem::path &path, std::error_code &ec) const + { + std::size_t count = 0; + for (std::size_t i = 1; i < maxRotationFilesCount; i++) { + if (!std::filesystem::exists(getRotatedFilePath(path, i), ec)) { + break; + } + if (ec) { + break; + } + count++; + } + return count; + } + + std::string getRotatedFileExtension(int count) const + { + return extension + "." + std::to_string(count); + } + + std::filesystem::path getRotatedFilePath(const std::filesystem::path &source, int rotationCount) const + { + auto path = source; + path.replace_extension(getRotatedFileExtension(rotationCount)); + return path; + } + + public: + explicit Rotator(std::string extension) : extension{extension} + {} + + virtual ~Rotator() = default; + + bool rotateFile(const std::filesystem::path &path) const + { + std::error_code ec; + if (const auto firstDump = !std::filesystem::exists(path, ec); ec) { + return false; + } + else if (firstDump) { + return true; + } + const auto rotatedFiles = getRotatedFilesCount(path, ec); + if (ec) { + return false; + } + for (std::size_t i = rotatedFiles; i > 0; i--) { + const auto src = getRotatedFilePath(path, i); + if (i == maxRotationFilesCount - 1) { + std::filesystem::remove(src, ec); + if (ec) { + return false; + } + continue; + } + const auto dest = getRotatedFilePath(path, i + 1); + std::filesystem::rename(src, dest, ec); + if (ec) { + return false; + } + } + auto rotatedLogPath = getRotatedFilePath(path, 1); + std::filesystem::rename(path, rotatedLogPath, ec); + return (ec) ? false : true; + } + }; +} // namespace utils diff --git a/module-utils/rotator/tests/CMakeLists.txt b/module-utils/rotator/tests/CMakeLists.txt new file mode 100644 index 0000000000000000000000000000000000000000..91da783de190c8f52b44c208fe025008a86903c2 --- /dev/null +++ b/module-utils/rotator/tests/CMakeLists.txt @@ -0,0 +1,9 @@ +# Rotator tests +add_catch2_executable( + NAME + rotator-test + SRCS + test_Rotator.cpp + LIBS + utils-rotator +) diff --git a/module-utils/rotator/tests/test_Rotator.cpp b/module-utils/rotator/tests/test_Rotator.cpp new file mode 100644 index 0000000000000000000000000000000000000000..0fd57bd7b0fdf8b50c0bfa40ab5987cd48d5d547 --- /dev/null +++ b/module-utils/rotator/tests/test_Rotator.cpp @@ -0,0 +1,94 @@ +// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. +// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md + +#define CATCH_CONFIG_MAIN + +#include +#include + +#include +#include + +namespace +{ + constexpr std::size_t maxRotationFilesCount = 5; + + class HexRotator : public utils::Rotator + { + public: + HexRotator() : utils::Rotator{".hex"} + {} + + std::filesystem::path rotatedFilePath(const std::filesystem::path &source, int rotationCount) + { + return this->getRotatedFilePath(source, rotationCount); + } + }; +} // namespace + +TEST_CASE("Rotation of hex dump files") +{ + HexRotator rotator; + constexpr auto fileName = "crashdump.hex"; + SECTION("Handle first dump") + { + REQUIRE(rotator.rotateFile(fileName)); + } + SECTION("Handle first rotate") + { + constexpr auto firstRotationName = "crashdump.hex.1"; + std::ofstream file(fileName); + REQUIRE(rotator.rotateFile(fileName)); + REQUIRE(std::filesystem::exists(firstRotationName)); + REQUIRE_FALSE(std::filesystem::exists(fileName)); + REQUIRE(std::filesystem::remove(firstRotationName)); + } + SECTION("Handle max rotates") + { + for (std::size_t i = 1; i <= maxRotationFilesCount; i++) { + const auto rotatedFileName = rotator.rotatedFilePath(fileName, i); + std::ofstream file(fileName); + REQUIRE(rotator.rotateFile(fileName)); + for (std::size_t j = i - 1; j > 0; j--) { + REQUIRE(std::filesystem::exists(rotator.rotatedFilePath(fileName, j))); + } + if (i == maxRotationFilesCount) { + REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, i))); + } + REQUIRE_FALSE(std::filesystem::exists(fileName)); + } + for (std::size_t i = 1; i < maxRotationFilesCount; i++) { + const auto rotatedFileName = rotator.rotatedFilePath(fileName, i); + REQUIRE(std::filesystem::remove(rotatedFileName)); + } + } + + SECTION("Handle more than max files") + { + for (std::size_t i = 1; i <= maxRotationFilesCount; i++) { + const auto rotatedFileName = rotator.rotatedFilePath(fileName, i); + std::ofstream file(fileName); + REQUIRE(rotator.rotateFile(fileName)); + for (std::size_t j = i - 1; j > 0; j--) { + REQUIRE(std::filesystem::exists(rotator.rotatedFilePath(fileName, j))); + } + if (i == maxRotationFilesCount) { + REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, i))); + } + REQUIRE_FALSE(std::filesystem::exists(fileName)); + } + std::ofstream file(fileName); + REQUIRE(rotator.rotateFile(fileName)); + REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, maxRotationFilesCount + 1))); + for (std::size_t i = 1; i < maxRotationFilesCount; i++) { + const auto rotatedFileName = rotator.rotatedFilePath(fileName, i); + REQUIRE(std::filesystem::exists(rotatedFileName)); + } + REQUIRE_FALSE(std::filesystem::exists(rotator.rotatedFilePath(fileName, maxRotationFilesCount))); + REQUIRE_FALSE(std::filesystem::exists(fileName)); + for (std::size_t i = 1; i < maxRotationFilesCount; i++) { + const auto rotatedFileName = rotator.rotatedFilePath(fileName, i); + REQUIRE(std::filesystem::remove(rotatedFileName)); + } + } +}