From ee437407f4d7b604ba4257abec0817a1ee8495f9 Mon Sep 17 00:00:00 2001 From: Rustam Gamidov Date: Tue, 4 Nov 2025 08:52:59 +0200 Subject: [PATCH 1/3] Add censor API to Log It allows to set values to be masked from the log messages at runtime Relates-To: NLAM-140 Signed-off-by: Rustam Gamidov --- .../include/olp/core/logging/Log.h | 10 ++ olp-cpp-sdk-core/src/logging/Log.cpp | 109 ++++++++++++++---- olp-cpp-sdk-core/tests/logging/LogTest.cpp | 36 ++++++ 3 files changed, 135 insertions(+), 20 deletions(-) diff --git a/olp-cpp-sdk-core/include/olp/core/logging/Log.h b/olp-cpp-sdk-core/include/olp/core/logging/Log.h index f25ae6760..c5a9e27a4 100644 --- a/olp-cpp-sdk-core/include/olp/core/logging/Log.h +++ b/olp-cpp-sdk-core/include/olp/core/logging/Log.h @@ -562,6 +562,16 @@ class CORE_API Log { const std::string& message, const char* file, unsigned int line, const char* function, const char* fullFunction); + + /** + * @brief Adds a line to be censored out from the log. + * + * Censoring out is a replacement with a predefiend mask with length not + * correlated with the original line length. + * + * @param message The line to be censored out from the log. + */ + static void censor(const std::string& message); }; } // namespace logging } // namespace olp diff --git a/olp-cpp-sdk-core/src/logging/Log.cpp b/olp-cpp-sdk-core/src/logging/Log.cpp index 2fff401e4..5c579fc75 100644 --- a/olp-cpp-sdk-core/src/logging/Log.cpp +++ b/olp-cpp-sdk-core/src/logging/Log.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -30,6 +31,15 @@ namespace olp { namespace logging { + +namespace { +constexpr auto kSecretMask = "*****"; +} + +struct LogMessageExt : public LogMessage { + olp::porting::optional adjusted_message; +}; + class LogImpl { public: friend class olp::thread::Atomic; @@ -67,14 +77,20 @@ class LogImpl { unsigned int line, const char* function, const char* fullFunction); + void censor(std::string msg); + private: LogImpl(); template void appendLogItem(const LogItem& log_item); + template + void censorLogItem(LogItem& log_item, const std::string& original); + Configuration m_configuration; std::unordered_map m_logLevels; Level m_defaultLevel; + std::vector m_toCensor; }; LogImpl::LogImpl() @@ -120,7 +136,8 @@ void LogImpl::setLevel(Level level, const std::string& tag) { } porting::optional LogImpl::getLevel(const std::string& tag) const { - if (tag.empty()) return getLevel(); + if (tag.empty()) + return getLevel(); auto foundIter = m_logLevels.find(tag); if (foundIter == m_logLevels.end()) @@ -130,7 +147,8 @@ porting::optional LogImpl::getLevel(const std::string& tag) const { } void LogImpl::clearLevel(const std::string& tag) { - if (tag.empty()) return; + if (tag.empty()) + return; m_logLevels.erase(tag); } @@ -138,13 +156,15 @@ void LogImpl::clearLevel(const std::string& tag) { void LogImpl::clearLevels() { m_logLevels.clear(); } bool LogImpl::isEnabled(Level level) const { - if (level == Level::Off) return false; + if (level == Level::Off) + return false; return static_cast(level) >= static_cast(m_defaultLevel); } bool LogImpl::isEnabled(Level level, const std::string& tag) const { - if (level == Level::Off) return false; + if (level == Level::Off) + return false; auto foundIter = m_logLevels.find(tag); Level targetLevel; @@ -159,7 +179,7 @@ void LogImpl::logMessage(Level level, const std::string& tag, const std::string& message, const char* file, unsigned int line, const char* function, const char* fullFunction) { - LogMessage logMessage; + LogMessageExt logMessage; logMessage.level = level; logMessage.tag = tag.c_str(); logMessage.message = message.c_str(); @@ -170,6 +190,7 @@ void LogImpl::logMessage(Level level, const std::string& tag, logMessage.time = std::chrono::system_clock::now(); logMessage.threadId = getThreadId(); + censorLogItem(logMessage, message); appendLogItem(logMessage); } @@ -181,11 +202,38 @@ void LogImpl::appendLogItem(const LogItem& log_item) { } } +template +void LogImpl::censorLogItem(LogItem& log_item, const std::string& original) { + bool has_copy = log_item.adjusted_message.has_value(); + const std::string& src = + has_copy ? log_item.adjusted_message.value() : original; + + for (const std::string& secret : m_toCensor) { + auto found_pos = src.find(secret); + while (found_pos != std::string::npos) { + if (!has_copy) { + log_item.adjusted_message = original; + has_copy = true; + log_item.message = log_item.adjusted_message.value().c_str(); + } + log_item.adjusted_message.value().replace(found_pos, secret.length(), + kSecretMask); + found_pos = log_item.adjusted_message.value().find( + secret, found_pos + std::strlen(kSecretMask)); + } + } +} + +void LogImpl::censor(std::string msg) { + m_toCensor.emplace_back(std::move(msg)); +} + // implementation of public static Log API //-------------------------------------------------------- bool Log::configure(Configuration configuration) { - if (!LogImpl::aliveStatus()) return false; + if (!LogImpl::aliveStatus()) + return false; return LogImpl::getInstance().locked([&configuration](LogImpl& log) { return log.configure(std::move(configuration)); @@ -193,27 +241,31 @@ bool Log::configure(Configuration configuration) { } Configuration Log::getConfiguration() { - if (!LogImpl::aliveStatus()) return Configuration(); + if (!LogImpl::aliveStatus()) + return Configuration(); return LogImpl::getInstance().locked( [](const LogImpl& log) { return log.getConfiguration(); }); } void Log::setLevel(Level level) { - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked([level](LogImpl& log) { log.setLevel(level); }); } Level Log::getLevel() { - if (!LogImpl::aliveStatus()) return Level::Off; + if (!LogImpl::aliveStatus()) + return Level::Off; return LogImpl::getInstance().locked( [](const LogImpl& log) { return log.getLevel(); }); } void Log::setLevel(Level level, const std::string& tag) { - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked( [level, &tag](LogImpl& log) { log.setLevel(level, tag); }); @@ -221,55 +273,64 @@ void Log::setLevel(Level level, const std::string& tag) { void Log::setLevel(const std::string& level, const std::string& tag) { auto log_level = FilterGroup::stringToLevel(level); - if (!log_level) return; + if (!log_level) + return; - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked( [&log_level, &tag](LogImpl& log) { log.setLevel(*log_level, tag); }); } porting::optional Log::getLevel(const std::string& tag) { - if (!LogImpl::aliveStatus()) return porting::none; + if (!LogImpl::aliveStatus()) + return porting::none; return LogImpl::getInstance().locked( [&tag](const LogImpl& log) { return log.getLevel(tag); }); } void Log::clearLevel(const std::string& tag) { - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked([&tag](LogImpl& log) { log.clearLevel(tag); }); } void Log::clearLevels() { - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked([](LogImpl& log) { log.clearLevels(); }); } void Log::applyFilterGroup(const FilterGroup& filters) { - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked([&filters](LogImpl& log) { log.clearLevels(); log.clearLevels(); auto defaultLevel = filters.getLevel(); - if (defaultLevel) log.setLevel(*defaultLevel); + if (defaultLevel) + log.setLevel(*defaultLevel); for (const auto& tagLevel : filters.m_tagLevels) log.setLevel(tagLevel.second, tagLevel.first); }); } bool Log::isEnabled(Level level) { - if (!LogImpl::aliveStatus()) return false; + if (!LogImpl::aliveStatus()) + return false; return LogImpl::getInstance().locked( [level](const LogImpl& log) { return log.isEnabled(level); }); } bool Log::isEnabled(Level level, const std::string& tag) { - if (!LogImpl::aliveStatus()) return false; + if (!LogImpl::aliveStatus()) + return false; return LogImpl::getInstance().locked( [level, &tag](const LogImpl& log) { return log.isEnabled(level, tag); }); @@ -279,12 +340,20 @@ void Log::logMessage(Level level, const std::string& tag, const std::string& message, const char* file, unsigned int line, const char* function, const char* fullFunction) { - if (!LogImpl::aliveStatus()) return; + if (!LogImpl::aliveStatus()) + return; LogImpl::getInstance().locked([&](LogImpl& log) { log.logMessage(level, tag, message, file, line, function, fullFunction); }); } +void Log::censor(const std::string& message) { + if (!LogImpl::aliveStatus()) + return; + + LogImpl::getInstance().locked([&](LogImpl& log) { log.censor(message); }); +} + } // namespace logging } // namespace olp diff --git a/olp-cpp-sdk-core/tests/logging/LogTest.cpp b/olp-cpp-sdk-core/tests/logging/LogTest.cpp index d3c8d11e5..c8e1a7ce7 100644 --- a/olp-cpp-sdk-core/tests/logging/LogTest.cpp +++ b/olp-cpp-sdk-core/tests/logging/LogTest.cpp @@ -409,6 +409,42 @@ TEST(LogTest, LogFormat) { ++index; } +TEST(LogTest, LogCensor) { + auto appender = std::make_shared(); + olp::logging::Configuration configuration; + configuration.addAppender(appender); + EXPECT_TRUE(olp::logging::Log::configure(configuration)); + olp::logging::Log::setLevel(olp::logging::Level::Trace); + + constexpr auto secret_01 = "1st secret"; + constexpr auto secret_02 = "to hide"; + constexpr auto secret_mask = "******"; + + olp::logging::Log::censor(secret_01); + olp::logging::Log::censor(secret_02); + olp::logging::Log::censor(secret_mask); + + OLP_SDK_LOG_INFO_F("", "Nothing to censor"); + OLP_SDK_LOG_INFO_F("", "Inlined1st secretin message"); + OLP_SDK_LOG_TRACE_F("trace", "%s %s", "plain", secret_01); + OLP_SDK_LOG_DEBUG_F("debug", "%s %s%s", "no spaces", secret_02, "**"); + OLP_SDK_LOG_INFO_F("info", "%s %s %s", secret_02, "several", secret_01); + OLP_SDK_LOG_WARNING_F("warning", "%s %s %s", secret_02, "twice", secret_02); + OLP_SDK_LOG_ERROR_F("error", "%s %s%s", "no loop", secret_mask, secret_mask); + OLP_SDK_LOG_FATAL_F("fatal", "%s %s", "Fatal", secret_02); + + ASSERT_EQ(8U, appender->messages_.size()); + + EXPECT_EQ("Nothing to censor", appender->messages_[0].message_); + EXPECT_EQ("Inlined*****in message", appender->messages_[1].message_); + EXPECT_EQ("plain *****", appender->messages_[2].message_); + EXPECT_EQ("no spaces *******", appender->messages_[3].message_); + EXPECT_EQ("***** several *****", appender->messages_[4].message_); + EXPECT_EQ("***** twice *****", appender->messages_[5].message_); + EXPECT_EQ("no loop **********", appender->messages_[6].message_); + EXPECT_EQ("Fatal *****", appender->messages_[7].message_); +} + TEST(LogTest, LogLimits) { auto appender = std::make_shared(); olp::logging::Configuration configuration; From b09408c1684a6c75e79fb11c839e0587cd9baffe Mon Sep 17 00:00:00 2001 From: Rustam Gamidov Date: Tue, 4 Nov 2025 13:08:00 +0200 Subject: [PATCH 2/3] Extend Logs tests caused by formatting changes Codecov prefers to have patch config to be not lower than the rest of repository. The file has not been formatted properly before so there are changes not related to the censoring Relates-To: NLAM-140 Signed-off-by: Rustam Gamidov --- olp-cpp-sdk-core/tests/logging/LogTest.cpp | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/olp-cpp-sdk-core/tests/logging/LogTest.cpp b/olp-cpp-sdk-core/tests/logging/LogTest.cpp index c8e1a7ce7..211779e51 100644 --- a/olp-cpp-sdk-core/tests/logging/LogTest.cpp +++ b/olp-cpp-sdk-core/tests/logging/LogTest.cpp @@ -43,8 +43,9 @@ TEST(LogTest, Levels) { EXPECT_TRUE(olp::logging::Log::isEnabled(olp::logging::Level::Info)); EXPECT_FALSE(olp::logging::Log::isEnabled(olp::logging::Level::Debug)); - olp::logging::Log::setLevel(olp::logging::Level::Debug, "test1"); + olp::logging::Log::setLevel("Debug", "test1"); olp::logging::Log::setLevel(olp::logging::Level::Warning, "test2"); + olp::logging::Log::setLevel("WrongLevel", "test1"); EXPECT_EQ(olp::logging::Level::Debug, olp::logging::Log::getLevel("test1")); EXPECT_EQ(olp::logging::Level::Warning, olp::logging::Log::getLevel("test2")); @@ -73,6 +74,14 @@ TEST(LogTest, Levels) { EXPECT_FALSE( olp::logging::Log::isEnabled(olp::logging::Level::Debug, "test2")); + olp::logging::Log::clearLevel(""); + EXPECT_EQ(olp::logging::Level::Debug, olp::logging::Log::getLevel("test1")); + EXPECT_EQ(olp::porting::none, olp::logging::Log::getLevel("test2")); + EXPECT_TRUE( + olp::logging::Log::isEnabled(olp::logging::Level::Warning, "test2")); + EXPECT_FALSE( + olp::logging::Log::isEnabled(olp::logging::Level::Debug, "test2")); + olp::logging::Log::clearLevels(); EXPECT_EQ(olp::porting::none, olp::logging::Log::getLevel("test1")); EXPECT_EQ(olp::porting::none, olp::logging::Log::getLevel("test2")); @@ -664,6 +673,22 @@ TEST(LogTest, LogLevelOff) { ++index; } +TEST(LogTest, IsEnabled_ReturnsFalse_WhenLogLevelOff_) { + auto appender = std::make_shared(); + olp::logging::Configuration configuration; + configuration.addAppender(appender); + EXPECT_TRUE(olp::logging::Log::configure(configuration)); + + EXPECT_FALSE(olp::logging::Log::isEnabled(olp::logging::Level::Off)); + olp::logging::Log::setLevel(olp::logging::Level::Off); + EXPECT_FALSE(olp::logging::Log::isEnabled(olp::logging::Level::Off)); + + constexpr auto tag = "random tag"; + EXPECT_FALSE(olp::logging::Log::isEnabled(olp::logging::Level::Off, tag)); + olp::logging::Log::setLevel(olp::logging::Level::Off, tag); + EXPECT_FALSE(olp::logging::Log::isEnabled(olp::logging::Level::Off, tag)); +} + TEST(LogTest, ReConfigure) { auto appender1 = std::make_shared(); auto appender2 = std::make_shared(); From f3e3968b0254ed9328fd1744957a0a79839b2674 Mon Sep 17 00:00:00 2001 From: Rustam Gamidov Date: Wed, 5 Nov 2025 12:52:12 +0200 Subject: [PATCH 3/3] Add removing censor API to Log There are secrets that can be updated with time and it make sense not to censor them anymore not to let performance suffer. Relates-To: NLAM-140 Signed-off-by: Rustam Gamidov --- .../include/olp/core/logging/Log.h | 12 +++++++- olp-cpp-sdk-core/src/logging/Log.cpp | 30 +++++++++++++++---- olp-cpp-sdk-core/tests/logging/LogTest.cpp | 16 ++++++++-- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/olp-cpp-sdk-core/include/olp/core/logging/Log.h b/olp-cpp-sdk-core/include/olp/core/logging/Log.h index c5a9e27a4..1c5d9c4e8 100644 --- a/olp-cpp-sdk-core/include/olp/core/logging/Log.h +++ b/olp-cpp-sdk-core/include/olp/core/logging/Log.h @@ -571,7 +571,17 @@ class CORE_API Log { * * @param message The line to be censored out from the log. */ - static void censor(const std::string& message); + static void addCensor(const std::string& message); + + /** + * @brief Removes a line from censoring out from the log. + * + * Censoring out is a replacement with a predefiend mask with length not + * correlated with the original line length. + * + * @param message The line to be excluded from censoring out from the log. + */ + static void removeCensor(const std::string& message); }; } // namespace logging } // namespace olp diff --git a/olp-cpp-sdk-core/src/logging/Log.cpp b/olp-cpp-sdk-core/src/logging/Log.cpp index 5c579fc75..7a368587c 100644 --- a/olp-cpp-sdk-core/src/logging/Log.cpp +++ b/olp-cpp-sdk-core/src/logging/Log.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -34,7 +35,8 @@ namespace logging { namespace { constexpr auto kSecretMask = "*****"; -} +const auto kSecretMaskLength = std::strlen(kSecretMask); +} // namespace struct LogMessageExt : public LogMessage { olp::porting::optional adjusted_message; @@ -77,7 +79,8 @@ class LogImpl { unsigned int line, const char* function, const char* fullFunction); - void censor(std::string msg); + void addCensor(std::string msg); + void removeCensor(const std::string& msg); private: LogImpl(); @@ -219,15 +222,22 @@ void LogImpl::censorLogItem(LogItem& log_item, const std::string& original) { log_item.adjusted_message.value().replace(found_pos, secret.length(), kSecretMask); found_pos = log_item.adjusted_message.value().find( - secret, found_pos + std::strlen(kSecretMask)); + secret, found_pos + kSecretMaskLength); } } } -void LogImpl::censor(std::string msg) { +void LogImpl::addCensor(std::string msg) { m_toCensor.emplace_back(std::move(msg)); } +void LogImpl::removeCensor(const std::string& msg) { + auto it = std::find(m_toCensor.begin(), m_toCensor.end(), msg); + if (it != m_toCensor.end()) { + m_toCensor.erase(it); + } +} + // implementation of public static Log API //-------------------------------------------------------- @@ -348,11 +358,19 @@ void Log::logMessage(Level level, const std::string& tag, }); } -void Log::censor(const std::string& message) { +void Log::addCensor(const std::string& message) { + if (!LogImpl::aliveStatus()) + return; + + LogImpl::getInstance().locked([&](LogImpl& log) { log.addCensor(message); }); +} + +void Log::removeCensor(const std::string& message) { if (!LogImpl::aliveStatus()) return; - LogImpl::getInstance().locked([&](LogImpl& log) { log.censor(message); }); + LogImpl::getInstance().locked( + [&](LogImpl& log) { log.removeCensor(message); }); } } // namespace logging diff --git a/olp-cpp-sdk-core/tests/logging/LogTest.cpp b/olp-cpp-sdk-core/tests/logging/LogTest.cpp index 211779e51..1bdc10051 100644 --- a/olp-cpp-sdk-core/tests/logging/LogTest.cpp +++ b/olp-cpp-sdk-core/tests/logging/LogTest.cpp @@ -429,9 +429,9 @@ TEST(LogTest, LogCensor) { constexpr auto secret_02 = "to hide"; constexpr auto secret_mask = "******"; - olp::logging::Log::censor(secret_01); - olp::logging::Log::censor(secret_02); - olp::logging::Log::censor(secret_mask); + olp::logging::Log::addCensor(secret_01); + olp::logging::Log::addCensor(secret_02); + olp::logging::Log::addCensor(secret_mask); OLP_SDK_LOG_INFO_F("", "Nothing to censor"); OLP_SDK_LOG_INFO_F("", "Inlined1st secretin message"); @@ -452,6 +452,16 @@ TEST(LogTest, LogCensor) { EXPECT_EQ("***** twice *****", appender->messages_[5].message_); EXPECT_EQ("no loop **********", appender->messages_[6].message_); EXPECT_EQ("Fatal *****", appender->messages_[7].message_); + + olp::logging::Log::removeCensor("not exisiting secret"); + OLP_SDK_LOG_TRACE_F("trace", "%s %s", "plain", secret_01); + olp::logging::Log::removeCensor(secret_01); + OLP_SDK_LOG_TRACE_F("trace", "%s %s", "plain", secret_01); + + ASSERT_EQ(10U, appender->messages_.size()); + + EXPECT_EQ("plain *****", appender->messages_[8].message_); + EXPECT_EQ("plain 1st secret", appender->messages_[9].message_); } TEST(LogTest, LogLimits) {