From a56ad4050adc0a5f1d3442e273bac9c438746f6e Mon Sep 17 00:00:00 2001 From: atarekra Date: Tue, 9 Dec 2025 00:29:27 +0200 Subject: [PATCH 1/9] Adding initial implementation for log file size control --- score/json/examples/logging.json | 16 +- score/mw/log/configuration/configuration.cpp | 50 +++ score/mw/log/configuration/configuration.h | 30 ++ .../configuration/target_config_reader.cpp | 69 ++++ score/mw/log/detail/file_logging/BUILD | 3 + .../file_logging/console_recorder_factory.cpp | 8 +- .../file_logging/file_output_backend.cpp | 31 +- .../detail/file_logging/file_output_backend.h | 10 +- .../file_logging/file_output_backend_test.cpp | 42 ++- .../file_logging/file_recorder_factory.cpp | 22 +- .../file_logging/non_blocking_writer.cpp | 341 +++++++++++++++++- .../detail/file_logging/non_blocking_writer.h | 41 ++- .../file_logging/non_blocking_writer_test.cpp | 77 +++- .../log/detail/file_logging/slot_drainer.cpp | 18 +- .../mw/log/detail/file_logging/slot_drainer.h | 7 + .../detail/file_logging/slot_drainer_test.cpp | 37 +- score/mw/log/detail/recorder_factory.cpp | 2 + score/mw/log/detail/recorder_factory_stub.cpp | 7 +- score/os/BUILD | 5 + 19 files changed, 754 insertions(+), 62 deletions(-) diff --git a/score/json/examples/logging.json b/score/json/examples/logging.json index 53027f2c..e752b9f4 100644 --- a/score/json/examples/logging.json +++ b/score/json/examples/logging.json @@ -1,6 +1,14 @@ + { - "appId": "JSON", - "appDesc": "JSON example programs", - "logMode" : "kConsole", - "logLevel": "kVerbose" + "ecuId": "ECU1", + "appId": "log", + "logLevel": "kError", + "logMode": "kConsole|kFile", + "logLevelThresholdConsole": "kInfo", + "logFilePath": "./", + "circularFileLogging": false, + "overwriteLogOnFull": false, + "maxLogFileSizeBytes": 500, + "deleteOldLogFiles": false, + "noOfLogFiles": 2 } diff --git a/score/mw/log/configuration/configuration.cpp b/score/mw/log/configuration/configuration.cpp index d478955a..b3e10cce 100644 --- a/score/mw/log/configuration/configuration.cpp +++ b/score/mw/log/configuration/configuration.cpp @@ -208,6 +208,56 @@ void Configuration::SetDynamicDatarouterIdentifiers(const bool enable_dynamic_id { dynamic_datarouter_identifiers_ = enable_dynamic_identifiers; } +bool Configuration::IsCircularFileLogging() const noexcept +{ + return circular_file_logging_; +} + +void Configuration::SetCircularFileLogging(const bool circular_file_logging) noexcept +{ + circular_file_logging_ = circular_file_logging; +} + +std::size_t Configuration::GetMaxLogFileSizeBytes() const noexcept +{ + return max_log_file_size_bytes_; +} + +void Configuration::SetMaxLogFileSizeBytes(const std::size_t max_log_file_size_bytes) noexcept +{ + max_log_file_size_bytes_ = max_log_file_size_bytes; +} + +bool Configuration::IsOverwriteLogOnFull() const noexcept +{ + return overwrite_log_on_full_; +} + +void Configuration::SetOverwriteLogOnFull(const bool overwrite_log_on_full) noexcept +{ + // The JSON configuration key for this parameter is "overwriteLogOnFull". + overwrite_log_on_full_ = overwrite_log_on_full; +} + +std::size_t Configuration::GetNoOfLogFiles() const noexcept +{ + return no_of_log_files_; +} + +void Configuration::SetNoOfLogFiles(const std::size_t no_of_log_files) noexcept +{ + no_of_log_files_ = no_of_log_files; +} + +bool Configuration::IsDeleteOldLogFiles() const noexcept +{ + return delete_old_log_files_; +} + +void Configuration::SetDeleteOldLogFiles(const bool delete_old_log_files) noexcept +{ + delete_old_log_files_ = delete_old_log_files; +} } // namespace detail } // namespace log diff --git a/score/mw/log/configuration/configuration.h b/score/mw/log/configuration/configuration.h index 80f5be33..babcb1fc 100644 --- a/score/mw/log/configuration/configuration.h +++ b/score/mw/log/configuration/configuration.h @@ -84,6 +84,21 @@ class Configuration final bool GetDynamicDatarouterIdentifiers() const noexcept; void SetDynamicDatarouterIdentifiers(const bool enable_dynamic_identifiers) noexcept; + bool IsCircularFileLogging() const noexcept; + void SetCircularFileLogging(const bool) noexcept; + + std::size_t GetMaxLogFileSizeBytes() const noexcept; + void SetMaxLogFileSizeBytes(const std::size_t) noexcept; + + bool IsOverwriteLogOnFull() const noexcept; + void SetOverwriteLogOnFull(const bool) noexcept; + + std::size_t GetNoOfLogFiles() const noexcept; + void SetNoOfLogFiles(const std::size_t) noexcept; + + bool IsDeleteOldLogFiles() const noexcept; + void SetDeleteOldLogFiles(const bool) noexcept; + /// \brief Returns true if the log level is enabled for the context. /// \param use_console_default_level Set to true if threshold for console logging should be considered as default /// log level. Otherwise default_log_level_ will be used instead. @@ -138,6 +153,21 @@ class Configuration final /// \brief Toggle between dynamic datarouter identifiers. bool dynamic_datarouter_identifiers_{false}; + + /// \brief Enable circular file logging. + bool circular_file_logging_{false}; + + /// \brief Maximum log file size in bytes for circular file logging. + std::size_t max_log_file_size_bytes_{10485760}; // 10 MB default + + /// \brief Overwrite log file on full. + bool overwrite_log_on_full_{false}; + + /// \brief Number of log files to keep. + std::size_t no_of_log_files_{1}; + + /// \brief Delete old log files when the maximum number of files is reached. + bool delete_old_log_files_{false}; }; } // namespace detail diff --git a/score/mw/log/configuration/target_config_reader.cpp b/score/mw/log/configuration/target_config_reader.cpp index fb421fe6..b231b8a2 100644 --- a/score/mw/log/configuration/target_config_reader.cpp +++ b/score/mw/log/configuration/target_config_reader.cpp @@ -48,6 +48,11 @@ constexpr StringLiteral kNumberOfSlotsKey{"numberOfSlots"}; constexpr StringLiteral kSlotSizeBytesKey{"slotSizeBytes"}; constexpr StringLiteral kDatarouterUidKey{"datarouterUid"}; constexpr StringLiteral kDynamicDatarouterIdentifiersKey{"dynamicDatarouterIdentifiers"}; +constexpr StringLiteral kCircularFileLoggingKey{"circularFileLogging"}; +constexpr StringLiteral kMaxLogFileSizeBytesKey{"maxLogFileSizeBytes"}; +constexpr StringLiteral kOverwriteKey{"overwriteLogOnFull"}; +constexpr StringLiteral kNoOfLogFilesKey{"noOfLogFiles"}; +constexpr StringLiteral kDeleteOldLogFilesKey{"deleteOldLogFiles"}; // Suppress Coverity warning because: // 1. 'constexpr' cannot be used with std::unordered_map. @@ -427,6 +432,65 @@ score::ResultBlank ParseDynamicDatarouterIdentifiers(const score::json::Object& // clang-format on } +score::ResultBlank ParseCircularFileLogging(const score::json::Object& root, Configuration& config) noexcept +{ + // clang-format off + return GetElementAndThen( + root, + kCircularFileLoggingKey, + [&config](auto value) noexcept { config.SetCircularFileLogging(value); } + ); + // clang-format on +} + +score::ResultBlank ParseMaxLogFileSizeBytes(const score::json::Object& root, Configuration& config) noexcept +{ + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation + // clang-format off + return GetElementAndThen( + root, + kMaxLogFileSizeBytesKey, + [&config](auto value) noexcept { config.SetMaxLogFileSizeBytes(value); } + ); + // clang-format on +} + +score::ResultBlank ParseOverwriteLogOnFull(const score::json::Object& root, Configuration& config) noexcept +{ + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation + // clang-format off + return GetElementAndThen( + root, + kOverwriteKey, + [&config](auto value) noexcept { config.SetOverwriteLogOnFull(value); } + ); + // clang-format on +} + +score::ResultBlank ParseNoOfLogFiles(const score::json::Object& root, Configuration& config) noexcept +{ + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation + // clang-format off + return GetElementAndThen( + root, + kNoOfLogFilesKey, + [&config](auto value) noexcept { config.SetNoOfLogFiles(value); } + ); + // clang-format on +} + +score::ResultBlank ParseDeleteOldLogFiles(const score::json::Object& root, Configuration& config) noexcept +{ + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation + // clang-format off + return GetElementAndThen( + root, + kDeleteOldLogFilesKey, + [&config](auto value) noexcept { config.SetDeleteOldLogFiles(value); } + ); + // clang-format on +} + void ParseConfigurationElements(const score::json::Object& root, const std::string& path, Configuration& config) noexcept { ReportOnError(ParseEcuId(root, config), path); @@ -444,6 +508,11 @@ void ParseConfigurationElements(const score::json::Object& root, const std::stri ReportOnError(ParseSlotSizeBytes(root, config), path); ReportOnError(ParseDatarouterUid(root, config), path); ReportOnError(ParseDynamicDatarouterIdentifiers(root, config), path); + ReportOnError(ParseCircularFileLogging(root, config), path); + ReportOnError(ParseMaxLogFileSizeBytes(root, config), path); + ReportOnError(ParseOverwriteLogOnFull(root, config), path); + ReportOnError(ParseNoOfLogFiles(root, config), path); + ReportOnError(ParseDeleteOldLogFiles(root, config), path); } // Suppress "AUTOSAR C++14 A15-5-3" rule findings: "The std::terminate() function shall not be called implicitly". diff --git a/score/mw/log/detail/file_logging/BUILD b/score/mw/log/detail/file_logging/BUILD index f3d5ae9d..da68237b 100644 --- a/score/mw/log/detail/file_logging/BUILD +++ b/score/mw/log/detail/file_logging/BUILD @@ -153,8 +153,11 @@ cc_library( "@score_baselibs//score/mw/log/detail:__pkg__", ], deps = [ + "@score_baselibs//score/filesystem:filesystem", "@score_baselibs//score/language/futurecpp", "@score_baselibs//score/mw/log/detail:types_and_errors", + "@score_baselibs//score/os:fcntl", + "@score_baselibs//score/os:os", "@score_baselibs//score/os:unistd", ], ) diff --git a/score/mw/log/detail/file_logging/console_recorder_factory.cpp b/score/mw/log/detail/file_logging/console_recorder_factory.cpp index 8ab44a59..f1f4ab4c 100644 --- a/score/mw/log/detail/file_logging/console_recorder_factory.cpp +++ b/score/mw/log/detail/file_logging/console_recorder_factory.cpp @@ -40,9 +40,15 @@ std::unique_ptr ConsoleRecorderFactory::CreateConsoleLoggingBackend( return std::make_unique(std::move(message_builder), STDOUT_FILENO, + "", std::move(allocator), score::os::FcntlImpl::Default(memory_resource), - score::os::Unistd::Default(memory_resource)); + score::os::Unistd::Default(memory_resource), + false, + false, + 0, + 1, + false); } } // namespace detail diff --git a/score/mw/log/detail/file_logging/file_output_backend.cpp b/score/mw/log/detail/file_logging/file_output_backend.cpp index 26919b1d..2749564f 100644 --- a/score/mw/log/detail/file_logging/file_output_backend.cpp +++ b/score/mw/log/detail/file_logging/file_output_backend.cpp @@ -25,21 +25,30 @@ namespace detail FileOutputBackend::FileOutputBackend(std::unique_ptr message_builder, const std::int32_t file_descriptor, + const std::string& file_path, std::unique_ptr> allocator, - score::cpp::pmr::unique_ptr fcntl_instance, - score::cpp::pmr::unique_ptr unistd) noexcept + score::cpp::pmr::unique_ptr fcntl, + score::cpp::pmr::unique_ptr unistd, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files) noexcept : Backend(), buffer_allocator_(std::move(allocator)), - slot_drainer_(std::move(message_builder), buffer_allocator_, file_descriptor, std::move(unistd)) + slot_drainer_(std::move(message_builder), + buffer_allocator_, + file_descriptor, + file_path, + std::move(unistd), + std::move(fcntl), + circular_file_logging, + overwrite_log_on_full, + max_log_file_size_bytes, + no_of_log_files, + delete_old_log_files) + { - const auto flags = fcntl_instance->fcntl(file_descriptor, score::os::Fcntl::Command::kFileGetStatusFlags); - if (flags.has_value()) - { - std::ignore = fcntl_instance->fcntl( - file_descriptor, - score::os::Fcntl::Command::kFileSetStatusFlags, - flags.value() | score::os::Fcntl::Open::kNonBlocking | score::os::Fcntl::Open::kCloseOnExec); - } } score::cpp::optional FileOutputBackend::ReserveSlot() noexcept diff --git a/score/mw/log/detail/file_logging/file_output_backend.h b/score/mw/log/detail/file_logging/file_output_backend.h index dcd6734a..7014063b 100644 --- a/score/mw/log/detail/file_logging/file_output_backend.h +++ b/score/mw/log/detail/file_logging/file_output_backend.h @@ -33,9 +33,15 @@ class FileOutputBackend final : public Backend public: FileOutputBackend(std::unique_ptr message_builder, const std::int32_t file_descriptor, + const std::string& file_path, std::unique_ptr> allocator, - score::cpp::pmr::unique_ptr fcntl_instance, - score::cpp::pmr::unique_ptr unistd) noexcept; + score::cpp::pmr::unique_ptr fcntl, + score::cpp::pmr::unique_ptr unistd, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files) noexcept; /// \brief Before a producer can store data in our buffer, he has to reserve a slot. /// /// \return SlotHandle if a slot was able to be reserved, empty otherwise. diff --git a/score/mw/log/detail/file_logging/file_output_backend_test.cpp b/score/mw/log/detail/file_logging/file_output_backend_test.cpp index a66eb8fe..39507629 100644 --- a/score/mw/log/detail/file_logging/file_output_backend_test.cpp +++ b/score/mw/log/detail/file_logging/file_output_backend_test.cpp @@ -81,7 +81,12 @@ TEST_F(FileOutputBackendFixture, ReserveSlotShouldTriggerFlushing) file_descriptor_, std::move(allocator_), std::move(fcntl_mock), - std::move(unistd_mock)); + std::move(unistd_mock), + false, + false, + 0, + 1, + false); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) .WillOnce(Return(OptionalSpan{})) @@ -107,7 +112,12 @@ TEST_F(FileOutputBackendFixture, FlushSlotShouldTriggerFlushing) file_descriptor_, std::move(allocator_), std::move(fcntl_mock), - std::move(unistd_mock)); + std::move(unistd_mock), + false, + false, + 0, + 1, + false); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) .WillOnce(Return(OptionalSpan{})) // first unitialized @@ -140,7 +150,12 @@ TEST_F(FileOutputBackendFixture, DepletedAllocatorShouldCauseEmptyOptionalReturn file_descriptor_, std::move(allocator_), std::move(fcntl_mock), - std::move(unistd_mock)); + std::move(unistd_mock), + false, + false, + 0, + 1, + false); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) .WillOnce(Return(OptionalSpan{})) // first unitialized @@ -164,7 +179,12 @@ TEST_F(FileOutputBackendFixture, GetLogRecordReturnsObjectSameAsAllocatorWould) file_descriptor_, std::move(allocator_), std::move(fcntl_mock), - std::move(unistd_mock)); + std::move(unistd_mock), + false, + false, + 0, + 1, + false); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan).WillRepeatedly(Return(OptionalSpan{})); const auto slot = unit.ReserveSlot(); @@ -204,7 +224,12 @@ TEST_F(FileOutputBackendFixture, BackendConstructionShallCallNonBlockingFileSetu file_descriptor_, std::move(allocator_), std::move(fcntl_mock), - std::move(unistd_mock)); + std::move(unistd_mock), + false, + false, + 0, + 1, + false); } TEST_F(FileOutputBackendFixture, MissingFlagsShallSkipCallToSetupFile) @@ -231,7 +256,12 @@ TEST_F(FileOutputBackendFixture, MissingFlagsShallSkipCallToSetupFile) file_descriptor_, std::move(allocator_), std::move(fcntl_mock), - std::move(unistd_mock)); + std::move(unistd_mock), + false, + false, + 0, + 1, + false); } } // namespace diff --git a/score/mw/log/detail/file_logging/file_recorder_factory.cpp b/score/mw/log/detail/file_logging/file_recorder_factory.cpp index 36f13f2a..9c70c299 100644 --- a/score/mw/log/detail/file_logging/file_recorder_factory.cpp +++ b/score/mw/log/detail/file_logging/file_recorder_factory.cpp @@ -38,13 +38,21 @@ std::unique_ptr FileRecorderFactory::CreateFileLoggingBackend( const Configuration& config, score::cpp::pmr::memory_resource* memory_resource) noexcept { - const std::string file_name{std::string(config.GetLogFilePath().data(), config.GetLogFilePath().size()) + "/" + - std::string{config.GetAppId().data(), config.GetAppId().size()} + ".dlt"}; + const std::string file_path_base{std::string(config.GetLogFilePath().data(), config.GetLogFilePath().size())}; + const std::string app_id{std::string{config.GetAppId().data(), config.GetAppId().size()}}; + const std::string file_extension{".dlt"}; + std::string file_name{file_path_base + "/" + app_id + file_extension}; + if (config.GetNoOfLogFiles() > 1) { + file_name = file_path_base + "/" + app_id + "_1" + file_extension; + } + + auto open_flags = + score::os::Fcntl::Open::kReadWrite | score::os::Fcntl::Open::kCreate | score::os::Fcntl::Open::kCloseOnExec; // NOLINTBEGIN(score-banned-function): FileLoggingBackend is disabled in production. Argumentation: Ticket-75726 const auto descriptor_result = fcntl_->open( file_name.data(), - score::os::Fcntl::Open::kWriteOnly | score::os::Fcntl::Open::kCreate | score::os::Fcntl::Open::kCloseOnExec, + open_flags, score::os::Stat::Mode::kReadUser | score::os::Stat::Mode::kWriteUser | score::os::Stat::Mode::kReadGroup | score::os::Stat::Mode::kReadOthers); // NOLINTEND(score-banned-function): see above for detailed explanation @@ -72,9 +80,15 @@ std::unique_ptr FileRecorderFactory::CreateFileLoggingBackend( return std::make_unique(std::move(message_builder), descriptor, + file_name, std::move(allocator), score::os::Fcntl::Default(memory_resource), - score::os::Unistd::Default(memory_resource)); + score::os::Unistd::Default(memory_resource), + config.IsCircularFileLogging(), + config.IsOverwriteLogOnFull(), + config.GetMaxLogFileSizeBytes(), + config.GetNoOfLogFiles(), + config.IsDeleteOldLogFiles()); } } // namespace detail diff --git a/score/mw/log/detail/file_logging/non_blocking_writer.cpp b/score/mw/log/detail/file_logging/non_blocking_writer.cpp index 21663179..7a511f2c 100644 --- a/score/mw/log/detail/file_logging/non_blocking_writer.cpp +++ b/score/mw/log/detail/file_logging/non_blocking_writer.cpp @@ -12,10 +12,21 @@ ********************************************************************************/ #include "score/mw/log/detail/file_logging/non_blocking_writer.h" +#include "score/filesystem/path.h" +#include "score/os/fcntl.h" +#include "score/os/stat.h" + +// #include "score/concurrency/clock.h" +// #include "score/mw/log/logging.h" + +#include +#include + #if defined(__QNXNTO__) #include #endif // __QNXNTO__ + namespace score { namespace mw @@ -42,17 +53,125 @@ std::size_t NonBlockingWriter::GetMaxChunkSize() noexcept return kMaxChunkSizeSupportedByOs; } -NonBlockingWriter::NonBlockingWriter(const int32_t fileHandle, +NonBlockingWriter::NonBlockingWriter(const std::string& file_path, std::size_t max_chunk_size, - score::cpp::pmr::unique_ptr unistd) noexcept + score::cpp::pmr::unique_ptr unistd, + score::cpp::pmr::unique_ptr fcntl, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files) noexcept : unistd_{std::move(unistd)}, - file_handle_(fileHandle), + fcntl_{std::move(fcntl)}, number_of_flushed_bytes_{0U}, buffer_{nullptr, 0UL}, buffer_flushed_{Result::kWouldBlock}, - max_chunk_size_(std::min(max_chunk_size, GetMaxChunkSize())) + max_chunk_size_(std::min(max_chunk_size, GetMaxChunkSize())), + circular_file_logging_(circular_file_logging), + overwrite_log_on_full_(overwrite_log_on_full), + max_log_file_size_bytes_(max_log_file_size_bytes), + current_file_position_{0U}, + no_of_log_files_(no_of_log_files), + delete_old_log_files_(delete_old_log_files), + current_log_file_index_{1} +{ + const score::filesystem::Path path(file_path); + file_path_ = path.ParentPath().Native(); + file_extension_ = path.Extension().Native(); + file_name_ = path.Stem().Native(); + + std::string initial_file_path = file_path; + if (no_of_log_files_ > 1) + { + initial_file_path = file_path_ + "/" + file_name_ + "_" + std::to_string(current_log_file_index_) + file_extension_; + } + + auto open_flags = + score::os::Fcntl::Open::kReadWrite | score::os::Fcntl::Open::kCreate | score::os::Fcntl::Open::kCloseOnExec; + + // NOLINTBEGIN(score-banned-function): FileLoggingBackend is disabled in production. Argumentation: Ticket-75726 + const auto descriptor_result = fcntl_->open( + initial_file_path.data(), + open_flags, + score::os::Stat::Mode::kReadUser | score::os::Stat::Mode::kWriteUser | score::os::Stat::Mode::kReadGroup | + score::os::Stat::Mode::kReadOthers); + + if (descriptor_result.has_value()) + { + file_handle_ = descriptor_result.value(); + const auto flags = fcntl_->fcntl(file_handle_, score::os::Fcntl::Command::kFileGetStatusFlags); + if (flags.has_value()) + { + std::ignore = fcntl_->fcntl( + file_handle_, + score::os::Fcntl::Command::kFileSetStatusFlags, + flags.value() | score::os::Fcntl::Open::kNonBlocking | score::os::Fcntl::Open::kCloseOnExec); + } + } + + if (circular_file_logging_ && max_log_file_size_bytes_ > 0 && file_handle_ != -1) + { + const auto file_size_or_error = unistd_->lseek(file_handle_, 0, SEEK_END); + if (file_size_or_error.has_value()) + { + current_file_position_ = static_cast(file_size_or_error.value()); + } + // If lseek fails, current_file_position_ remains 0, which is a safe fallback. + } +} +NonBlockingWriter::NonBlockingWriter(const std::string& file_path, + std::int32_t file_descriptor, + std::size_t max_chunk_size, + score::cpp::pmr::unique_ptr unistd, + score::cpp::pmr::unique_ptr fcntl, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files) noexcept + : unistd_{std::move(unistd)}, + fcntl_{std::move(fcntl)}, + file_handle_{file_descriptor}, + number_of_flushed_bytes_{0U}, + buffer_{nullptr, 0UL}, + buffer_flushed_{Result::kWouldBlock}, + max_chunk_size_(std::min(max_chunk_size, GetMaxChunkSize())), + circular_file_logging_(circular_file_logging), + overwrite_log_on_full_(overwrite_log_on_full), + max_log_file_size_bytes_(max_log_file_size_bytes), + current_file_position_{0U}, + no_of_log_files_(no_of_log_files), + delete_old_log_files_(delete_old_log_files), + current_log_file_index_{1} { + const score::filesystem::Path path(file_path); + file_path_ = path.ParentPath().Native(); + file_extension_ = path.Extension().Native(); + file_name_ = path.Stem().Native(); + if (no_of_log_files_ > 1 && file_name_.size() > 2 && file_name_.substr(file_name_.size() - 2) == "_1") + { + file_name_ = file_name_.substr(0, file_name_.size() - 2); + } + const auto flags = fcntl_->fcntl(file_handle_, score::os::Fcntl::Command::kFileGetStatusFlags); + if (flags.has_value()) + { + std::ignore = fcntl_->fcntl( + file_handle_, + score::os::Fcntl::Command::kFileSetStatusFlags, + flags.value() | score::os::Fcntl::Open::kNonBlocking | score::os::Fcntl::Open::kCloseOnExec); + } + + if (circular_file_logging_ && max_log_file_size_bytes_ > 0 && file_handle_ != -1) + { + const auto file_size_or_error = unistd_->lseek(file_handle_, 0, SEEK_END); + if (file_size_or_error.has_value()) + { + current_file_position_ = static_cast(file_size_or_error.value()); + } + // If lseek fails, current_file_position_ remains 0, which is a safe fallback. + } } void NonBlockingWriter::SetSpan(const score::cpp::span& buffer) noexcept @@ -87,27 +206,217 @@ score::cpp::expected N score::cpp::expected NonBlockingWriter::InternalFlush(const uint64_t size_to_flush) noexcept { + if (file_handle_ == -1) + { + return 0; // File is not open, do nothing. + } + const auto buffer_size = static_cast(buffer_.size()); - if (number_of_flushed_bytes_ < buffer_size) - { - score::cpp::expected num_of_bytes_written{}; - // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) used on span which is an array - // NOLINTBEGIN(score-banned-function) it is among safety headers. - // Needs ptr to access score::cpp::span elements - // coverity[autosar_cpp14_m5_0_15_violation] - num_of_bytes_written = unistd_->write(file_handle_, &(buffer_.data()[number_of_flushed_bytes_]), size_to_flush); - // NOLINTEND(score-banned-function) it is among safety headers. - // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) used on span which is an array + if (number_of_flushed_bytes_ >= buffer_size) + { + return 0; // Nothing left to flush + } + + // --- Multi-file rotation logic --- + if (circular_file_logging_ && no_of_log_files_ > 1) + { + // If the file size is limited and the next write would exceed it, rotate the file. + if (max_log_file_size_bytes_ > 0 && (current_file_position_ + size_to_flush > max_log_file_size_bytes_)) + { + RotateLogFile(); + } + + // After potential rotation, we write to the current file. + // We must still guard against a single write being larger than the max file size. + const uint64_t remaining_space = (max_log_file_size_bytes_ > 0) + ? (max_log_file_size_bytes_ - current_file_position_) + : size_to_flush; + const uint64_t write_size = std::min(size_to_flush, remaining_space); + + if (write_size == 0 && max_log_file_size_bytes_ > 0) + { + // This can happen if the file is exactly full. The next call will rotate. + return 0; + } + + auto num_of_bytes_written = + unistd_->write(file_handle_, &(buffer_.data()[number_of_flushed_bytes_]), write_size); + if (!num_of_bytes_written.has_value()) { return score::cpp::make_unexpected(num_of_bytes_written.error()); } + + const auto written_count = static_cast(num_of_bytes_written.value()); + number_of_flushed_bytes_ += written_count; + current_file_position_ += written_count; + return num_of_bytes_written.value(); + } + + // --- Fallback to simple write or single-file circular logging --- + + // If circular logging is not enabled, perform a simple write. + if (!circular_file_logging_ || max_log_file_size_bytes_ == 0) { + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto num_of_bytes_written = + unistd_->write(file_handle_, &(buffer_.data()[number_of_flushed_bytes_]), size_to_flush); + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) + if (!num_of_bytes_written.has_value()) + { + return score::cpp::make_unexpected(num_of_bytes_written.error()); + } + const auto written_count = static_cast(num_of_bytes_written.value()); + number_of_flushed_bytes_ += written_count; + return num_of_bytes_written.value(); + } + + // --- Circular logging is enabled from this point on (and it's single-file mode) --- + + if (!overwrite_log_on_full_) + { + // CASE: Circular logging, NO overwrite. + if (current_file_position_ >= max_log_file_size_bytes_) + { + const std::string console_warning = "WARNING: Log file size limit reached. Logging has stopped.\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + return 0; // File is full, stop writing. + } + + const uint64_t remaining_space = max_log_file_size_bytes_ - current_file_position_; + const uint64_t write_size = std::min(size_to_flush, remaining_space); + + if (write_size == 0) { + return 0; + } + + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto num_of_bytes_written = + unistd_->write(file_handle_, &(buffer_.data()[number_of_flushed_bytes_]), write_size); + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) + if (!num_of_bytes_written.has_value()) + { + return score::cpp::make_unexpected(num_of_bytes_written.error()); + } + + const auto written_count = static_cast(num_of_bytes_written.value()); + number_of_flushed_bytes_ += written_count; + current_file_position_ += written_count; + return num_of_bytes_written.value(); + } + else + { + // CASE: Circular logging, WITH overwrite. + if (current_file_position_ + size_to_flush <= max_log_file_size_bytes_) + { + // The write fits without wrapping. + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto num_of_bytes_written = + unistd_->write(file_handle_, &(buffer_.data()[number_of_flushed_bytes_]), size_to_flush); + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) + if (!num_of_bytes_written.has_value()) + { + return score::cpp::make_unexpected(num_of_bytes_written.error()); + } + const auto written_count = static_cast(num_of_bytes_written.value()); + number_of_flushed_bytes_ += written_count; + current_file_position_ += written_count; + return num_of_bytes_written.value(); + } else { - number_of_flushed_bytes_ += static_cast(num_of_bytes_written.value()); + // The write crosses the boundary and needs to wrap. + const std::string console_warning = "WARNING: Log file size limit reached. Wrapping around.\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + + // Seek to the beginning of the file to overwrite. + const auto seek_result = unistd_->lseek(file_handle_, 0, SEEK_SET); + if (!seek_result.has_value()) + { + // If seek fails, we cannot proceed with the circular write. + return score::cpp::make_unexpected(seek_result.error()); + } + + const uint64_t write_size = std::min(size_to_flush, max_log_file_size_bytes_); + + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto num_of_bytes_written = + unistd_->write(file_handle_, &(buffer_.data()[number_of_flushed_bytes_]), write_size); + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) + if (!num_of_bytes_written.has_value()) + { + return score::cpp::make_unexpected(num_of_bytes_written.error()); + } + + const auto written_count = static_cast(num_of_bytes_written.value()); + number_of_flushed_bytes_ += written_count; + current_file_position_ = written_count; + return num_of_bytes_written.value(); + } + } +} + +void NonBlockingWriter::RotateLogFile() noexcept +{ + if (no_of_log_files_ <= 1) + { + return; + } + if (file_handle_ != -1) + { + unistd_->close(file_handle_); + file_handle_ = -1; // Invalidate handle immediately after closing. + } + + current_log_file_index_++; + if (current_log_file_index_ > no_of_log_files_) + { + current_log_file_index_ = 1; + } + + std::string next_file_path = file_path_ + "/" + file_name_ + "_" + std::to_string(current_log_file_index_) + file_extension_; + + // Check for file existence using the Unistd wrapper. + const bool file_exists = unistd_->access(next_file_path.c_str(), score::os::Unistd::AccessMode::kExists).has_value(); + + if (file_exists && !overwrite_log_on_full_) + { + const std::string console_warning = "WARNING: Log file rotation stopped. File exists and overwrite is disabled.\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + current_file_position_ = 0; + return; // Stop, leaving file_handle_ as -1. + } + + auto open_flags = + score::os::Fcntl::Open::kWriteOnly | score::os::Fcntl::Open::kCreate | score::os::Fcntl::Open::kCloseOnExec; + + // Only truncate if both overwrite and delete are enabled, matching user's expectation. + if (overwrite_log_on_full_ && delete_old_log_files_) + { + open_flags |= score::os::Fcntl::Open::kTruncate; + } + + const auto descriptor_result = fcntl_->open( + next_file_path.data(), + open_flags, + score::os::Stat::Mode::kReadUser | score::os::Stat::Mode::kWriteUser | score::os::Stat::Mode::kReadGroup | + score::os::Stat::Mode::kReadOthers); + + if (descriptor_result.has_value()) + { + file_handle_ = descriptor_result.value(); + if (file_exists && overwrite_log_on_full_ && !delete_old_log_files_) + { + unistd_->lseek(file_handle_, 0, SEEK_SET); } } - return number_of_flushed_bytes_; + else + { + // Handle remains -1 from earlier. + const std::string console_warning = "ERROR: Could not open next log file: " + next_file_path + "\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + } + + current_file_position_ = 0; } } // namespace detail diff --git a/score/mw/log/detail/file_logging/non_blocking_writer.h b/score/mw/log/detail/file_logging/non_blocking_writer.h index 10207692..da087114 100644 --- a/score/mw/log/detail/file_logging/non_blocking_writer.h +++ b/score/mw/log/detail/file_logging/non_blocking_writer.h @@ -16,11 +16,15 @@ // Be careful what you include here. Each additional header will be included in logging.h and thus exposed to the user. // We need to try to keep the includes low to reduce the compile footprint of using this library. #include "score/os/unistd.h" +#include "score/os/fcntl.h" #include "score/mw/log/detail/error.h" #include +#include + #include #include +#include namespace score { @@ -44,30 +48,59 @@ class NonBlockingWriter final }; /// \brief Constructor that accepts fileHandle of the file to flush data into it and max_chunk size controlled by /// user and have limit to kMaxChunkSizeSupportedByOs. - explicit NonBlockingWriter(const std::int32_t fileHandle, + explicit NonBlockingWriter(const std::string& file_path, + std::size_t max_chunk_size, + score::cpp::pmr::unique_ptr unistd, + score::cpp::pmr::unique_ptr fcntl, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files) noexcept; + + explicit NonBlockingWriter(const std::string& file_path, + std::int32_t file_descriptor, std::size_t max_chunk_size, - score::cpp::pmr::unique_ptr unistd) noexcept; + score::cpp::pmr::unique_ptr unistd, + score::cpp::pmr::unique_ptr fcntl, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files) noexcept; /// \brief method to write buffer contents to the given file handle in non blocking manner with SSIZE_MAX. /// Returns Result::kDone when all the data has been written score::cpp::expected FlushIntoFile() noexcept; - + /// \brief Method to Re initialize the current instance of the non blocking writer to be used to flush another span. void SetSpan(const score::cpp::span& buffer) noexcept; static std::size_t GetMaxChunkSize() noexcept; private: + void RotateLogFile() noexcept; score::cpp::expected InternalFlush(const uint64_t size_to_flush) noexcept; score::cpp::pmr::unique_ptr unistd_; + score::cpp::pmr::unique_ptr fcntl_; - std::int32_t file_handle_; // given file handle to write to it. + std::int32_t file_handle_{-1}; // given file handle to write to it. uint64_t number_of_flushed_bytes_; // last written byte location to be used to continue writing in other flush calls. score::cpp::span buffer_; // the sent buffer to flush data from it to the file. Result buffer_flushed_; // Internal flag used to raise it once the whole buffer is flushed. std::size_t max_chunk_size_; + bool circular_file_logging_; + bool overwrite_log_on_full_; + std::size_t max_log_file_size_bytes_; + uint64_t current_file_position_; + std::size_t no_of_log_files_; + bool delete_old_log_files_; + std::size_t current_log_file_index_{0}; + std::string file_path_; + std::string file_name_; + std::string file_extension_; }; } // namespace detail diff --git a/score/mw/log/detail/file_logging/non_blocking_writer_test.cpp b/score/mw/log/detail/file_logging/non_blocking_writer_test.cpp index 6f05b9c2..618e2076 100644 --- a/score/mw/log/detail/file_logging/non_blocking_writer_test.cpp +++ b/score/mw/log/detail/file_logging/non_blocking_writer_test.cpp @@ -13,6 +13,7 @@ #include "score/mw/log/detail/file_logging/non_blocking_writer.h" #include "score/os/mocklib/unistdmock.h" +#include "score/os/mocklib/fcntlmock.h" #include "gtest/gtest.h" #include @@ -38,23 +39,95 @@ struct NonBlockingWriterTestFixture : ::testing::Test { auto unistd = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); unistd_ = unistd.get(); - writer_ = std::make_unique(kFileDescriptor, max_chunk_size, std::move(unistd)); - // score::os::Unistd::set_testing_instance(unistd_); + auto fcntl = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); + fcntl_ = fcntl.get(); + writer_ = std::make_unique("", max_chunk_size, std::move(unistd), std::move(fcntl), false, false, 0, 1, false); } void TearDown() override { writer_.reset(); unistd_ = nullptr; + fcntl_ = nullptr; } protected: std::unique_ptr writer_; score::os::UnistdMock* unistd_{}; + score::os::FcntlMock* fcntl_{}; std::int32_t kFileDescriptor{}; }; +TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenCircularLoggingExceedsMaxSize) +{ + RecordProperty("ParentRequirement", "SCR-861578"); + RecordProperty("ASIL", "B"); + RecordProperty("Description", "NonBlockingWrite wraps around when circular logging is enabled and max size is reached."); + RecordProperty("TestingTechnique", "Requirements-based test"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + auto unistd = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); + unistd_ = unistd.get(); + auto fcntl = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); + fcntl_ = fcntl.get(); + writer_ = std::make_unique("", max_chunk_size, std::move(unistd), std::move(fcntl), true, true, max_chunk_size * 2, 1, false); + + std::array payload{}; + const score::cpp::span buffer{payload.data(), + static_cast::size_type>(payload.size())}; + + writer_->SetSpan(buffer); + + EXPECT_CALL(*fcntl_, open(_,_,_)).WillOnce(Return(kFileDescriptor)); + EXPECT_CALL(*unistd_, lseek(kFileDescriptor, 0, SEEK_END)).WillOnce(Return(0)); + EXPECT_CALL(*unistd_, write(kFileDescriptor, buffer.data(), max_chunk_size)).WillOnce(Return(max_chunk_size)); + ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); + + EXPECT_CALL(*unistd_, write(kFileDescriptor, &(buffer.data()[max_chunk_size]), max_chunk_size)) + .WillOnce(Return(max_chunk_size)); + ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); + + EXPECT_CALL(*unistd_, lseek(kFileDescriptor, 0, SEEK_SET)).WillOnce(Return(0)); + EXPECT_CALL(*unistd_, write(kFileDescriptor, &(buffer.data()[2*max_chunk_size]), max_chunk_size)) + .WillOnce(Return(max_chunk_size)); + ASSERT_EQ(NonBlockingWriter::Result::kDone, writer_->FlushIntoFile().value()); +} + +TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenNonCircularLoggingExceedsMaxSize) +{ + RecordProperty("ParentRequirement", "SCR-86157TBD"); + RecordProperty("ASIL", "B"); + RecordProperty("Description", "NonBlockingWrite does not wrap around when circular logging is disabled."); + RecordProperty("TestingTechnique", "Requirements-based test"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + auto unistd = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); + unistd_ = unistd.get(); + auto fcntl = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); + fcntl_ = fcntl.get(); + writer_ = std::make_unique("", max_chunk_size, std::move(unistd), std::move(fcntl), false, false, max_chunk_size * 2, 1, false); + + std::array payload{}; + const score::cpp::span buffer{payload.data(), + static_cast::size_type>(payload.size())}; + + writer_->SetSpan(buffer); + + EXPECT_CALL(*fcntl_, open(_,_,_)).WillOnce(Return(kFileDescriptor)); + EXPECT_CALL(*unistd_, write(kFileDescriptor, buffer.data(), max_chunk_size)).WillOnce(Return(max_chunk_size)); + ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); + + EXPECT_CALL(*unistd_, write(kFileDescriptor, &(buffer.data()[max_chunk_size]), max_chunk_size)) + .WillOnce(Return(max_chunk_size)); + ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); + + EXPECT_CALL(*unistd_, lseek(_, _, _)).Times(0); + EXPECT_CALL(*unistd_, write(kFileDescriptor, &(buffer.data()[2*max_chunk_size]), max_chunk_size)) + .WillOnce(Return(max_chunk_size)); + ASSERT_EQ(NonBlockingWriter::Result::kDone, writer_->FlushIntoFile().value()); +} + TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenFlushingTwiceMaxChunkSizeShallReturnTrue) { RecordProperty("ParentRequirement", "SCR-861578"); diff --git a/score/mw/log/detail/file_logging/slot_drainer.cpp b/score/mw/log/detail/file_logging/slot_drainer.cpp index 79780ef5..d53ab23c 100644 --- a/score/mw/log/detail/file_logging/slot_drainer.cpp +++ b/score/mw/log/detail/file_logging/slot_drainer.cpp @@ -26,11 +26,27 @@ namespace detail SlotDrainer::SlotDrainer(std::unique_ptr message_builder, std::shared_ptr> allocator, const std::int32_t file_descriptor, + const std::string& file_path, score::cpp::pmr::unique_ptr unistd, + score::cpp::pmr::unique_ptr fcntl, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files, const std::size_t limit_slots_in_one_cycle) : allocator_(allocator), message_builder_(std::move(message_builder)), - non_blocking_writer_(file_descriptor, NonBlockingWriter::GetMaxChunkSize(), std::move(unistd)), + non_blocking_writer_(file_path, + file_descriptor, + NonBlockingWriter::GetMaxChunkSize(), + std::move(unistd), + std::move(fcntl), + circular_file_logging, + overwrite_log_on_full, + max_log_file_size_bytes, + no_of_log_files, + delete_old_log_files), limit_slots_in_one_cycle_(limit_slots_in_one_cycle) { } diff --git a/score/mw/log/detail/file_logging/slot_drainer.h b/score/mw/log/detail/file_logging/slot_drainer.h index dcd22dd4..cb52fb44 100644 --- a/score/mw/log/detail/file_logging/slot_drainer.h +++ b/score/mw/log/detail/file_logging/slot_drainer.h @@ -46,7 +46,14 @@ class SlotDrainer SlotDrainer(std::unique_ptr message_builder, std::shared_ptr> allocator, const std::int32_t file_descriptor, + const std::string& file_path, score::cpp::pmr::unique_ptr unistd, + score::cpp::pmr::unique_ptr fcntl, + const bool circular_file_logging, + const bool overwrite_log_on_full, + const std::size_t max_log_file_size_bytes, + const std::size_t no_of_log_files, + const bool delete_old_log_files, const std::size_t limit_slots_in_one_cycle = 32UL); SlotDrainer(SlotDrainer&&) noexcept = delete; diff --git a/score/mw/log/detail/file_logging/slot_drainer_test.cpp b/score/mw/log/detail/file_logging/slot_drainer_test.cpp index 07dec5aa..bff61df3 100644 --- a/score/mw/log/detail/file_logging/slot_drainer_test.cpp +++ b/score/mw/log/detail/file_logging/slot_drainer_test.cpp @@ -80,8 +80,13 @@ TEST_F(SlotDrainerFixture, TestOneWriteFileFailurePath) RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); // Given write file error - SlotDrainer unit( - std::move(message_builder), allocator_, file_descriptor_, std::move(unistd_ptr_), kLimitSlotsInOneCycle); + SlotDrainer unit(std::move(message_builder), + allocator_, + file_descriptor_, + std::move(unistd_ptr_), + false, + 0, + kLimitSlotsInOneCycle); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) .WillOnce(Return(OptionalSpan{})) // first unitialized @@ -109,8 +114,13 @@ TEST_F(SlotDrainerFixture, IncompleteWriteFileShouldMakeFlushSpansReturnWouldBlo RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); // Given write file error - SlotDrainer unit( - std::move(message_builder), allocator_, file_descriptor_, std::move(unistd_ptr_), kLimitSlotsInOneCycle); + SlotDrainer unit(std::move(message_builder), + allocator_, + file_descriptor_, + std::move(unistd_ptr_), + false, + 0, + kLimitSlotsInOneCycle); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) .WillOnce(Return(OptionalSpan{})) // first unitialized @@ -143,8 +153,13 @@ TEST_F(SlotDrainerFixture, TestOneSlotOneSpan) RecordProperty("DerivationTechnique", "Generation and analysis of equivalence classes"); // Given one slot flushed - SlotDrainer unit( - std::move(message_builder), allocator_, file_descriptor_, std::move(unistd_ptr_), kLimitSlotsInOneCycle); + SlotDrainer unit(std::move(message_builder), + allocator_, + file_descriptor_, + std::move(unistd_ptr_), + false, + 0, + kLimitSlotsInOneCycle); // Expect sequence of calls to check if any spans from possible previous slots are remaining: EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) @@ -183,10 +198,12 @@ TEST_F(SlotDrainerFixture, TestTooManySlotsForSingleCallShallNotBeAbleToFlushAll // Given SlotDrainer set to limit number of slots processed in one call to: SlotDrainer unit(std::move(message_builder), - allocator_, - file_descriptor_, - std::move(unistd_ptr_), - kLimiNumberOfSlotsProcesssedInOneCall); + allocator_, + file_descriptor_, + std::move(unistd_ptr_), + false, + 0, + kLimiNumberOfSlotsProcesssedInOneCall); // Given 3 slots queued due to 'would block' returns: EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) diff --git a/score/mw/log/detail/recorder_factory.cpp b/score/mw/log/detail/recorder_factory.cpp index a4357890..2b1136b2 100644 --- a/score/mw/log/detail/recorder_factory.cpp +++ b/score/mw/log/detail/recorder_factory.cpp @@ -12,6 +12,8 @@ ********************************************************************************/ #include "score/mw/log/detail/recorder_factory.h" +#include + #include "score/mw/log/detail/composite_recorder.h" #include "score/mw/log/detail/empty_recorder.h" #include "score/mw/log/detail/error.h" diff --git a/score/mw/log/detail/recorder_factory_stub.cpp b/score/mw/log/detail/recorder_factory_stub.cpp index 737f7dbb..0f4ab6dd 100644 --- a/score/mw/log/detail/recorder_factory_stub.cpp +++ b/score/mw/log/detail/recorder_factory_stub.cpp @@ -44,7 +44,12 @@ std::unique_ptr CreateConsoleLoggingBackend(const Configuration& config STDOUT_FILENO, std::move(allocator), score::os::FcntlImpl::Default(memory_resource), - score::os::Unistd::Default(memory_resource)); + score::os::Unistd::Default(memory_resource), + config.IsCircularFileLogging(), + false, // overwrite log on full + config.GetMaxLogFileSizeBytes(), + 1, // no of log files + false); // delete old log files } std::unique_ptr RecorderFactory::CreateFromConfiguration( diff --git a/score/os/BUILD b/score/os/BUILD index 0f9fd688..dbbb92a3 100644 --- a/score/os/BUILD +++ b/score/os/BUILD @@ -906,6 +906,11 @@ cc_library( "@score_baselibs//score/result", ], ) +cc_library( + name = "os", + hdrs = glob(["*.h"]), + visibility = ["//visibility:public"], +) cc_library( name = "version", From 0b033f8c6fdf591eeebbcf9e90be62a1453b1ad4 Mon Sep 17 00:00:00 2001 From: atarekra Date: Wed, 10 Dec 2025 15:40:03 +0200 Subject: [PATCH 2/9] Updating parameters name --- score/json/examples/logging.json | 12 ++-- score/mw/log/README.md | 6 ++ .../configuration/target_config_reader.cpp | 62 +++++++++++++---- .../target_config_reader_test.cpp | 40 +++++++++++ .../file_logging/non_blocking_writer.cpp | 68 ++++++++++++++++++- .../detail/file_logging/non_blocking_writer.h | 1 + 6 files changed, 166 insertions(+), 23 deletions(-) diff --git a/score/json/examples/logging.json b/score/json/examples/logging.json index e752b9f4..ff0e1d76 100644 --- a/score/json/examples/logging.json +++ b/score/json/examples/logging.json @@ -6,9 +6,11 @@ "logMode": "kConsole|kFile", "logLevelThresholdConsole": "kInfo", "logFilePath": "./", - "circularFileLogging": false, - "overwriteLogOnFull": false, - "maxLogFileSizeBytes": 500, - "deleteOldLogFiles": false, - "noOfLogFiles": 2 + "logFileSizePolicy": { + "enabled": false, + "maxFileSizeInBytes": 500, + "numberOfFiles": 2, + "overwriteExistingFiles": false, + "truncateOnRotation": false + } } diff --git a/score/mw/log/README.md b/score/mw/log/README.md index a758e9e1..48a6ec10 100644 --- a/score/mw/log/README.md +++ b/score/mw/log/README.md @@ -102,6 +102,12 @@ in parallel * **logFilePath** -- used for file logging, if ```logMode``` includes ```kFile```, this is the directory, where the logfile ```appId.dlt``` will be put +* **logFileSizePolicy** -- an object that defines the rules for log file rotation. + * **enabled** (`true`/`false`): Enables or disables the file rotation/sizing feature. + * **maxFileSizeInBytes** (integer): The maximum size in bytes that a single log file can reach before rotation is triggered. + * **numberOfFiles** (integer): The total number of log files to use in the rotation cycle (e.g., `appId_1.dlt`, `appId_2.dlt`). + * **overwriteExistingFiles** (`true`/`false`): If `true`, allows the logger to overwrite an existing log file when it rotates back to it. If `false`, rotation will stop if the next file in the cycle already exists. + * **truncateOnRotation** (`true`/`false`): If `true`, the contents of the next log file in the cycle will be deleted (truncated) before new logs are written to it. * **logLevel** -- default value: LogLevel::kWarn, global log level threshold for the application * **logLevelThresholdConsole** -- if ```logMode``` includes ```kConsole```, diff --git a/score/mw/log/configuration/target_config_reader.cpp b/score/mw/log/configuration/target_config_reader.cpp index b231b8a2..0f7be6b4 100644 --- a/score/mw/log/configuration/target_config_reader.cpp +++ b/score/mw/log/configuration/target_config_reader.cpp @@ -48,11 +48,13 @@ constexpr StringLiteral kNumberOfSlotsKey{"numberOfSlots"}; constexpr StringLiteral kSlotSizeBytesKey{"slotSizeBytes"}; constexpr StringLiteral kDatarouterUidKey{"datarouterUid"}; constexpr StringLiteral kDynamicDatarouterIdentifiersKey{"dynamicDatarouterIdentifiers"}; -constexpr StringLiteral kCircularFileLoggingKey{"circularFileLogging"}; -constexpr StringLiteral kMaxLogFileSizeBytesKey{"maxLogFileSizeBytes"}; -constexpr StringLiteral kOverwriteKey{"overwriteLogOnFull"}; -constexpr StringLiteral kNoOfLogFilesKey{"noOfLogFiles"}; -constexpr StringLiteral kDeleteOldLogFilesKey{"deleteOldLogFiles"}; + +constexpr StringLiteral kLogFileSizePolicyKey{"logFileSizePolicy"}; +constexpr StringLiteral kEnabledKey{"enabled"}; +constexpr StringLiteral kMaxFileSizeInBytesKey{"maxFileSizeInBytes"}; +constexpr StringLiteral kNumberOfFilesKey{"numberOfFiles"}; +constexpr StringLiteral kOverwriteExistingFilesKey{"overwriteExistingFiles"}; +constexpr StringLiteral kTruncateOnRotationKey{"truncateOnRotation"}; // Suppress Coverity warning because: // 1. 'constexpr' cannot be used with std::unordered_map. @@ -434,10 +436,16 @@ score::ResultBlank ParseDynamicDatarouterIdentifiers(const score::json::Object& score::ResultBlank ParseCircularFileLogging(const score::json::Object& root, Configuration& config) noexcept { + const auto file_rotation_obj_result = GetElementAsRef(root, kLogFileSizePolicyKey); + if (!file_rotation_obj_result.has_value()) + { + return {}; // The whole fileRotation object is optional. + } + // clang-format off return GetElementAndThen( - root, - kCircularFileLoggingKey, + file_rotation_obj_result.value().get(), + kEnabledKey, [&config](auto value) noexcept { config.SetCircularFileLogging(value); } ); // clang-format on @@ -445,11 +453,17 @@ score::ResultBlank ParseCircularFileLogging(const score::json::Object& root, Con score::ResultBlank ParseMaxLogFileSizeBytes(const score::json::Object& root, Configuration& config) noexcept { + const auto file_rotation_obj_result = GetElementAsRef(root, kLogFileSizePolicyKey); + if (!file_rotation_obj_result.has_value()) + { + return {}; // The whole fileRotation object is optional. + } + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation // clang-format off return GetElementAndThen( - root, - kMaxLogFileSizeBytesKey, + file_rotation_obj_result.value().get(), + kMaxFileSizeInBytesKey, [&config](auto value) noexcept { config.SetMaxLogFileSizeBytes(value); } ); // clang-format on @@ -457,11 +471,17 @@ score::ResultBlank ParseMaxLogFileSizeBytes(const score::json::Object& root, Con score::ResultBlank ParseOverwriteLogOnFull(const score::json::Object& root, Configuration& config) noexcept { + const auto file_rotation_obj_result = GetElementAsRef(root, kLogFileSizePolicyKey); + if (!file_rotation_obj_result.has_value()) + { + return {}; // The whole fileRotation object is optional. + } + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation // clang-format off return GetElementAndThen( - root, - kOverwriteKey, + file_rotation_obj_result.value().get(), + kOverwriteExistingFilesKey, [&config](auto value) noexcept { config.SetOverwriteLogOnFull(value); } ); // clang-format on @@ -469,11 +489,17 @@ score::ResultBlank ParseOverwriteLogOnFull(const score::json::Object& root, Conf score::ResultBlank ParseNoOfLogFiles(const score::json::Object& root, Configuration& config) noexcept { + const auto file_rotation_obj_result = GetElementAsRef(root, kLogFileSizePolicyKey); + if (!file_rotation_obj_result.has_value()) + { + return {}; // The whole fileRotation object is optional. + } + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation // clang-format off return GetElementAndThen( - root, - kNoOfLogFilesKey, + file_rotation_obj_result.value().get(), + kNumberOfFilesKey, [&config](auto value) noexcept { config.SetNoOfLogFiles(value); } ); // clang-format on @@ -481,11 +507,17 @@ score::ResultBlank ParseNoOfLogFiles(const score::json::Object& root, Configurat score::ResultBlank ParseDeleteOldLogFiles(const score::json::Object& root, Configuration& config) noexcept { + const auto file_rotation_obj_result = GetElementAsRef(root, kLogFileSizePolicyKey); + if (!file_rotation_obj_result.has_value()) + { + return {}; // The whole fileRotation object is optional. + } + // Disabling clang-format to address Coverity warning: autosar_cpp14_a7_1_7_violation // clang-format off return GetElementAndThen( - root, - kDeleteOldLogFilesKey, + file_rotation_obj_result.value().get(), + kTruncateOnRotationKey, [&config](auto value) noexcept { config.SetDeleteOldLogFiles(value); } ); // clang-format on diff --git a/score/mw/log/configuration/target_config_reader_test.cpp b/score/mw/log/configuration/target_config_reader_test.cpp index 89c59a29..d8d8cfdb 100644 --- a/score/mw/log/configuration/target_config_reader_test.cpp +++ b/score/mw/log/configuration/target_config_reader_test.cpp @@ -516,6 +516,46 @@ TEST_F(TargetConfigReaderFixture, ConfigReaderShallFallBackToContextLogLevelDefa EXPECT_EQ(GetReader().ReadConfig()->GetContextLogLevel(), ContextLogLevelMap{}); } +TEST_F(TargetConfigReaderFixture, ConfigReaderShallParseFileRotationStructure) +{ + RecordProperty("Requirement", "N/A"); + RecordProperty("ASIL", "B"); + RecordProperty("Description", + "TargetConfigReader shall parse the new nested fileRotation configuration structure correctly."); + RecordProperty("TestingTechnique", "Requirements-based test"); + RecordProperty("DerivationTechnique", "Analysis of requirements"); + + // Create a temporary config file with the new structure + const std::string new_config_content = R"({ + "logFileSizePolicy": { + "enabled": true, + "maxFileSizeInBytes": 12345, + "numberOfFiles": 5, + "overwriteExistingFiles": true, + "truncateOnRotation": true + } + })"; + const std::string temp_file_path = "new_config.json"; + std::ofstream temp_file(temp_file_path); + temp_file << new_config_content; + temp_file.close(); + + SetConfigurationFiles({temp_file_path}); + + const auto config_result = GetReader().ReadConfig(); + ASSERT_TRUE(config_result.has_value()); + const auto& config = config_result.value(); + + EXPECT_TRUE(config.IsCircularFileLogging()); + EXPECT_EQ(config.GetMaxLogFileSizeBytes(), 12345); + EXPECT_EQ(config.GetNoOfLogFiles(), 5); + EXPECT_TRUE(config.IsOverwriteLogOnFull()); + EXPECT_TRUE(config.IsDeleteOldLogFiles()); + + // Clean up the temporary file + std::remove(temp_file_path.c_str()); +} + } // namespace } // namespace detail } // namespace log diff --git a/score/mw/log/detail/file_logging/non_blocking_writer.cpp b/score/mw/log/detail/file_logging/non_blocking_writer.cpp index 7a511f2c..018e63bd 100644 --- a/score/mw/log/detail/file_logging/non_blocking_writer.cpp +++ b/score/mw/log/detail/file_logging/non_blocking_writer.cpp @@ -208,7 +208,15 @@ score::cpp::expected NonBlockingWriter::InternalFlush { if (file_handle_ == -1) { - return 0; // File is not open, do nothing. + if (rotation_failed_) + { + return 0; // A rotation attempt already failed, do nothing. + } + RotateLogFile(); + if (file_handle_ == -1) + { + return 0; // Rotation failed, do nothing. + } } const auto buffer_size = static_cast(buffer_.size()); @@ -223,6 +231,10 @@ score::cpp::expected NonBlockingWriter::InternalFlush // If the file size is limited and the next write would exceed it, rotate the file. if (max_log_file_size_bytes_ > 0 && (current_file_position_ + size_to_flush > max_log_file_size_bytes_)) { + if (rotation_failed_) + { + return 0; // A rotation attempt already failed, do nothing. + } RotateLogFile(); } @@ -333,6 +345,7 @@ score::cpp::expected NonBlockingWriter::InternalFlush if (!seek_result.has_value()) { // If seek fails, we cannot proceed with the circular write. + rotation_failed_ = true; return score::cpp::make_unexpected(seek_result.error()); } @@ -350,6 +363,7 @@ score::cpp::expected NonBlockingWriter::InternalFlush const auto written_count = static_cast(num_of_bytes_written.value()); number_of_flushed_bytes_ += written_count; current_file_position_ = written_count; + rotation_failed_ = false; // Success return num_of_bytes_written.value(); } } @@ -376,13 +390,22 @@ void NonBlockingWriter::RotateLogFile() noexcept std::string next_file_path = file_path_ + "/" + file_name_ + "_" + std::to_string(current_log_file_index_) + file_extension_; // Check for file existence using the Unistd wrapper. - const bool file_exists = unistd_->access(next_file_path.c_str(), score::os::Unistd::AccessMode::kExists).has_value(); + const auto access_result = unistd_->access(next_file_path.c_str(), score::os::Unistd::AccessMode::kExists); + if (!access_result.has_value() && access_result.error() != score::os::Error::Code::kNoSuchFileOrDirectory) + { + rotation_failed_ = true; + const std::string console_warning = "ERROR: Could not access next log file: " + next_file_path + "\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + return; + } + const bool file_exists = access_result.has_value(); if (file_exists && !overwrite_log_on_full_) { const std::string console_warning = "WARNING: Log file rotation stopped. File exists and overwrite is disabled.\n"; unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); current_file_position_ = 0; + rotation_failed_ = true; // Set failure flag return; // Stop, leaving file_handle_ as -1. } @@ -404,16 +427,55 @@ void NonBlockingWriter::RotateLogFile() noexcept if (descriptor_result.has_value()) { file_handle_ = descriptor_result.value(); + + const auto flags = fcntl_->fcntl(file_handle_, score::os::Fcntl::Command::kFileGetStatusFlags); + if (!flags.has_value()) + { + rotation_failed_ = true; + const std::string console_warning = "ERROR: Could not get flags for new log file: " + next_file_path + "\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + unistd_->close(file_handle_); + file_handle_ = -1; + return; + } + + auto fcntl_result = fcntl_->fcntl( + file_handle_, + score::os::Fcntl::Command::kFileSetStatusFlags, + flags.value() | score::os::Fcntl::Open::kNonBlocking | score::os::Fcntl::Open::kCloseOnExec); + + if (!fcntl_result.has_value()) + { + rotation_failed_ = true; + const std::string console_warning = "ERROR: Could not set flags for new log file: " + next_file_path + "\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + unistd_->close(file_handle_); + file_handle_ = -1; + return; + } + + if (file_exists && overwrite_log_on_full_ && !delete_old_log_files_) { - unistd_->lseek(file_handle_, 0, SEEK_SET); + const auto seek_result = unistd_->lseek(file_handle_, 0, SEEK_SET); + if (!seek_result.has_value()) + { + rotation_failed_ = true; + const std::string console_warning = "ERROR: Could not seek to beginning of log file: " + next_file_path + "\n"; + unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + unistd_->close(file_handle_); + file_handle_ = -1; + return; + } } + rotation_failed_ = false; // Rotation fully succeeded } else { // Handle remains -1 from earlier. const std::string console_warning = "ERROR: Could not open next log file: " + next_file_path + "\n"; unistd_->write(STDERR_FILENO, console_warning.c_str(), console_warning.length()); + rotation_failed_ = true; // Set failure flag } current_file_position_ = 0; diff --git a/score/mw/log/detail/file_logging/non_blocking_writer.h b/score/mw/log/detail/file_logging/non_blocking_writer.h index da087114..2d15fe80 100644 --- a/score/mw/log/detail/file_logging/non_blocking_writer.h +++ b/score/mw/log/detail/file_logging/non_blocking_writer.h @@ -101,6 +101,7 @@ class NonBlockingWriter final std::string file_path_; std::string file_name_; std::string file_extension_; + bool rotation_failed_{false}; }; } // namespace detail From f9a1a511a7860eabaccc78f99754c9519e060978 Mon Sep 17 00:00:00 2001 From: atarekra Date: Sun, 25 Jan 2026 11:45:12 +0200 Subject: [PATCH 3/9] rebasing and fixing writing corruption --- score/mw/log/detail/text_recorder/non_blocking_writer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/score/mw/log/detail/text_recorder/non_blocking_writer.cpp b/score/mw/log/detail/text_recorder/non_blocking_writer.cpp index 500b8f57..5d317346 100644 --- a/score/mw/log/detail/text_recorder/non_blocking_writer.cpp +++ b/score/mw/log/detail/text_recorder/non_blocking_writer.cpp @@ -231,11 +231,11 @@ score::cpp::expected NonBlockingWriter::InternalFlush // If the file size is limited and the next write would exceed it, rotate the file. if (max_log_file_size_bytes_ > 0 && (current_file_position_ + size_to_flush > max_log_file_size_bytes_)) { + RotateLogFile(); if (rotation_failed_) { - return 0; // A rotation attempt already failed, do nothing. + return 0; // A rotation attempt just failed, do nothing. } - RotateLogFile(); } // After potential rotation, we write to the current file. From 994bb4082a6b490a5c6dc87af94d6a1f68939291 Mon Sep 17 00:00:00 2001 From: atarekra Date: Sun, 25 Jan 2026 19:57:08 +0200 Subject: [PATCH 4/9] Update naming --- score/mw/log/configuration/configuration.cpp | 8 ++++---- score/mw/log/configuration/configuration.h | 8 ++++---- score/mw/log/configuration/target_config_reader.cpp | 6 +++--- score/mw/log/configuration/target_config_reader_test.cpp | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/score/mw/log/configuration/configuration.cpp b/score/mw/log/configuration/configuration.cpp index b3e10cce..4d75d448 100644 --- a/score/mw/log/configuration/configuration.cpp +++ b/score/mw/log/configuration/configuration.cpp @@ -249,14 +249,14 @@ void Configuration::SetNoOfLogFiles(const std::size_t no_of_log_files) noexcept no_of_log_files_ = no_of_log_files; } -bool Configuration::IsDeleteOldLogFiles() const noexcept +bool Configuration::IsTruncateOnRotation() const noexcept { - return delete_old_log_files_; + return truncate_on_rotation_; } -void Configuration::SetDeleteOldLogFiles(const bool delete_old_log_files) noexcept +void Configuration::SetTruncateOnRotation(const bool truncate_on_rotation) noexcept { - delete_old_log_files_ = delete_old_log_files; + truncate_on_rotation_ = truncate_on_rotation; } } // namespace detail diff --git a/score/mw/log/configuration/configuration.h b/score/mw/log/configuration/configuration.h index babcb1fc..5bc4d8f4 100644 --- a/score/mw/log/configuration/configuration.h +++ b/score/mw/log/configuration/configuration.h @@ -96,8 +96,8 @@ class Configuration final std::size_t GetNoOfLogFiles() const noexcept; void SetNoOfLogFiles(const std::size_t) noexcept; - bool IsDeleteOldLogFiles() const noexcept; - void SetDeleteOldLogFiles(const bool) noexcept; + bool IsTruncateOnRotation() const noexcept; + void SetTruncateOnRotation(const bool) noexcept; /// \brief Returns true if the log level is enabled for the context. /// \param use_console_default_level Set to true if threshold for console logging should be considered as default @@ -166,8 +166,8 @@ class Configuration final /// \brief Number of log files to keep. std::size_t no_of_log_files_{1}; - /// \brief Delete old log files when the maximum number of files is reached. - bool delete_old_log_files_{false}; + /// \brief Truncate on rotation. + bool truncate_on_rotation_{false}; }; } // namespace detail diff --git a/score/mw/log/configuration/target_config_reader.cpp b/score/mw/log/configuration/target_config_reader.cpp index 880f3346..f1331c0d 100644 --- a/score/mw/log/configuration/target_config_reader.cpp +++ b/score/mw/log/configuration/target_config_reader.cpp @@ -507,7 +507,7 @@ score::ResultBlank ParseNoOfLogFiles(const score::json::Object& root, Configurat // clang-format on } -score::ResultBlank ParseDeleteOldLogFiles(const score::json::Object& root, Configuration& config) noexcept +score::ResultBlank ParseTruncateOnRotation(const score::json::Object& root, Configuration& config) noexcept { const auto file_rotation_obj_result = GetElementAsRef(root, kLogFileSizePolicyKey); if (!file_rotation_obj_result.has_value()) @@ -520,7 +520,7 @@ score::ResultBlank ParseDeleteOldLogFiles(const score::json::Object& root, Confi return GetElementAndThen( file_rotation_obj_result.value().get(), kTruncateOnRotationKey, - [&config](auto value) noexcept { config.SetDeleteOldLogFiles(value); } + [&config](auto value) noexcept { config.SetTruncateOnRotation(value); } ); // clang-format on } @@ -546,7 +546,7 @@ void ParseConfigurationElements(const score::json::Object& root, const std::stri ReportOnError(ParseMaxLogFileSizeBytes(root, config), path); ReportOnError(ParseOverwriteLogOnFull(root, config), path); ReportOnError(ParseNoOfLogFiles(root, config), path); - ReportOnError(ParseDeleteOldLogFiles(root, config), path); + ReportOnError(ParseTruncateOnRotation(root, config), path); } // Suppress "AUTOSAR C++14 A15-5-3" rule findings: "The std::terminate() function shall not be called implicitly". diff --git a/score/mw/log/configuration/target_config_reader_test.cpp b/score/mw/log/configuration/target_config_reader_test.cpp index d8d8cfdb..b5d5d94d 100644 --- a/score/mw/log/configuration/target_config_reader_test.cpp +++ b/score/mw/log/configuration/target_config_reader_test.cpp @@ -550,7 +550,7 @@ TEST_F(TargetConfigReaderFixture, ConfigReaderShallParseFileRotationStructure) EXPECT_EQ(config.GetMaxLogFileSizeBytes(), 12345); EXPECT_EQ(config.GetNoOfLogFiles(), 5); EXPECT_TRUE(config.IsOverwriteLogOnFull()); - EXPECT_TRUE(config.IsDeleteOldLogFiles()); + EXPECT_TRUE(config.IsTruncateOnRotation()); // Clean up the temporary file std::remove(temp_file_path.c_str()); From fe5dedfe619befe9c65f7d779af49f7bd32203bd Mon Sep 17 00:00:00 2001 From: atarekra Date: Mon, 26 Jan 2026 12:35:54 +0200 Subject: [PATCH 5/9] Removing unecessary headers --- score/mw/log/detail/text_recorder/non_blocking_writer.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/score/mw/log/detail/text_recorder/non_blocking_writer.cpp b/score/mw/log/detail/text_recorder/non_blocking_writer.cpp index 5d317346..4b7cc50b 100644 --- a/score/mw/log/detail/text_recorder/non_blocking_writer.cpp +++ b/score/mw/log/detail/text_recorder/non_blocking_writer.cpp @@ -11,16 +11,10 @@ * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ #include "score/mw/log/detail/text_recorder/non_blocking_writer.h" - #include "score/filesystem/path.h" #include "score/os/fcntl.h" #include "score/os/stat.h" - -// #include "score/concurrency/clock.h" -// #include "score/mw/log/logging.h" - #include -#include #if defined(__QNXNTO__) #include From 8a2acf934c75a013cb9b6e727534525bf2ecf677 Mon Sep 17 00:00:00 2001 From: atarekra Date: Mon, 26 Jan 2026 15:18:33 +0200 Subject: [PATCH 6/9] Updating naming --- .../log/detail/text_recorder/file_output_backend.cpp | 4 ++-- .../log/detail/text_recorder/file_output_backend.h | 2 +- .../log/detail/text_recorder/non_blocking_writer.cpp | 12 ++++++------ .../log/detail/text_recorder/non_blocking_writer.h | 6 +++--- score/mw/log/detail/text_recorder/slot_drainer.cpp | 4 ++-- score/mw/log/detail/text_recorder/slot_drainer.h | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/score/mw/log/detail/text_recorder/file_output_backend.cpp b/score/mw/log/detail/text_recorder/file_output_backend.cpp index c8c0cc1d..06f9b443 100644 --- a/score/mw/log/detail/text_recorder/file_output_backend.cpp +++ b/score/mw/log/detail/text_recorder/file_output_backend.cpp @@ -33,7 +33,7 @@ FileOutputBackend::FileOutputBackend(std::unique_ptr message_bu const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files) noexcept + const bool truncate_on_rotation) noexcept : Backend(), buffer_allocator_(std::move(allocator)), slot_drainer_(std::move(message_builder), @@ -46,7 +46,7 @@ FileOutputBackend::FileOutputBackend(std::unique_ptr message_bu overwrite_log_on_full, max_log_file_size_bytes, no_of_log_files, - delete_old_log_files) + truncate_on_rotation) { } diff --git a/score/mw/log/detail/text_recorder/file_output_backend.h b/score/mw/log/detail/text_recorder/file_output_backend.h index 3b6531e8..9489f404 100644 --- a/score/mw/log/detail/text_recorder/file_output_backend.h +++ b/score/mw/log/detail/text_recorder/file_output_backend.h @@ -41,7 +41,7 @@ class FileOutputBackend final : public Backend const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files) noexcept; + const bool truncate_on_rotation) noexcept; /// \brief Before a producer can store data in our buffer, he has to reserve a slot. /// /// \return SlotHandle if a slot was able to be reserved, empty otherwise. diff --git a/score/mw/log/detail/text_recorder/non_blocking_writer.cpp b/score/mw/log/detail/text_recorder/non_blocking_writer.cpp index 4b7cc50b..13f120e8 100644 --- a/score/mw/log/detail/text_recorder/non_blocking_writer.cpp +++ b/score/mw/log/detail/text_recorder/non_blocking_writer.cpp @@ -55,7 +55,7 @@ NonBlockingWriter::NonBlockingWriter(const std::string& file_path, const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files) noexcept + const bool truncate_on_rotation) noexcept : unistd_{std::move(unistd)}, fcntl_{std::move(fcntl)}, number_of_flushed_bytes_{0U}, @@ -67,7 +67,7 @@ NonBlockingWriter::NonBlockingWriter(const std::string& file_path, max_log_file_size_bytes_(max_log_file_size_bytes), current_file_position_{0U}, no_of_log_files_(no_of_log_files), - delete_old_log_files_(delete_old_log_files), + truncate_on_rotation_(truncate_on_rotation), current_log_file_index_{1} { const score::filesystem::Path path(file_path); @@ -124,7 +124,7 @@ NonBlockingWriter::NonBlockingWriter(const std::string& file_path, const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files) noexcept + const bool truncate_on_rotation) noexcept : unistd_{std::move(unistd)}, fcntl_{std::move(fcntl)}, file_handle_{file_descriptor}, @@ -137,7 +137,7 @@ NonBlockingWriter::NonBlockingWriter(const std::string& file_path, max_log_file_size_bytes_(max_log_file_size_bytes), current_file_position_{0U}, no_of_log_files_(no_of_log_files), - delete_old_log_files_(delete_old_log_files), + truncate_on_rotation_(truncate_on_rotation), current_log_file_index_{1} { const score::filesystem::Path path(file_path); @@ -407,7 +407,7 @@ void NonBlockingWriter::RotateLogFile() noexcept score::os::Fcntl::Open::kWriteOnly | score::os::Fcntl::Open::kCreate | score::os::Fcntl::Open::kCloseOnExec; // Only truncate if both overwrite and delete are enabled, matching user's expectation. - if (overwrite_log_on_full_ && delete_old_log_files_) + if (overwrite_log_on_full_ && truncate_on_rotation_) { open_flags |= score::os::Fcntl::Open::kTruncate; } @@ -449,7 +449,7 @@ void NonBlockingWriter::RotateLogFile() noexcept } - if (file_exists && overwrite_log_on_full_ && !delete_old_log_files_) + if (file_exists && overwrite_log_on_full_ && !truncate_on_rotation_) { const auto seek_result = unistd_->lseek(file_handle_, 0, SEEK_SET); if (!seek_result.has_value()) diff --git a/score/mw/log/detail/text_recorder/non_blocking_writer.h b/score/mw/log/detail/text_recorder/non_blocking_writer.h index 2d15fe80..ccd9b812 100644 --- a/score/mw/log/detail/text_recorder/non_blocking_writer.h +++ b/score/mw/log/detail/text_recorder/non_blocking_writer.h @@ -56,7 +56,7 @@ class NonBlockingWriter final const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files) noexcept; + const bool truncate_on_rotation) noexcept; explicit NonBlockingWriter(const std::string& file_path, std::int32_t file_descriptor, @@ -67,7 +67,7 @@ class NonBlockingWriter final const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files) noexcept; + const bool truncate_on_rotation) noexcept; /// \brief method to write buffer contents to the given file handle in non blocking manner with SSIZE_MAX. /// Returns Result::kDone when all the data has been written @@ -96,7 +96,7 @@ class NonBlockingWriter final std::size_t max_log_file_size_bytes_; uint64_t current_file_position_; std::size_t no_of_log_files_; - bool delete_old_log_files_; + bool truncate_on_rotation_; std::size_t current_log_file_index_{0}; std::string file_path_; std::string file_name_; diff --git a/score/mw/log/detail/text_recorder/slot_drainer.cpp b/score/mw/log/detail/text_recorder/slot_drainer.cpp index 0bd8fa1d..65c58437 100644 --- a/score/mw/log/detail/text_recorder/slot_drainer.cpp +++ b/score/mw/log/detail/text_recorder/slot_drainer.cpp @@ -33,7 +33,7 @@ SlotDrainer::SlotDrainer(std::unique_ptr message_builder, const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files, + const bool truncate_on_rotation, const std::size_t limit_slots_in_one_cycle) : allocator_(allocator), message_builder_(std::move(message_builder)), @@ -46,7 +46,7 @@ SlotDrainer::SlotDrainer(std::unique_ptr message_builder, overwrite_log_on_full, max_log_file_size_bytes, no_of_log_files, - delete_old_log_files), + truncate_on_rotation), limit_slots_in_one_cycle_(limit_slots_in_one_cycle) { } diff --git a/score/mw/log/detail/text_recorder/slot_drainer.h b/score/mw/log/detail/text_recorder/slot_drainer.h index bce04123..34826756 100644 --- a/score/mw/log/detail/text_recorder/slot_drainer.h +++ b/score/mw/log/detail/text_recorder/slot_drainer.h @@ -53,7 +53,7 @@ class SlotDrainer const bool overwrite_log_on_full, const std::size_t max_log_file_size_bytes, const std::size_t no_of_log_files, - const bool delete_old_log_files, + const bool truncate_on_rotation, const std::size_t limit_slots_in_one_cycle = 32UL); SlotDrainer(SlotDrainer&&) noexcept = delete; From 6844dbb4e1253b31da972220a07db898f4b33833 Mon Sep 17 00:00:00 2001 From: atarekra Date: Mon, 26 Jan 2026 16:07:50 +0200 Subject: [PATCH 7/9] Fixing Tests failure --- score/mw/log/detail/recorder_factory_stub.cpp | 1 + score/mw/log/detail/text_recorder/BUILD | 1 + .../file_output_backend_test.cpp | 6 +++++ .../non_blocking_writer_test.cpp | 2 +- .../text_recorder/slot_drainer_test.cpp | 26 +++++++++++++++++++ 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/score/mw/log/detail/recorder_factory_stub.cpp b/score/mw/log/detail/recorder_factory_stub.cpp index 911e50f4..e62140bc 100644 --- a/score/mw/log/detail/recorder_factory_stub.cpp +++ b/score/mw/log/detail/recorder_factory_stub.cpp @@ -42,6 +42,7 @@ std::unique_ptr CreateConsoleLoggingBackend(const Configuration& config LogRecord{config.GetSlotSizeInBytes()}); return std::make_unique(std::move(message_builder), STDOUT_FILENO, + std::string{}, std::move(allocator), score::os::FcntlImpl::Default(memory_resource), score::os::Unistd::Default(memory_resource), diff --git a/score/mw/log/detail/text_recorder/BUILD b/score/mw/log/detail/text_recorder/BUILD index 8d6e362b..6ceace1c 100644 --- a/score/mw/log/detail/text_recorder/BUILD +++ b/score/mw/log/detail/text_recorder/BUILD @@ -215,6 +215,7 @@ cc_test( deps = [ ":non_blocking_writer", "@score_baselibs//score/os/mocklib:unistd_mock", + "@score_baselibs//score/os/mocklib:fcntl_mock", #"@score_baselibs//score/mw/log/test/console_logging_environment", "@googletest//:gtest_main", ], diff --git a/score/mw/log/detail/text_recorder/file_output_backend_test.cpp b/score/mw/log/detail/text_recorder/file_output_backend_test.cpp index 69e3add2..ff7f1487 100644 --- a/score/mw/log/detail/text_recorder/file_output_backend_test.cpp +++ b/score/mw/log/detail/text_recorder/file_output_backend_test.cpp @@ -79,6 +79,7 @@ TEST_F(FileOutputBackendFixture, ReserveSlotShouldTriggerFlushing) auto unistd_mock = score::cpp::pmr::make_unique(memory_resource_); FileOutputBackend unit(std::move(message_builder), file_descriptor_, + std::string{}, std::move(allocator_), std::move(fcntl_mock), std::move(unistd_mock), @@ -110,6 +111,7 @@ TEST_F(FileOutputBackendFixture, FlushSlotShouldTriggerFlushing) const auto& slot_index = allocator_->AcquireSlotToWrite(); FileOutputBackend unit(std::move(message_builder), file_descriptor_, + std::string{}, std::move(allocator_), std::move(fcntl_mock), std::move(unistd_mock), @@ -148,6 +150,7 @@ TEST_F(FileOutputBackendFixture, DepletedAllocatorShouldCauseEmptyOptionalReturn FileOutputBackend unit(std::move(message_builder), file_descriptor_, + std::string{}, std::move(allocator_), std::move(fcntl_mock), std::move(unistd_mock), @@ -177,6 +180,7 @@ TEST_F(FileOutputBackendFixture, GetLogRecordReturnsObjectSameAsAllocatorWould) auto unistd_mock = score::cpp::pmr::make_unique(memory_resource_); FileOutputBackend unit(std::move(message_builder), file_descriptor_, + std::string{}, std::move(allocator_), std::move(fcntl_mock), std::move(unistd_mock), @@ -222,6 +226,7 @@ TEST_F(FileOutputBackendFixture, BackendConstructionShallCallNonBlockingFileSetu // Given construction FileOutputBackend unit(std::move(message_builder), file_descriptor_, + std::string{}, std::move(allocator_), std::move(fcntl_mock), std::move(unistd_mock), @@ -254,6 +259,7 @@ TEST_F(FileOutputBackendFixture, MissingFlagsShallSkipCallToSetupFile) // Given construction FileOutputBackend unit(std::move(message_builder), file_descriptor_, + std::string{}, std::move(allocator_), std::move(fcntl_mock), std::move(unistd_mock), diff --git a/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp b/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp index cf54f3a5..22c2f4a4 100644 --- a/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp +++ b/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp @@ -13,7 +13,7 @@ #include "score/mw/log/detail/text_recorder/non_blocking_writer.h" #include "score/os/mocklib/unistdmock.h" -#include "score/os/mocklib/fcntlmock.h" +#include "score/os/mocklib/fcntl_mock.h" #include "gtest/gtest.h" #include diff --git a/score/mw/log/detail/text_recorder/slot_drainer_test.cpp b/score/mw/log/detail/text_recorder/slot_drainer_test.cpp index 7702b668..6b7b9148 100644 --- a/score/mw/log/detail/text_recorder/slot_drainer_test.cpp +++ b/score/mw/log/detail/text_recorder/slot_drainer_test.cpp @@ -13,6 +13,7 @@ #include "score/mw/log/detail/text_recorder/slot_drainer.h" #include "score/os/mocklib/unistdmock.h" +#include "score/os/mocklib/fcntl_mock.h" #include "score/mw/log/detail/error.h" #include "score/mw/log/detail/text_recorder/mock/message_builder_mock.h" @@ -48,6 +49,9 @@ class SlotDrainerFixture : public ::testing::Test unistd_ptr_ = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); unistd_mock_ = unistd_ptr_.get(); + fcntl_ptr_ = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); + fcntl_mock_ = fcntl_ptr_.get(); + allocator_ = std::make_unique>(pool_size_); message_builder_mock_ = std::make_unique(); @@ -65,6 +69,8 @@ class SlotDrainerFixture : public ::testing::Test score::cpp::pmr::unique_ptr<::score::os::UnistdMock> unistd_ptr_{}; ::score::os::UnistdMock* unistd_mock_{}; + score::cpp::pmr::unique_ptr<::score::os::FcntlMock> fcntl_ptr_{}; + ::score::os::FcntlMock* fcntl_mock_{}; std::unique_ptr message_builder = nullptr; std::shared_ptr> allocator_ = nullptr; std::int32_t file_descriptor_ = 23; // random number file descriptor @@ -83,9 +89,14 @@ TEST_F(SlotDrainerFixture, TestOneWriteFileFailurePath) SlotDrainer unit(std::move(message_builder), allocator_, file_descriptor_, + std::string{}, std::move(unistd_ptr_), + std::move(fcntl_ptr_), + false, false, 0, + 1, + false, kLimitSlotsInOneCycle); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) @@ -117,9 +128,14 @@ TEST_F(SlotDrainerFixture, IncompleteWriteFileShouldMakeFlushSpansReturnWouldBlo SlotDrainer unit(std::move(message_builder), allocator_, file_descriptor_, + std::string{}, std::move(unistd_ptr_), + std::move(fcntl_ptr_), + false, false, 0, + 1, + false, kLimitSlotsInOneCycle); EXPECT_CALL(*raw_message_builder_mock_, GetNextSpan) @@ -156,9 +172,14 @@ TEST_F(SlotDrainerFixture, TestOneSlotOneSpan) SlotDrainer unit(std::move(message_builder), allocator_, file_descriptor_, + std::string{}, std::move(unistd_ptr_), + std::move(fcntl_ptr_), + false, false, 0, + 1, + false, kLimitSlotsInOneCycle); // Expect sequence of calls to check if any spans from possible previous slots are remaining: @@ -200,9 +221,14 @@ TEST_F(SlotDrainerFixture, TestTooManySlotsForSingleCallShallNotBeAbleToFlushAll SlotDrainer unit(std::move(message_builder), allocator_, file_descriptor_, + std::string{}, std::move(unistd_ptr_), + std::move(fcntl_ptr_), + false, false, 0, + 1, + false, kLimiNumberOfSlotsProcesssedInOneCall); // Given 3 slots queued due to 'would block' returns: From 2a22f3963d7f405fd825302d705c64775d0c3a7f Mon Sep 17 00:00:00 2001 From: atarekra Date: Tue, 27 Jan 2026 11:36:55 +0200 Subject: [PATCH 8/9] Fixing test failure --- .../text_recorder/non_blocking_writer_test.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp b/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp index 22c2f4a4..33c2d8a3 100644 --- a/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp +++ b/score/mw/log/detail/text_recorder/non_blocking_writer_test.cpp @@ -71,6 +71,12 @@ TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenCircularLoggingExceeds unistd_ = unistd.get(); auto fcntl = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); fcntl_ = fcntl.get(); + + EXPECT_CALL(*fcntl_, open(_, _, _)).WillOnce(Return(kFileDescriptor)); + EXPECT_CALL(*fcntl_, fcntl(kFileDescriptor, score::os::Fcntl::Command::kFileGetStatusFlags)).WillOnce(Return(score::os::Fcntl::Open{})); + EXPECT_CALL(*fcntl_, fcntl(kFileDescriptor, score::os::Fcntl::Command::kFileSetStatusFlags, _)).WillOnce(Return(score::cpp::expected_blank())); + EXPECT_CALL(*unistd_, lseek(kFileDescriptor, 0, SEEK_END)).WillOnce(Return(0)); + writer_ = std::make_unique("", max_chunk_size, std::move(unistd), std::move(fcntl), true, true, max_chunk_size * 2, 1, false); std::array payload{}; @@ -79,8 +85,6 @@ TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenCircularLoggingExceeds writer_->SetSpan(buffer); - EXPECT_CALL(*fcntl_, open(_,_,_)).WillOnce(Return(kFileDescriptor)); - EXPECT_CALL(*unistd_, lseek(kFileDescriptor, 0, SEEK_END)).WillOnce(Return(0)); EXPECT_CALL(*unistd_, write(kFileDescriptor, buffer.data(), max_chunk_size)).WillOnce(Return(max_chunk_size)); ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); @@ -88,6 +92,7 @@ TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenCircularLoggingExceeds .WillOnce(Return(max_chunk_size)); ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); + EXPECT_CALL(*unistd_, write(STDERR_FILENO, _, _)).WillOnce(Return(0)); EXPECT_CALL(*unistd_, lseek(kFileDescriptor, 0, SEEK_SET)).WillOnce(Return(0)); EXPECT_CALL(*unistd_, write(kFileDescriptor, &(buffer.data()[2*max_chunk_size]), max_chunk_size)) .WillOnce(Return(max_chunk_size)); @@ -106,6 +111,11 @@ TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenNonCircularLoggingExce unistd_ = unistd.get(); auto fcntl = score::cpp::pmr::make_unique(score::cpp::pmr::get_default_resource()); fcntl_ = fcntl.get(); + + EXPECT_CALL(*fcntl_, open(_, _, _)).WillOnce(Return(kFileDescriptor)); + EXPECT_CALL(*fcntl_, fcntl(kFileDescriptor, score::os::Fcntl::Command::kFileGetStatusFlags)).WillOnce(Return(score::os::Fcntl::Open{})); + EXPECT_CALL(*fcntl_, fcntl(kFileDescriptor, score::os::Fcntl::Command::kFileSetStatusFlags, _)).WillOnce(Return(score::cpp::expected_blank())); + writer_ = std::make_unique("", max_chunk_size, std::move(unistd), std::move(fcntl), false, false, max_chunk_size * 2, 1, false); std::array payload{}; @@ -114,7 +124,6 @@ TEST_F(NonBlockingWriterTestFixture, NonBlockingWriterWhenNonCircularLoggingExce writer_->SetSpan(buffer); - EXPECT_CALL(*fcntl_, open(_,_,_)).WillOnce(Return(kFileDescriptor)); EXPECT_CALL(*unistd_, write(kFileDescriptor, buffer.data(), max_chunk_size)).WillOnce(Return(max_chunk_size)); ASSERT_EQ(NonBlockingWriter::Result::kWouldBlock, writer_->FlushIntoFile().value()); From 1d0b2c495ebf466092b9cb8eab322b30a7b72142 Mon Sep 17 00:00:00 2001 From: atarekra Date: Tue, 27 Jan 2026 12:41:05 +0200 Subject: [PATCH 9/9] Fixing format check test failing --- score/mw/log/detail/text_recorder/BUILD | 4 ++-- score/os/BUILD | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/score/mw/log/detail/text_recorder/BUILD b/score/mw/log/detail/text_recorder/BUILD index 6ceace1c..8a7f18dc 100644 --- a/score/mw/log/detail/text_recorder/BUILD +++ b/score/mw/log/detail/text_recorder/BUILD @@ -98,11 +98,11 @@ cc_library( "//visibility:public", ], deps = [ - "@score_baselibs//score/filesystem:filesystem", + "@score_baselibs//score/filesystem", "@score_baselibs//score/language/futurecpp", "@score_baselibs//score/mw/log/detail:types_and_errors", + "@score_baselibs//score/os", "@score_baselibs//score/os:fcntl", - "@score_baselibs//score/os:os", "@score_baselibs//score/os:unistd", ], ) diff --git a/score/os/BUILD b/score/os/BUILD index 7b135f76..f83e0fa3 100644 --- a/score/os/BUILD +++ b/score/os/BUILD @@ -906,6 +906,7 @@ cc_library( "@score_baselibs//score/result", ], ) + cc_library( name = "os", hdrs = glob(["*.h"]),