You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by ph...@apache.org on 2019/09/09 14:28:29 UTC
[nifi-minifi-cpp] branch master updated: MINIFICPP-14: Prune class
names. Add option to config (#643)
This is an automated email from the ASF dual-hosted git repository.
phrocker pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
The following commit(s) were added to refs/heads/master by this push:
new ad917bb MINIFICPP-14: Prune class names. Add option to config (#643)
ad917bb is described below
commit ad917bb79660e6e029fe52581180a2ad1b93b5de
Author: Marc <ph...@apache.org>
AuthorDate: Mon Sep 9 10:28:25 2019 -0400
MINIFICPP-14: Prune class names. Add option to config (#643)
* MINIFICPP-14: Prune class names. Add option to config
* MINIFICPP-14: update API
Signed-off-by: Marc Parisi <ph...@apache.org>
---
conf/minifi-log.properties | 2 +
.../include/core/logging/LoggerConfiguration.h | 33 +++++---
libminifi/include/utils/ClassUtils.h | 44 +++++++++++
libminifi/include/utils/StringUtils.h | 3 +-
libminifi/src/core/logging/LoggerConfiguration.cpp | 15 ++++
libminifi/src/utils/ClassUtils.cpp | 58 ++++++++++++++
libminifi/test/TestBase.h | 92 ++++++++++++++++++----
libminifi/test/unit/ClassUtilsTests.cpp | 61 ++++++++++++++
libminifi/test/unit/LoggerTests.cpp | 36 ++++++++-
9 files changed, 316 insertions(+), 28 deletions(-)
diff --git a/conf/minifi-log.properties b/conf/minifi-log.properties
index dc25406..55d07c3 100644
--- a/conf/minifi-log.properties
+++ b/conf/minifi-log.properties
@@ -16,6 +16,8 @@
#More verbose pattern by default
#Format details at https://github.com/gabime/spdlog/wiki/3.-Custom-formatting
spdlog.pattern=[%Y-%m-%d %H:%M:%S.%e] [%n] [%l] %v
+# uncomment to prune package names
+#spdlog.shorten_names=true
#Old format
#spdlog.pattern=[%Y-%m-%d %H:%M:%S.%e] [minifi log] [%l] %v
diff --git a/libminifi/include/core/logging/LoggerConfiguration.h b/libminifi/include/core/logging/LoggerConfiguration.h
index 7f9fdd2..ea714e9 100644
--- a/libminifi/include/core/logging/LoggerConfiguration.h
+++ b/libminifi/include/core/logging/LoggerConfiguration.h
@@ -57,7 +57,9 @@ struct LoggerNamespace {
class LoggerProperties : public Properties {
public:
- LoggerProperties() : Properties("Logger properties") {}
+ LoggerProperties()
+ : Properties("Logger properties") {
+ }
/**
* Gets all keys that start with the given prefix and do not have a "." after the prefix and "." separator.
*
@@ -92,13 +94,21 @@ class LoggerConfiguration {
return logger_configuration;
}
- void disableLogging(){
+ static std::unique_ptr<LoggerConfiguration> newInstance() {
+ return std::unique_ptr<LoggerConfiguration>(new LoggerConfiguration());
+ }
+
+ void disableLogging() {
controller_->setEnabled(false);
}
- void enableLogging(){
- controller_->setEnabled(true);
- }
+ void enableLogging() {
+ controller_->setEnabled(true);
+ }
+
+ bool shortenClassNames() const {
+ return shorten_names_;
+ }
/**
* (Re)initializes the logging configuation with the given logger properties.
*/
@@ -118,10 +128,11 @@ class LoggerConfiguration {
class LoggerImpl : public Logger {
public:
- LoggerImpl(std::string name, std::shared_ptr<LoggerControl> controller, std::shared_ptr<spdlog::logger> delegate)
- : Logger(delegate,controller),
+ explicit LoggerImpl(const std::string &name, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
+ : Logger(delegate, controller),
name(name) {
}
+
void set_delegate(std::shared_ptr<spdlog::logger> delegate) {
std::lock_guard<std::mutex> lock(mutex_);
delegate_ = delegate;
@@ -137,6 +148,8 @@ class LoggerConfiguration {
std::mutex mutex;
std::shared_ptr<LoggerImpl> logger_ = nullptr;
std::shared_ptr<LoggerControl> controller_;
+ bool shorten_names_;
+
};
template<typename T>
@@ -151,9 +164,9 @@ class LoggerFactory {
}
static std::shared_ptr<Logger> getAliasedLogger(const std::string &alias) {
- std::shared_ptr<Logger> logger = LoggerConfiguration::getConfiguration().getLogger(alias);
- return logger;
- }
+ std::shared_ptr<Logger> logger = LoggerConfiguration::getConfiguration().getLogger(alias);
+ return logger;
+ }
};
} /* namespace logging */
diff --git a/libminifi/include/utils/ClassUtils.h b/libminifi/include/utils/ClassUtils.h
new file mode 100644
index 0000000..3d82c43
--- /dev/null
+++ b/libminifi/include/utils/ClassUtils.h
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_UTILS_CLASSUTILS_H_
+#define LIBMINIFI_INCLUDE_UTILS_CLASSUTILS_H_
+
+#include <string>
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace ClassUtils {
+
+/**
+ * Shortens class names via the canonical representation ( package with name )
+ * @param class_name input class name
+ * @param out output class name that is shortened.
+ * @return true if out has been updated, false otherwise
+ */
+bool shortenClassName(const std::string &class_name, std::string &out);
+
+} /* namespace ClassUtils */
+} /* namespace utils */
+} /* namespace minifi */
+} /* namespace nifi */
+} /* namespace apache */
+} /* namespace org */
+
+#endif /* LIBMINIFI_INCLUDE_UTILS_CLASSUTILS_H_ */
diff --git a/libminifi/include/utils/StringUtils.h b/libminifi/include/utils/StringUtils.h
index 6ecbebd..b45f73d 100644
--- a/libminifi/include/utils/StringUtils.h
+++ b/libminifi/include/utils/StringUtils.h
@@ -374,8 +374,7 @@ enum TimeUnit {
NANOSECOND
};
-} /* namespace core */
-
+} /* namespace utils */
} /* namespace minifi */
} /* namespace nifi */
} /* namespace apache */
diff --git a/libminifi/src/core/logging/LoggerConfiguration.cpp b/libminifi/src/core/logging/LoggerConfiguration.cpp
index 862baaf..0997f09 100644
--- a/libminifi/src/core/logging/LoggerConfiguration.cpp
+++ b/libminifi/src/core/logging/LoggerConfiguration.cpp
@@ -29,6 +29,7 @@
#include "core/Core.h"
#include "utils/StringUtils.h"
+#include "utils/ClassUtils.h"
#include "spdlog/spdlog.h"
#include "spdlog/sinks/stdout_sinks.h"
@@ -63,6 +64,7 @@ std::vector<std::string> LoggerProperties::get_keys_of_type(const std::string &t
LoggerConfiguration::LoggerConfiguration()
: root_namespace_(create_default_root()),
loggers(std::vector<std::shared_ptr<LoggerImpl>>()),
+ shorten_names_(false),
formatter_(std::make_shared<spdlog::pattern_formatter>(spdlog_default_pattern)) {
controller_ = std::make_shared<LoggerControl>();
logger_ = std::shared_ptr<LoggerImpl>(
@@ -77,6 +79,15 @@ void LoggerConfiguration::initialize(const std::shared_ptr<LoggerProperties> &lo
if (!logger_properties->get("spdlog.pattern", spdlog_pattern)) {
spdlog_pattern = spdlog_default_pattern;
}
+
+ /**
+ * There is no need to shorten names per spdlog sink as this is a per log instance.
+ */
+ std::string shorten_names_str;
+ if (logger_properties->get("spdlog.shorten_names", shorten_names_str)) {
+ utils::StringUtils::StringToBool(shorten_names_str, shorten_names_);
+ }
+
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) {
@@ -100,6 +111,10 @@ std::shared_ptr<Logger> LoggerConfiguration::getLogger(const std::string &name)
auto haz_clazz = name.find(clazz);
if (haz_clazz == 0)
adjusted_name = name.substr(clazz.length(), name.length() - clazz.length());
+ if (shorten_names_) {
+ utils::ClassUtils::shortenClassName(adjusted_name, adjusted_name);
+ }
+
std::shared_ptr<LoggerImpl> result = std::make_shared<LoggerImpl>(adjusted_name, controller_, get_logger(logger_, root_namespace_, adjusted_name, formatter_));
loggers.push_back(result);
return result;
diff --git a/libminifi/src/utils/ClassUtils.cpp b/libminifi/src/utils/ClassUtils.cpp
new file mode 100644
index 0000000..9ef2c0d
--- /dev/null
+++ b/libminifi/src/utils/ClassUtils.cpp
@@ -0,0 +1,58 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "utils/ClassUtils.h"
+#include "utils/StringUtils.h"
+#include <iostream>
+#include <string>
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool ClassUtils::shortenClassName(const std::string &class_name, std::string &out) {
+ std::string class_delim = "::";
+ auto class_split = utils::StringUtils::split(class_name, class_delim);
+ // support . and ::
+ if (class_split.size() <= 1) {
+ if (class_name.find(".") != std::string::npos) {
+ class_delim = ".";
+ class_split = utils::StringUtils::split(class_name, class_delim);
+ } else {
+ // if no update can be performed, return false to let the developer know
+ // this. Out will have no updates
+ return false;
+ }
+ }
+ for (auto &elem : class_split) {
+ if (&elem != &class_split.back() && elem.size() > 1) {
+ elem = elem.substr(0, 1);
+ }
+ }
+
+ out = utils::StringUtils::join(class_delim, class_split);
+ return true;
+}
+
+} /* namespace utils */
+} /* namespace minifi */
+} /* namespace nifi */
+} /* namespace apache */
+} /* namespace org */
+
diff --git a/libminifi/test/TestBase.h b/libminifi/test/TestBase.h
index f7af0c1..4d7d363 100644
--- a/libminifi/test/TestBase.h
+++ b/libminifi/test/TestBase.h
@@ -46,14 +46,31 @@
#include "core/reporting/SiteToSiteProvenanceReportingTask.h"
#include "core/state/nodes/FlowInformation.h"
#include "properties/Configure.h"
+#include "utils/ClassUtils.h"
class LogTestController {
public:
+ ~LogTestController() {
+ }
static LogTestController& getInstance() {
static LogTestController instance;
return instance;
}
+ static std::shared_ptr<LogTestController> getInstance(const std::shared_ptr<logging::LoggerProperties> &logger_properties) {
+ static std::map<std::shared_ptr<logging::LoggerProperties>, std::shared_ptr<LogTestController>> map;
+ auto fnd = map.find(logger_properties);
+ if (fnd != std::end(map)) {
+ return fnd->second;
+ } else {
+ // in practice I'd use a derivation here or another paradigm entirely but for the purposes of this test code
+ // having extra overhead is negligible. this is the most readable and least impactful way
+ auto instance = std::shared_ptr<LogTestController>(new LogTestController(logger_properties));
+ map.insert(std::make_pair(logger_properties, instance));
+ return map.find(logger_properties)->second;
+ }
+ }
+
template<typename T>
void setTrace() {
setLevel<T>(spdlog::level::trace);
@@ -84,12 +101,35 @@ class LogTestController {
setLevel<T>(spdlog::level::off);
}
+ /**
+ * Most tests use the main logging framework. this addition allows us to have and control variants for the purposes
+ * of changeable test formats
+ */
+ template<typename T>
+ std::shared_ptr<logging::Logger> getLogger() {
+ std::string name = core::getClassName<T>();
+ return config ? config->getLogger(name) : logging::LoggerConfiguration::getConfiguration().getLogger(name);
+ }
+
template<typename T>
void setLevel(spdlog::level::level_enum level) {
logging::LoggerFactory<T>::getLogger();
std::string name = core::getClassName<T>();
- modified_loggers.push_back(name);
+ if (config)
+ config->getLogger(name);
+ else
+ logging::LoggerConfiguration::getConfiguration().getLogger(name);
+ modified_loggers.insert(name);
setLevel(name, level);
+ // also support shortened classnames
+ if (config && config->shortenClassNames()) {
+ std::string adjusted = name;
+ if (utils::ClassUtils::shortenClassName(name, adjusted)) {
+ modified_loggers.insert(name);
+ setLevel(name, level);
+ }
+ }
+
}
bool contains(const std::string &ending, std::chrono::seconds timeout = std::chrono::seconds(3)) {
@@ -121,7 +161,9 @@ class LogTestController {
for (auto const & name : modified_loggers) {
setLevel(name, spdlog::level::err);
}
- modified_loggers = std::vector<std::string>();
+ modified_loggers.clear();
+ if (config)
+ config = std::move(logging::LoggerConfiguration::newInstance());
resetStream(log_output);
}
@@ -133,7 +175,7 @@ class LogTestController {
std::ostringstream log_output;
std::shared_ptr<logging::Logger> logger_;
- private:
+ protected:
class TestBootstrapLogger : public logging::Logger {
public:
TestBootstrapLogger(std::shared_ptr<spdlog::logger> logger)
@@ -141,22 +183,39 @@ class LogTestController {
}
;
};
- LogTestController() {
- std::shared_ptr<logging::LoggerProperties> logger_properties = std::make_shared<logging::LoggerProperties>();
- logger_properties->set("logger.root", "ERROR,ostream");
- logger_properties->set("logger." + core::getClassName<LogTestController>(), "INFO");
- logger_properties->set("logger." + core::getClassName<logging::LoggerConfiguration>(), "DEBUG");
+ LogTestController()
+ : LogTestController(nullptr) {
+ }
+
+ explicit LogTestController(const std::shared_ptr<logging::LoggerProperties> &loggerProps) {
+ my_properties_ = loggerProps;
+ bool initMain = false;
+ if (nullptr == my_properties_) {
+ my_properties_ = std::make_shared<logging::LoggerProperties>();
+ initMain = true;
+ }
+ my_properties_->set("logger.root", "ERROR,ostream");
+ my_properties_->set("logger." + core::getClassName<LogTestController>(), "INFO");
+ my_properties_->set("logger." + core::getClassName<logging::LoggerConfiguration>(), "DEBUG");
std::shared_ptr<spdlog::sinks::dist_sink_mt> dist_sink = std::make_shared<spdlog::sinks::dist_sink_mt>();
dist_sink->add_sink(std::make_shared<spdlog::sinks::ostream_sink_mt>(log_output, true));
dist_sink->add_sink(spdlog::sinks::stderr_sink_mt::instance());
- logger_properties->add_sink("ostream", dist_sink);
- logging::LoggerConfiguration::getConfiguration().initialize(logger_properties);
- logger_ = logging::LoggerFactory<LogTestController>::getLogger();
+ my_properties_->add_sink("ostream", dist_sink);
+ if (initMain) {
+ logging::LoggerConfiguration::getConfiguration().initialize(my_properties_);
+ logger_ = logging::LoggerConfiguration::getConfiguration().getLogger(core::getClassName<LogTestController>());
+ } else {
+ config = std::move(logging::LoggerConfiguration::newInstance());
+ // create for test purposes. most tests use the main logging factory, but this exists to test the logging
+ // framework itself.
+ config->initialize(my_properties_);
+ logger_ = config->getLogger(core::getClassName<LogTestController>());
+ }
+
}
LogTestController(LogTestController const&);
LogTestController& operator=(LogTestController const&);
- ~LogTestController() {
- }
+
;
void setLevel(const std::string name, spdlog::level::level_enum level) {
@@ -166,9 +225,14 @@ class LogTestController {
auto haz_clazz = name.find(clazz);
if (haz_clazz == 0)
adjusted_name = name.substr(clazz.length(), name.length() - clazz.length());
+ if (config && config->shortenClassNames()) {
+ utils::ClassUtils::shortenClassName(adjusted_name, adjusted_name);
+ }
spdlog::get(adjusted_name)->set_level(level);
}
- std::vector<std::string> modified_loggers;
+ std::shared_ptr<logging::LoggerProperties> my_properties_;
+ std::unique_ptr<logging::LoggerConfiguration> config;
+ std::set<std::string> modified_loggers;
};
class TestPlan {
diff --git a/libminifi/test/unit/ClassUtilsTests.cpp b/libminifi/test/unit/ClassUtilsTests.cpp
new file mode 100644
index 0000000..61d888d
--- /dev/null
+++ b/libminifi/test/unit/ClassUtilsTests.cpp
@@ -0,0 +1,61 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <string>
+#include <vector>
+#include "utils/ClassUtils.h"
+#include "../TestBase.h"
+
+TEST_CASE("Test ShortNames", "[testcrc1]") {
+ std::string className, adjusted;
+ SECTION("EMPTY") {
+ className = "";
+ adjusted = "";
+ REQUIRE(!utils::ClassUtils::shortenClassName(className, adjusted));
+ REQUIRE(adjusted.empty());
+ }
+
+ SECTION("SINGLE") {
+ className = "Class";
+ adjusted = "";
+ // class name not shortened
+ REQUIRE(!utils::ClassUtils::shortenClassName(className, adjusted));
+ REQUIRE(adjusted.empty());
+ className = "org::Test";
+ adjusted = "";
+ REQUIRE(utils::ClassUtils::shortenClassName(className, adjusted));
+ REQUIRE("o::Test" == adjusted);
+ }
+
+
+
+ SECTION("MULTIPLE") {
+ className = "org::apache::Test";
+ adjusted = "";
+ REQUIRE(utils::ClassUtils::shortenClassName(className, adjusted));
+ REQUIRE("o::a::Test" == adjusted);
+ className = "org.apache.Test";
+ adjusted = "";
+ REQUIRE(utils::ClassUtils::shortenClassName(className, adjusted));
+ REQUIRE("o.a.Test" == adjusted);
+ className = adjusted;
+ adjusted = "";
+ REQUIRE(utils::ClassUtils::shortenClassName(className, adjusted));
+ REQUIRE("o.a.Test" == adjusted);
+ }
+}
diff --git a/libminifi/test/unit/LoggerTests.cpp b/libminifi/test/unit/LoggerTests.cpp
index 60a57c3..e9db9a7 100644
--- a/libminifi/test/unit/LoggerTests.cpp
+++ b/libminifi/test/unit/LoggerTests.cpp
@@ -21,7 +21,7 @@
#include <vector>
#include <ctime>
#include "../TestBase.h"
-
+#include "core/logging/LoggerConfiguration.h"
TEST_CASE("Test log Levels", "[ttl1]") {
LogTestController::getInstance().setTrace<logging::Logger>();
@@ -73,5 +73,37 @@ TEST_CASE("Test log Levels change", "[ttl5]") {
LogTestController::getInstance().reset();
}
-TEST_CASE("Test Demangle template", "[ttl6]") {
+namespace single {
+class TestClass {
+};
+}
+
+class TestClass2 {
+};
+
+TEST_CASE("Test ShortenNames", "[ttl6]") {
+ std::shared_ptr<logging::LoggerProperties> props = std::make_shared<logging::LoggerProperties>();
+
+ props->set("spdlog.shorten_names", "true");
+
+ std::shared_ptr<logging::Logger> logger = LogTestController::getInstance(props)->getLogger<logging::Logger>();
+ logger->log_error("hello %s", "world");
+
+ REQUIRE(true == LogTestController::getInstance(props)->contains("[o::a::n::m::c::l::Logger] [error] hello world"));
+
+ logger = LogTestController::getInstance(props)->getLogger<single::TestClass>();
+ logger->log_error("hello %s", "world");
+
+ REQUIRE(true == LogTestController::getInstance(props)->contains("[s::TestClass] [error] hello world"));
+
+ logger = LogTestController::getInstance(props)->getLogger<TestClass2>();
+ logger->log_error("hello %s", "world");
+
+ REQUIRE(true == LogTestController::getInstance(props)->contains("[TestClass2] [error] hello world"));
+
+ LogTestController::getInstance(props)->reset();
+ LogTestController::getInstance().reset();
+
+ LogTestController::getInstance(props)->reset();
+ LogTestController::getInstance().reset();
}