szaszm commented on code in PR #1718:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1718#discussion_r1457526803
##########
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:
I'd change this to take a `const std::string&` instead: now we're calling it
with a `std::string`, creating a temporary `std::string_view`, only to use it
to create a temporary `std::string` that we can pass further.
##########
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:
Synchronization may become an issue here. Can we do these atomically on the
spdlog logger object?
--
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]
