szaszm commented on code in PR #1920:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1920#discussion_r1954764926
##########
libminifi/include/core/logging/LoggerBase.h:
##########
@@ -109,6 +140,7 @@ class LoggerBase : public Logger {
private:
std::atomic<int> max_log_size_{LOG_BUFFER_SIZE};
+ std::vector<std::function<void(LOG_LEVEL level, const std::string&)>>
log_callbacks_;
Review Comment:
do we need multiple callbacks? If not, I'd say keep it simple and only have
one.
##########
minifi-api/include/minifi-cpp/core/logging/Logger.h:
##########
@@ -124,7 +126,8 @@ class Logger {
if (!should_log(level)) {
return;
}
- log_string(level, stringify(std::move(fmt),
map_args(std::forward<Args>(args))...));
+ auto message = stringify(std::move(fmt),
map_args(std::forward<Args>(args))...);
+ log_string(level, message);
Review Comment:
why this change? it pessimizes the code by forcing another copy
##########
libminifi/include/core/logging/LoggerBase.h:
##########
@@ -80,6 +80,36 @@ inline LOG_LEVEL
mapFromSpdLogLevel(spdlog::level::level_enum level) {
throw std::invalid_argument(fmt::format("Invalid spdlog::level::level_enum
{}", magic_enum::enum_underlying(level)));
}
+inline std::string mapLogLevelToString(LOG_LEVEL level) {
+ switch (level) {
+ case trace: return "TRACE";
+ case debug: return "DEBUG";
+ case info: return "INFO";
+ case warn: return "WARN";
+ case err: return "ERROR";
+ case critical: return "CRITICAL";
+ case off: return "OFF";
+ }
+ throw std::invalid_argument(fmt::format("Invalid LOG_LEVEL {}",
magic_enum::enum_underlying(level)));
+}
+
+inline LOG_LEVEL mapStringToLogLevel(const std::string& level_str) {
+ if (level_str == "TRACE") {
+ return trace;
+ } else if (level_str == "DEBUG") {
+ return debug;
+ } else if (level_str == "INFO") {
+ return info;
+ } else if (level_str == "WARN") {
+ return warn;
+ } else if (level_str == "ERROR") {
+ return err;
+ } else if (level_str == "CRITICAL") {
+ return critical;
+ }
+ throw std::invalid_argument(fmt::format("Invalid LOG_LEVEL {}", level_str));
+}
Review Comment:
This mapping is already defined elsewhere, check LogUtils.h (and maybe
rename it if you have a better idea)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
