Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINIFICPP-14: Prune class names. Add option to config #643

Merged
merged 2 commits into from
Sep 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions conf/minifi-log.properties
Expand Up @@ -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
Expand Down
33 changes: 23 additions & 10 deletions libminifi/include/core/logging/LoggerConfiguration.h
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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;
Expand All @@ -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>
Expand All @@ -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 */
Expand Down
44 changes: 44 additions & 0 deletions 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool to have, I like it!
I also think some of the tempalte functions we already have (getClassName<T> and getClassNames<T>) could also be moved to here from classloader.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. made a follow on.


/**
* 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_ */
3 changes: 1 addition & 2 deletions libminifi/include/utils/StringUtils.h
Expand Up @@ -374,8 +374,7 @@ enum TimeUnit {
NANOSECOND
};

} /* namespace core */

} /* namespace utils */
} /* namespace minifi */
} /* namespace nifi */
} /* namespace apache */
Expand Down
15 changes: 15 additions & 0 deletions libminifi/src/core/logging/LoggerConfiguration.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -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>(
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
58 changes: 58 additions & 0 deletions 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tricky :)

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 */

92 changes: 78 additions & 14 deletions libminifi/test/TestBase.h
Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}

Expand All @@ -133,30 +175,47 @@ 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)
: Logger(logger) {
}
;
};
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) {
Expand All @@ -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 {
Expand Down