This is an automated email from the ASF dual-hosted git repository. szaszm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
commit 034ff6cbfb718e3224ee7b0966d7b759803179d7 Author: Gabor Gyimesi <[email protected]> AuthorDate: Tue Aug 8 13:26:33 2023 +0200 MINIFICPP-2165 Add logger max log entry length configuration property Closes #1621 Signed-off-by: Marton Szasz <[email protected]> --- conf/minifi-log.properties | 3 + docker/conf/minifi-log.properties | 3 + .../standard-processors/processors/LogAttribute.h | 1 + libminifi/include/core/logging/Logger.h | 15 ++--- .../include/core/logging/LoggerConfiguration.h | 3 + libminifi/include/properties/Configuration.h | 1 + libminifi/src/Configuration.cpp | 1 + libminifi/src/core/logging/Logger.cpp | 1 - libminifi/src/core/logging/LoggerConfiguration.cpp | 15 +++++ libminifi/test/unit/LoggerTests.cpp | 71 ++++++++++++++++++++++ 10 files changed, 106 insertions(+), 8 deletions(-) diff --git a/conf/minifi-log.properties b/conf/minifi-log.properties index 87fb9694f..aeb4e8bc8 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 (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 6b01d285e..dbc3212e6 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 (use "unlimited" or "-1" for no limit) +#max.log.entry.length=1024 diff --git a/extensions/standard-processors/processors/LogAttribute.h b/extensions/standard-processors/processors/LogAttribute.h index 03467c63f..18e222d3d 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 91b90686c..fc0c71081 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<size_t>(result); if (buf_size <= LOG_BUFFER_SIZE) { // static buffer was large enough - return {buf, buf_size}; - } - if (max_size >= 0 && gsl::narrow<size_t>(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<size_t>(max_size)) : buf_size}; } + // try to use dynamic buffer size_t dynamic_buffer_size = max_size < 0 ? buf_size : gsl::narrow<size_t>(std::min(result, max_size)); std::vector<char> 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<size_t>(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 c12465ae8..6dfd7effb 100644 --- a/libminifi/include/core/logging/LoggerConfiguration.h +++ b/libminifi/include/core/logging/LoggerConfiguration.h @@ -64,6 +64,8 @@ struct LoggerNamespace { inline std::optional<std::string> formatId(std::optional<utils::Identifier> opt_id) { return opt_id | utils::map([](auto id) { return " (" + std::string(id.to_string()) + ")"; }); } + +inline constexpr std::string_view UNLIMITED_LOG_ENTRY_LENGTH = "unlimited"; } // namespace internal class LoggerConfiguration { @@ -161,6 +163,7 @@ class LoggerConfiguration { std::shared_ptr<LoggerImpl> logger_ = nullptr; std::shared_ptr<LoggerControl> controller_; std::unordered_set<std::shared_ptr<AlertSink>> alert_sinks_; + std::optional<int> 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 14b0af4b9..1bda9ab9b 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 93f691ca1..7ac5f12a3 100644 --- a/libminifi/src/Configuration.cpp +++ b/libminifi/src/Configuration.cpp @@ -133,6 +133,7 @@ const std::unordered_map<std::string_view, gsl::not_null<const core::PropertyVal {Configuration::nifi_log_logger_root, gsl::make_not_null(&core::StandardPropertyTypes::VALID_TYPE)}, {Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(&core::StandardPropertyTypes::DATA_SIZE_TYPE)}, {Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(&core::StandardPropertyTypes::DATA_SIZE_TYPE)}, + {Configuration::nifi_log_max_log_entry_length, gsl::make_not_null(&core::StandardPropertyTypes::VALID_TYPE)}, {Configuration::nifi_log_alert_url, gsl::make_not_null(&core::StandardPropertyTypes::VALID_TYPE)}, {Configuration::nifi_log_alert_ssl_context_service, gsl::make_not_null(&core::StandardPropertyTypes::VALID_TYPE)}, {Configuration::nifi_log_alert_batch_size, gsl::make_not_null(&core::StandardPropertyTypes::VALID_TYPE)}, diff --git a/libminifi/src/core/logging/Logger.cpp b/libminifi/src/core/logging/Logger.cpp index 02425b2ec..8a751890f 100644 --- a/libminifi/src/core/logging/Logger.cpp +++ b/libminifi/src/core/logging/Logger.cpp @@ -37,7 +37,6 @@ void LoggerControl::setEnabled(bool status) { is_enabled_ = status; } - BaseLogger::~BaseLogger() = default; bool BaseLogger::should_log(const LOG_LEVEL& /*level*/) { diff --git a/libminifi/src/core/logging/LoggerConfiguration.cpp b/libminifi/src/core/logging/LoggerConfiguration.cpp index e7dcd70b1..87d74ff58 100644 --- a/libminifi/src/core/logging/LoggerConfiguration.cpp +++ b/libminifi/src/core/logging/LoggerConfiguration.cpp @@ -132,6 +132,18 @@ void LoggerConfiguration::initialize(const std::shared_ptr<LoggerProperties> &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 { + if (internal::UNLIMITED_LOG_ENTRY_LENGTH == *max_log_entry_length_str) { + max_log_entry_length_ = -1; + } else { + 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()); + } + } + formatter_ = std::make_shared<spdlog::pattern_formatter>(spdlog_pattern); std::map<std::string, std::shared_ptr<spdlog::logger>> spdloggers; for (auto const & logger_impl : loggers) { @@ -167,6 +179,9 @@ std::shared_ptr<Logger> LoggerConfiguration::getLogger(std::string_view name, co std::shared_ptr<LoggerImpl> result = std::make_shared<LoggerImpl>(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(gsl::narrow<int>(*max_log_entry_length_)); + } return result; } diff --git a/libminifi/test/unit/LoggerTests.cpp b/libminifi/test/unit/LoggerTests.cpp index 7a2e2ec65..00b7c84e7 100644 --- a/libminifi/test/unit/LoggerTests.cpp +++ b/libminifi/test/unit/LoggerTests.cpp @@ -278,3 +278,74 @@ 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<logging::LoggerProperties>(); + 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<InputStream> 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<logging::LoggerProperties>(); + 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<InputStream> 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<logging::LoggerProperties>(); + 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<InputStream> 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 unlimited results in unlimited log entry size", "[ttl15]") { + auto& log_config = logging::LoggerConfiguration::getConfiguration(); + auto properties = std::make_shared<logging::LoggerProperties>(); + 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"); + std::string log(5000, 'a'); + logger->log_error(log.c_str()); + + std::shared_ptr<InputStream> compressed_log{logging::LoggerConfiguration::getCompressedLog(true)}; + REQUIRE(compressed_log); + auto logs = decompress(compressed_log); + REQUIRE(logs.find(log) != std::string::npos); +}
