martinzink commented on code in PR #1718:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1718#discussion_r1457821386
##########
libminifi/src/core/logging/LoggerConfiguration.cpp:
##########
@@ -267,52 +257,56 @@ std::shared_ptr<internal::LoggerNamespace>
LoggerConfiguration::initialize_names
return root_namespace;
}
-std::shared_ptr<spdlog::logger> LoggerConfiguration::get_logger(const
std::shared_ptr<Logger>& logger, const
std::shared_ptr<internal::LoggerNamespace> &root_namespace, std::string_view
name_view,
- const
std::shared_ptr<spdlog::formatter>& formatter, bool remove_if_present) {
- std::string name{name_view};
- std::shared_ptr<spdlog::logger> spdlogger = spdlog::get(name);
- if (spdlogger) {
- if (remove_if_present) {
- spdlog::drop(name);
- } else {
- return spdlogger;
- }
+std::shared_ptr<spdlog::logger> LoggerConfiguration::get_logger(const
std::shared_ptr<internal::LoggerNamespace> &root_namespace, const
std::string_view name_view,
+ const
std::shared_ptr<spdlog::formatter>& formatter) {
+ const std::string name{name_view};
+ if (auto spdlogger = spdlog::get(name)) {
Review Comment:
Makes sense, included this in
https://github.com/apache/nifi-minifi-cpp/pull/1718/commits/d1d9008ca3f4edca4cc0cf847978f536581aa3ae
##########
libminifi/src/core/logging/LoggerConfiguration.cpp:
##########
@@ -267,52 +257,56 @@ std::shared_ptr<internal::LoggerNamespace>
LoggerConfiguration::initialize_names
return root_namespace;
}
-std::shared_ptr<spdlog::logger> LoggerConfiguration::get_logger(const
std::shared_ptr<Logger>& logger, const
std::shared_ptr<internal::LoggerNamespace> &root_namespace, std::string_view
name_view,
- const
std::shared_ptr<spdlog::formatter>& formatter, bool remove_if_present) {
- std::string name{name_view};
- std::shared_ptr<spdlog::logger> spdlogger = spdlog::get(name);
- if (spdlogger) {
- if (remove_if_present) {
- spdlog::drop(name);
- } else {
- return spdlogger;
- }
+std::shared_ptr<spdlog::logger> LoggerConfiguration::get_logger(const
std::shared_ptr<internal::LoggerNamespace> &root_namespace, const
std::string_view name_view,
+ const
std::shared_ptr<spdlog::formatter>& formatter) {
+ const std::string name{name_view};
+ if (auto spdlogger = spdlog::get(name)) {
+ return spdlogger;
}
+ return create_logger(root_namespace, name, formatter);
+}
+
+void LoggerConfiguration::setupSpdLogger(const
std::shared_ptr<spdlog::logger>& spd_logger,
+ const std::shared_ptr<internal::LoggerNamespace>& root_namespace,
+ const std::string& name,
+ const std::shared_ptr<spdlog::formatter>& formatter) {
+ if (!spd_logger)
+ return;
std::shared_ptr<internal::LoggerNamespace> current_namespace =
root_namespace;
std::vector<std::shared_ptr<spdlog::sinks::sink>> sinks =
root_namespace->sinks;
std::vector<std::shared_ptr<spdlog::sinks::sink>> inherited_sinks;
spdlog::level::level_enum level = root_namespace->level;
std::string current_namespace_str;
- std::string sink_namespace_str = "root";
- std::string level_namespace_str = "root";
for (auto const & name_segment : utils::string::split(name, "::")) {
current_namespace_str += name_segment;
auto child_pair = current_namespace->children.find(name_segment);
if (child_pair == current_namespace->children.end()) {
break;
}
- std::copy(current_namespace->exported_sinks.begin(),
current_namespace->exported_sinks.end(), std::back_inserter(inherited_sinks));
+ ranges::copy(current_namespace->exported_sinks,
std::back_inserter(inherited_sinks));
+
current_namespace = child_pair->second;
if (!current_namespace->sinks.empty()) {
sinks = current_namespace->sinks;
- sink_namespace_str = current_namespace_str;
}
if (current_namespace->has_level) {
level = current_namespace->level;
- level_namespace_str = current_namespace_str;
}
current_namespace_str += "::";
}
- if (logger != nullptr) {
- logger->log_debug("{} logger got sinks from namespace {} and level {} from
namespace {}", name, sink_namespace_str, spdlog::level::to_string_view(level),
level_namespace_str);
- }
- std::copy(inherited_sinks.begin(), inherited_sinks.end(),
std::back_inserter(sinks));
- spdlogger = std::make_shared<spdlog::logger>(name, begin(sinks), end(sinks));
- spdlogger->set_level(level);
- spdlogger->set_formatter(formatter->clone());
- spdlogger->flush_on(std::max(spdlog::level::info, current_namespace->level));
+ ranges::copy(inherited_sinks, std::back_inserter(sinks));
+ spd_logger->sinks() = sinks;
+ spd_logger->set_level(level);
+ spd_logger->set_formatter(formatter->clone());
+ spd_logger->flush_on(std::max(spdlog::level::info,
current_namespace->level));
Review Comment:
Good idea, unfortunetly spdlog doesnt offer that functionality...
However we are doing this from a singleton object (LoggerConfiguration is
used as a singleton from prod code), so with this information in mind, I've
expanded the already existing pattern of requiring the lock_guard for this type
of code to run. (of course this doesnt protect us from malicious code that
creates a lock_guard from a seperate mutex, we gotta refactor this whole mess
in a more indepth PR in my opinion)
https://github.com/apache/nifi-minifi-cpp/pull/1718/commits/d1d9008ca3f4edca4cc0cf847978f536581aa3ae
--
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]
