From f107d8b5df4fb911e89faeb98d347779e0296206 Mon Sep 17 00:00:00 2001 From: Gabor Gyimesi Date: Fri, 28 Jul 2023 16:39:40 +0200 Subject: [PATCH 1/4] MINIFICPP-2165 Add logger max log entry length configuration property --- conf/minifi-log.properties | 3 + docker/conf/minifi-log.properties | 3 + .../processors/LogAttribute.h | 1 + libminifi/include/core/logging/Logger.h | 15 +++-- .../core/logging/LoggerConfiguration.h | 1 + libminifi/include/properties/Configuration.h | 1 + libminifi/src/Configuration.cpp | 1 + libminifi/src/core/logging/Logger.cpp | 1 - .../src/core/logging/LoggerConfiguration.cpp | 11 ++++ libminifi/test/unit/LoggerTests.cpp | 66 +++++++++++++++++++ 10 files changed, 95 insertions(+), 8 deletions(-) diff --git a/conf/minifi-log.properties b/conf/minifi-log.properties index 87fb9694f6..dfc5f50e9d 100644 --- a/conf/minifi-log.properties +++ b/conf/minifi-log.properties @@ -68,3 +68,6 @@ logger.org::apache::nifi::minifi=INFO,rolling ## Setting any of these to 0 disables the in-memory log compression. #compression.cached.log.max.size=8 MB #compression.compressed.log.max.size=8 MB + +## Maximum length of a MiNiFi log entry +#max.log.entry.length=1024 diff --git a/docker/conf/minifi-log.properties b/docker/conf/minifi-log.properties index 6b01d285ea..1373ccf73a 100644 --- a/docker/conf/minifi-log.properties +++ b/docker/conf/minifi-log.properties @@ -57,3 +57,6 @@ logger.org::apache::nifi::minifi=INFO,stderr ## Setting any of these to 0 disables the in-memory log compression. #compression.cached.log.max.size=8 MB #compression.compressed.log.max.size=8 MB + +## Maximum length of a MiNiFi log entry +#max.log.entry.length=1024 diff --git a/extensions/standard-processors/processors/LogAttribute.h b/extensions/standard-processors/processors/LogAttribute.h index 03467c63fc..18e222d3d6 100644 --- a/extensions/standard-processors/processors/LogAttribute.h +++ b/extensions/standard-processors/processors/LogAttribute.h @@ -42,6 +42,7 @@ class LogAttribute : public core::Processor { public: explicit LogAttribute(std::string name, const utils::Identifier& uuid = {}) : Processor(std::move(name), uuid) { + logger_->set_max_log_size(-1); } ~LogAttribute() override = default; diff --git a/libminifi/include/core/logging/Logger.h b/libminifi/include/core/logging/Logger.h index 91b90686c5..fc0c71081d 100644 --- a/libminifi/include/core/logging/Logger.h +++ b/libminifi/include/core/logging/Logger.h @@ -81,12 +81,9 @@ std::string format_string(int max_size, char const* format_str, const Args& ...a const auto buf_size = gsl::narrow(result); if (buf_size <= LOG_BUFFER_SIZE) { // static buffer was large enough - return {buf, buf_size}; - } - if (max_size >= 0 && gsl::narrow(max_size) <= LOG_BUFFER_SIZE) { - // static buffer was already larger than allowed, use the filled buffer - return {buf, LOG_BUFFER_SIZE}; + return {buf, max_size >= 0 ? std::min(buf_size, gsl::narrow(max_size)) : buf_size}; } + // try to use dynamic buffer size_t dynamic_buffer_size = max_size < 0 ? buf_size : gsl::narrow(std::min(result, max_size)); std::vector buffer(dynamic_buffer_size + 1); // extra '\0' character @@ -97,8 +94,12 @@ std::string format_string(int max_size, char const* format_str, const Args& ...a return {buffer.cbegin(), buffer.cend() - 1}; // -1 to not include the terminating '\0' } -inline std::string format_string(int /*max_size*/, char const* format_str) { - return format_str; +inline std::string format_string(int max_size, char const* format_str) { + std::string return_value(format_str); + if (max_size >= 0 && return_value.size() > gsl::narrow(max_size)) { + return return_value.substr(0, max_size); + } + return return_value; } enum LOG_LEVEL { diff --git a/libminifi/include/core/logging/LoggerConfiguration.h b/libminifi/include/core/logging/LoggerConfiguration.h index c12465ae8e..6ebd8e560a 100644 --- a/libminifi/include/core/logging/LoggerConfiguration.h +++ b/libminifi/include/core/logging/LoggerConfiguration.h @@ -161,6 +161,7 @@ class LoggerConfiguration { std::shared_ptr logger_ = nullptr; std::shared_ptr controller_; std::unordered_set> alert_sinks_; + std::optional max_log_entry_length_; bool shorten_names_ = false; bool include_uuid_ = true; }; diff --git a/libminifi/include/properties/Configuration.h b/libminifi/include/properties/Configuration.h index 14b0af4b91..1bda9ab9be 100644 --- a/libminifi/include/properties/Configuration.h +++ b/libminifi/include/properties/Configuration.h @@ -166,6 +166,7 @@ class Configuration : public Properties { static constexpr const char *nifi_log_logger_root = "nifi.log.logger.root"; static constexpr const char *nifi_log_compression_cached_log_max_size = "nifi.log.compression.cached.log.max.size"; static constexpr const char *nifi_log_compression_compressed_log_max_size = "nifi.log.compression.compressed.log.max.size"; + static constexpr const char *nifi_log_max_log_entry_length = "nifi.log.max.log.entry.length"; // alert options static constexpr const char *nifi_log_alert_url = "nifi.log.alert.url"; diff --git a/libminifi/src/Configuration.cpp b/libminifi/src/Configuration.cpp index 93f691ca19..216005cf41 100644 --- a/libminifi/src/Configuration.cpp +++ b/libminifi/src/Configuration.cpp @@ -133,6 +133,7 @@ const std::unordered_map &lo include_uuid_ = utils::StringUtils::toBool(*include_uuid_str).value_or(true); } + if (const auto max_log_entry_length_str = logger_properties->getString("max.log.entry.length")) { + try { + max_log_entry_length_ = std::stoull(*max_log_entry_length_str); + } catch (const std::exception& ex) { + logger_->log_debug("Parsing max log entry length property failed with the following exception: %s", ex.what()); + } + } + formatter_ = std::make_shared(spdlog_pattern); std::map> spdloggers; for (auto const & logger_impl : loggers) { @@ -167,6 +175,9 @@ std::shared_ptr LoggerConfiguration::getLogger(std::string_view name, co std::shared_ptr result = std::make_shared(adjusted_name, id_if_enabled, controller_, get_logger(logger_, root_namespace_, adjusted_name, formatter_)); loggers.push_back(result); + if (max_log_entry_length_) { + result->set_max_log_size(*max_log_entry_length_); + } return result; } diff --git a/libminifi/test/unit/LoggerTests.cpp b/libminifi/test/unit/LoggerTests.cpp index 7a2e2ec650..ea53e26e90 100644 --- a/libminifi/test/unit/LoggerTests.cpp +++ b/libminifi/test/unit/LoggerTests.cpp @@ -278,3 +278,69 @@ TEST_CASE("Setting either properties to 0 disables in-memory compressed logs", " logger->log_error("Hi there"); REQUIRE((logging::LoggerConfiguration::getCompressedLog(true) == nullptr) == is_nullptr); } + +TEST_CASE("Setting max log entry length property trims long log entries", "[ttl12]") { + auto& log_config = logging::LoggerConfiguration::getConfiguration(); + auto properties = std::make_shared(); + properties->set("max.log.entry.length", "2"); + properties->set("logger.root", "INFO"); + log_config.initialize(properties); + auto logger = log_config.getLogger("SetMaxLogEntryLengthTestLogger"); + logger->log_error("Hi there"); + + std::shared_ptr compressed_log{logging::LoggerConfiguration::getCompressedLog(true)}; + REQUIRE(compressed_log); + auto logs = decompress(compressed_log); + REQUIRE(logs.find("Hi ") == std::string::npos); + REQUIRE(logs.find("Hi") != std::string::npos); +} + +TEST_CASE("Setting max log entry length property trims long formatted log entries", "[ttl13]") { + auto& log_config = logging::LoggerConfiguration::getConfiguration(); + auto properties = std::make_shared(); + properties->set("max.log.entry.length", "2"); + properties->set("logger.root", "INFO"); + log_config.initialize(properties); + auto logger = log_config.getLogger("SetMaxLogEntryLengthTestLogger"); + logger->log_error("Hi there %s", "John"); + + std::shared_ptr compressed_log{logging::LoggerConfiguration::getCompressedLog(true)}; + REQUIRE(compressed_log); + auto logs = decompress(compressed_log); + REQUIRE(logs.find("Hi ") == std::string::npos); + REQUIRE(logs.find("Hi") != std::string::npos); +} + +TEST_CASE("Setting max log entry length to a size larger than the internal buffer size", "[ttl14]") { + auto& log_config = logging::LoggerConfiguration::getConfiguration(); + auto properties = std::make_shared(); + properties->set("max.log.entry.length", "1500"); + properties->set("logger.root", "INFO"); + log_config.initialize(properties); + auto logger = log_config.getLogger("SetMaxLogEntryLengthTestLogger"); + std::string log(2000, 'a'); + std::string expected_log(1500, 'a'); + logger->log_error(log.c_str()); + + std::shared_ptr compressed_log{logging::LoggerConfiguration::getCompressedLog(true)}; + REQUIRE(compressed_log); + auto logs = decompress(compressed_log); + REQUIRE(logs.find(log) == std::string::npos); + REQUIRE(logs.find(expected_log) != std::string::npos); +} + +TEST_CASE("Setting max log entry length to negative number results in unlimited log entry size", "[ttl15]") { + auto& log_config = logging::LoggerConfiguration::getConfiguration(); + auto properties = std::make_shared(); + properties->set("max.log.entry.length", "-1"); + properties->set("logger.root", "INFO"); + log_config.initialize(properties); + auto logger = log_config.getLogger("SetMaxLogEntryLengthTestLogger"); + std::string log(5000, 'a'); + logger->log_error(log.c_str()); + + std::shared_ptr compressed_log{logging::LoggerConfiguration::getCompressedLog(true)}; + REQUIRE(compressed_log); + auto logs = decompress(compressed_log); + REQUIRE(logs.find(log) != std::string::npos); +} From baa69e91b21586d96926ffb46a967d0cc83dd934 Mon Sep 17 00:00:00 2001 From: Gabor Gyimesi Date: Thu, 3 Aug 2023 11:38:44 +0200 Subject: [PATCH 2/4] Review update --- conf/minifi-log.properties | 2 +- docker/conf/minifi-log.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conf/minifi-log.properties b/conf/minifi-log.properties index dfc5f50e9d..b304c637f2 100644 --- a/conf/minifi-log.properties +++ b/conf/minifi-log.properties @@ -69,5 +69,5 @@ logger.org::apache::nifi::minifi=INFO,rolling #compression.cached.log.max.size=8 MB #compression.compressed.log.max.size=8 MB -## Maximum length of a MiNiFi log entry +## Maximum length of a MiNiFi log entry, use -1 for unlimited #max.log.entry.length=1024 diff --git a/docker/conf/minifi-log.properties b/docker/conf/minifi-log.properties index 1373ccf73a..f319facb62 100644 --- a/docker/conf/minifi-log.properties +++ b/docker/conf/minifi-log.properties @@ -58,5 +58,5 @@ logger.org::apache::nifi::minifi=INFO,stderr #compression.cached.log.max.size=8 MB #compression.compressed.log.max.size=8 MB -## Maximum length of a MiNiFi log entry +## Maximum length of a MiNiFi log entry, use -1 for unlimited #max.log.entry.length=1024 From 9133ea4134b5fc1c68c09c000211e69a0eebf2a0 Mon Sep 17 00:00:00 2001 From: Gabor Gyimesi Date: Thu, 3 Aug 2023 14:44:22 +0200 Subject: [PATCH 3/4] Allow "unlimited" value --- conf/minifi-log.properties | 2 +- docker/conf/minifi-log.properties | 2 +- libminifi/include/core/logging/LoggerConfiguration.h | 4 +++- libminifi/src/Configuration.cpp | 2 +- libminifi/src/core/logging/LoggerConfiguration.cpp | 10 +++++++--- libminifi/test/unit/LoggerTests.cpp | 9 +++++++-- 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/conf/minifi-log.properties b/conf/minifi-log.properties index b304c637f2..aeb4e8bc85 100644 --- a/conf/minifi-log.properties +++ b/conf/minifi-log.properties @@ -69,5 +69,5 @@ logger.org::apache::nifi::minifi=INFO,rolling #compression.cached.log.max.size=8 MB #compression.compressed.log.max.size=8 MB -## Maximum length of a MiNiFi log entry, use -1 for unlimited +## Maximum length of a MiNiFi log entry (use "unlimited" or "-1" for no limit) #max.log.entry.length=1024 diff --git a/docker/conf/minifi-log.properties b/docker/conf/minifi-log.properties index f319facb62..dbc3212e64 100644 --- a/docker/conf/minifi-log.properties +++ b/docker/conf/minifi-log.properties @@ -58,5 +58,5 @@ logger.org::apache::nifi::minifi=INFO,stderr #compression.cached.log.max.size=8 MB #compression.compressed.log.max.size=8 MB -## Maximum length of a MiNiFi log entry, use -1 for unlimited +## Maximum length of a MiNiFi log entry (use "unlimited" or "-1" for no limit) #max.log.entry.length=1024 diff --git a/libminifi/include/core/logging/LoggerConfiguration.h b/libminifi/include/core/logging/LoggerConfiguration.h index 6ebd8e560a..c9cefeba59 100644 --- a/libminifi/include/core/logging/LoggerConfiguration.h +++ b/libminifi/include/core/logging/LoggerConfiguration.h @@ -64,6 +64,8 @@ struct LoggerNamespace { inline std::optional formatId(std::optional opt_id) { return opt_id | utils::map([](auto id) { return " (" + std::string(id.to_string()) + ")"; }); } + +constexpr std::string_view UNLIMITED_LOG_ENTRY_LENGTH = "unlimited"; } // namespace internal class LoggerConfiguration { @@ -161,7 +163,7 @@ class LoggerConfiguration { std::shared_ptr logger_ = nullptr; std::shared_ptr controller_; std::unordered_set> alert_sinks_; - std::optional max_log_entry_length_; + std::optional max_log_entry_length_; bool shorten_names_ = false; bool include_uuid_ = true; }; diff --git a/libminifi/src/Configuration.cpp b/libminifi/src/Configuration.cpp index 216005cf41..7ac5f12a3e 100644 --- a/libminifi/src/Configuration.cpp +++ b/libminifi/src/Configuration.cpp @@ -133,7 +133,7 @@ const std::unordered_map &lo if (const auto max_log_entry_length_str = logger_properties->getString("max.log.entry.length")) { try { - max_log_entry_length_ = std::stoull(*max_log_entry_length_str); + if (internal::UNLIMITED_LOG_ENTRY_LENGTH == *max_log_entry_length_str) { + max_log_entry_length_ = -1; + } else { + max_log_entry_length_ = std::stoull(*max_log_entry_length_str); + } } catch (const std::exception& ex) { - logger_->log_debug("Parsing max log entry length property failed with the following exception: %s", ex.what()); + logger_->log_error("Parsing max log entry length property failed with the following exception: %s", ex.what()); } } @@ -176,7 +180,7 @@ std::shared_ptr LoggerConfiguration::getLogger(std::string_view name, co std::shared_ptr result = std::make_shared(adjusted_name, id_if_enabled, controller_, get_logger(logger_, root_namespace_, adjusted_name, formatter_)); loggers.push_back(result); if (max_log_entry_length_) { - result->set_max_log_size(*max_log_entry_length_); + result->set_max_log_size(gsl::narrow(*max_log_entry_length_)); } return result; } diff --git a/libminifi/test/unit/LoggerTests.cpp b/libminifi/test/unit/LoggerTests.cpp index ea53e26e90..00b7c84e71 100644 --- a/libminifi/test/unit/LoggerTests.cpp +++ b/libminifi/test/unit/LoggerTests.cpp @@ -329,10 +329,15 @@ TEST_CASE("Setting max log entry length to a size larger than the internal buffe REQUIRE(logs.find(expected_log) != std::string::npos); } -TEST_CASE("Setting max log entry length to negative number results in unlimited log entry size", "[ttl15]") { +TEST_CASE("Setting max log entry length to unlimited results in unlimited log entry size", "[ttl15]") { auto& log_config = logging::LoggerConfiguration::getConfiguration(); auto properties = std::make_shared(); - properties->set("max.log.entry.length", "-1"); + SECTION("Use unlimited value") { + properties->set("max.log.entry.length", "unlimited"); + } + SECTION("Use -1 value") { + properties->set("max.log.entry.length", "-1"); + } properties->set("logger.root", "INFO"); log_config.initialize(properties); auto logger = log_config.getLogger("SetMaxLogEntryLengthTestLogger"); From 2d8abc50728273fc8407d05e406ae5ebb1799914 Mon Sep 17 00:00:00 2001 From: Gabor Gyimesi Date: Fri, 4 Aug 2023 15:13:39 +0200 Subject: [PATCH 4/4] Review update --- libminifi/include/core/logging/LoggerConfiguration.h | 2 +- libminifi/src/core/logging/LoggerConfiguration.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libminifi/include/core/logging/LoggerConfiguration.h b/libminifi/include/core/logging/LoggerConfiguration.h index c9cefeba59..6dfd7effb5 100644 --- a/libminifi/include/core/logging/LoggerConfiguration.h +++ b/libminifi/include/core/logging/LoggerConfiguration.h @@ -65,7 +65,7 @@ inline std::optional formatId(std::optional opt_ return opt_id | utils::map([](auto id) { return " (" + std::string(id.to_string()) + ")"; }); } -constexpr std::string_view UNLIMITED_LOG_ENTRY_LENGTH = "unlimited"; +inline constexpr std::string_view UNLIMITED_LOG_ENTRY_LENGTH = "unlimited"; } // namespace internal class LoggerConfiguration { diff --git a/libminifi/src/core/logging/LoggerConfiguration.cpp b/libminifi/src/core/logging/LoggerConfiguration.cpp index c2e2e74ef5..87d74ff587 100644 --- a/libminifi/src/core/logging/LoggerConfiguration.cpp +++ b/libminifi/src/core/logging/LoggerConfiguration.cpp @@ -137,7 +137,7 @@ void LoggerConfiguration::initialize(const std::shared_ptr &lo if (internal::UNLIMITED_LOG_ENTRY_LENGTH == *max_log_entry_length_str) { max_log_entry_length_ = -1; } else { - max_log_entry_length_ = std::stoull(*max_log_entry_length_str); + max_log_entry_length_ = std::stoi(*max_log_entry_length_str); } } catch (const std::exception& ex) { logger_->log_error("Parsing max log entry length property failed with the following exception: %s", ex.what());