From b02440d1c1b65561bf07bfacc9fdeea6950d7019 Mon Sep 17 00:00:00 2001 From: bergmann Date: Tue, 3 Sep 2019 22:41:25 +0200 Subject: [PATCH] * Removed singleton implementation for manager Users can now create own instances of the manager. To not break the old interface, the single global instance is still created by the logging framework if necessary. If a custom intance is created this one will be used. --- include/cpplogging/manager.h | 3 + .../cpplogging/manager/consumer/consumer.h | 13 ++- .../manager/consumer/consumer_stream.h | 12 +-- include/cpplogging/manager/manager.h | 60 +++++++++---- include/cpplogging/manager/manager.inl | 17 ++++ src/cpplogging/interface/logger.cpp | 2 +- src/cpplogging/manager/consumer/consumer.cpp | 7 +- .../manager/consumer/consumer_stream.cpp | 10 +-- src/cpplogging/manager/manager.cpp | 89 ++++++++++++++----- test/cpplogging/cpplogging_tests.cpp | 39 ++++---- 10 files changed, 179 insertions(+), 73 deletions(-) diff --git a/include/cpplogging/manager.h b/include/cpplogging/manager.h index d750b78..bf53e96 100644 --- a/include/cpplogging/manager.h +++ b/include/cpplogging/manager.h @@ -5,3 +5,6 @@ #include "manager/logger_impl.h" #include "manager/matcher.h" #include "manager/rule.h" + +#include "manager/manager.inl" +#include "manager/rule.inl" diff --git a/include/cpplogging/manager/consumer/consumer.h b/include/cpplogging/manager/consumer/consumer.h index 2a56506..ff35b5d 100644 --- a/include/cpplogging/manager/consumer/consumer.h +++ b/include/cpplogging/manager/consumer/consumer.h @@ -1,7 +1,8 @@ #pragma once -#include +#include #include +#include #include #include @@ -9,6 +10,8 @@ namespace cpplogging { + struct manager; + /** * @brief Class to consume log entries. */ @@ -16,10 +19,14 @@ namespace cpplogging { private: using format_type = std::vector; + using manager_set = std::set; + + friend manager; protected: std::string _name; //!< name of the consumer format_type _format; //!< parsed logging format + manager_set _manager; //!< Set of manager this consumer is assigned to. public: /** @@ -54,8 +61,8 @@ namespace cpplogging * @param[in] format Format to use for logging. */ consumer( - const std::string& name, - const std::string& format = default_format()); + const std::string& name, + const std::string& format = default_format()); /** * @breif Destructor. diff --git a/include/cpplogging/manager/consumer/consumer_stream.h b/include/cpplogging/manager/consumer/consumer_stream.h index 59ece99..d8a79bd 100644 --- a/include/cpplogging/manager/consumer/consumer_stream.h +++ b/include/cpplogging/manager/consumer/consumer_stream.h @@ -30,9 +30,9 @@ namespace cpplogging * @param[in] format Format to use for logging. */ consumer_stream( - const std::string& name, - std::ostream& stream, - const std::string& format = default_format()); + const std::string& name, + std::ostream& stream, + const std::string& format = default_format()); /** * @brief Constructor. @@ -42,9 +42,9 @@ namespace cpplogging * @param[in] format Format to use for logging. */ consumer_stream( - const std::string& name, - stream_ptr_u&& stream, - const std::string& format = default_format()); + const std::string& name, + stream_ptr_u&& stream, + const std::string& format = default_format()); /** * @brief Consume a log entry. diff --git a/include/cpplogging/manager/manager.h b/include/cpplogging/manager/manager.h index fc2b632..c5f04c5 100644 --- a/include/cpplogging/manager/manager.h +++ b/include/cpplogging/manager/manager.h @@ -81,13 +81,23 @@ namespace cpplogging using consumer_set = std::set; private: - static std::mutex _mutex; //!< mutex to protect the engine - static logger_impl _default; //!< default logger - static logger_map _logger; //!< normal logger instances - static rule_list _rules; //!< rules to combine loggers with consumers - static consumer_set _consumer; //!< consumers registered + std::recursive_mutex _mutex; //!< mutex to protect the manager + logger_impl _default; //!< default logger + logger_map _logger; //!< normal logger instances + rule_list _rules; //!< rules to combine loggers with consumers + consumer_set _consumer; //!< consumers registered public: + /** + * @brief Constructor. + */ + inline manager(); + + /** + * @brief Destructor. + */ + inline ~manager(); + /** * @brief Get the instance of the logger with the requested name. * @@ -95,37 +105,37 @@ namespace cpplogging * * @return Instance of the logger with the requested name. */ - static logger& get_logger(const std::string& name = ""); + logger& get_logger(const std::string& name = ""); /** * @brief Register a new consumer in the logging framework. * * @param[in] consumer Reference of the consumer to register. */ - static void register_consumer(consumer& consumer); + void register_consumer(consumer& consumer); /** * @brief Register a new consumer in the logging framework. * * @param[in] consumer Consumer to register (the logging framework will own the consumer). * - * @return Pointer to the consumer. + * @return Reference to the consumer. */ - static const consumer* register_consumer(consumer_ptr_u&& consumer); + const consumer& register_consumer(consumer_ptr_u&& consumer); /** * @brief Unregister a consumer from the logging framework. * * @param[in] consumer Reference of the consumer to unregister */ - static void unregister_consumer(consumer& consumer); + void unregister_consumer(consumer& consumer); /** * @brief Unregister a consumer from the logging framework. * * @param[in] consumer Consumer to unregister from the framework. */ - static void unregister_consumer(consumer* consumer); + void unregister_consumer(consumer* consumer); /** * @brief Define a new rule in the logging framework. @@ -137,7 +147,7 @@ namespace cpplogging * * @return Return the handle to the rule (for removing). */ - static rule_handle define_rule( + rule_handle define_rule( matcher_ptr_u&& logger_matcher, matcher_ptr_u&& consumer_matcher, log_level min_level = log_level::debug, @@ -148,12 +158,12 @@ namespace cpplogging * * @param[in] handle Handle of the rule to remove. */ - static void undefine_rule(rule_handle handle); + void undefine_rule(rule_handle handle); /** * @brief Remove all consumers, rules, matchers and loggers. */ - static void reset(); + void reset(); #ifdef CPPLOGGING_HAS_LOAD_CONFIG /** @@ -161,14 +171,32 @@ namespace cpplogging * * @param[in] filename Filename to load configuration from. */ - static void load(const std::string& filename); + void load(const std::string& filename); #endif private: /** * @brief Initialize the given logger instance. */ - static logger_impl& init_logger(logger_impl& logger); + logger_impl& init_logger(logger_impl& logger); + + private: + static manager* _instance; //!< First created manager instance. + static std::unique_ptr _singleton; //!< Default manager instance. + + public: + /** + * @brief Get the global manager instance + * + * This method will return the first created manager. If no manager is created yet, + * a new global manager is created. + */ + static manager& instance(); + + /** + * @brief Create the default manager writing all output to stdout and stderr. + */ + static std::unique_ptr default_(); }; } diff --git a/include/cpplogging/manager/manager.inl b/include/cpplogging/manager/manager.inl index 40878e4..c160bd5 100644 --- a/include/cpplogging/manager/manager.inl +++ b/include/cpplogging/manager/manager.inl @@ -41,4 +41,21 @@ namespace cpplogging ::operator*() const { return _consumer; } + /* manager */ + + manager::manager() + : _default("") + { + init_logger(_default); + if (_instance == nullptr) + _instance = this; + } + + manager::~manager() + { + reset(); + if (_instance == this) + _instance = nullptr; + } + } diff --git a/src/cpplogging/interface/logger.cpp b/src/cpplogging/interface/logger.cpp index 823d859..bc75ee2 100644 --- a/src/cpplogging/interface/logger.cpp +++ b/src/cpplogging/interface/logger.cpp @@ -4,4 +4,4 @@ using namespace ::cpplogging; logger& logger::get(const std::string& name) - { return manager::get_logger(name); } + { return manager::instance().get_logger(name); } diff --git a/src/cpplogging/manager/consumer/consumer.cpp b/src/cpplogging/manager/consumer/consumer.cpp index f281b61..55cb36a 100644 --- a/src/cpplogging/manager/consumer/consumer.cpp +++ b/src/cpplogging/manager/consumer/consumer.cpp @@ -3,12 +3,15 @@ using namespace ::cpplogging; -consumer::consumer(const std::string& name, const std::string& format) +consumer::consumer( + const std::string& name, + const std::string& format) : _name (name) , _format (parse_format(format)) { } consumer::~consumer() { - manager::unregister_consumer(this); + for (auto& mrg : _manager) + mrg->unregister_consumer(this); } diff --git a/src/cpplogging/manager/consumer/consumer_stream.cpp b/src/cpplogging/manager/consumer/consumer_stream.cpp index 6eced68..387039a 100644 --- a/src/cpplogging/manager/consumer/consumer_stream.cpp +++ b/src/cpplogging/manager/consumer/consumer_stream.cpp @@ -7,8 +7,8 @@ using namespace ::cpplogging; consumer_stream::consumer_stream( - const std::string& name, - std::ostream& stream, + const std::string& name, + std::ostream& stream, const std::string& format) : consumer (name, format) , _stream (stream) @@ -16,9 +16,9 @@ consumer_stream::consumer_stream( { } consumer_stream::consumer_stream( - const std::string& name, - stream_ptr_u&& stream, - const std::string& format) + const std::string& name, + stream_ptr_u&& stream, + const std::string& format) : consumer (name, format) , _stream (*stream) , _storage (std::move(stream)) diff --git a/src/cpplogging/manager/manager.cpp b/src/cpplogging/manager/manager.cpp index dea2d4b..2e213e2 100644 --- a/src/cpplogging/manager/manager.cpp +++ b/src/cpplogging/manager/manager.cpp @@ -13,15 +13,12 @@ using namespace ::cpplogging; -std::mutex manager::_mutex; -logger_impl manager::_default(""); -manager::logger_map manager::_logger; -manager::rule_list manager::_rules; -manager::consumer_set manager::_consumer; +manager* manager::_instance; +std::unique_ptr manager::_singleton; logger& manager::get_logger(const std::string& name) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); if (name.empty()) return _default; @@ -29,7 +26,7 @@ logger& manager::get_logger(const std::string& name) if (it == _logger.end()) { it = _logger.emplace_hint(it, name, std::make_unique(name)); - manager::init_logger(*it->second); + init_logger(*it->second); } return *it->second; @@ -37,9 +34,10 @@ logger& manager::get_logger(const std::string& name) void manager::register_consumer(consumer& consumer) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); auto it = _consumer.emplace(consumer).first; - auto& c = **it; + auto& c = const_cast(**it); + c._manager.emplace(this); for (auto& rule : _rules) { if (rule.consumer_matcher->match(c)) @@ -47,33 +45,36 @@ void manager::register_consumer(consumer& consumer) } } -const consumer* manager::register_consumer(consumer_ptr_u&& consumer) +const consumer& manager::register_consumer(consumer_ptr_u&& consumer) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); auto it = _consumer.emplace(std::move(consumer)).first; - auto& ret = **it; + auto& c = const_cast(**it); + c._manager.emplace(this); for (auto& rule : _rules) { - if (rule.consumer_matcher->match(ret)) - rule.add_consumer(ret); + if (rule.consumer_matcher->match(c)) + rule.add_consumer(c); } - return &ret; + return c; } void manager::unregister_consumer(consumer& consumer) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); + consumer._manager.erase(this); unregister_consumer(&consumer); } void manager::unregister_consumer(consumer* consumer) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); auto it = _consumer.find(consumer_wrapper(*consumer)); if (it == _consumer.end()) return; - auto& c = **it; + auto& c = const_cast(**it); + c._manager.erase(this); for (auto& rule : _rules) rule.remove_consumer(c); } @@ -84,7 +85,7 @@ rule_handle manager::define_rule( log_level min_level, log_level max_level) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); /* create rule */ _rules.emplace_back(std::move(logger_matcher), std::move(consumer_matcher), min_level, max_level); @@ -112,7 +113,7 @@ rule_handle manager::define_rule( void manager::undefine_rule(rule_handle handle) { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); /* find the rule in the list */ auto r = static_cast(handle); @@ -133,7 +134,11 @@ void manager::undefine_rule(rule_handle handle) void manager::reset() { - std::lock_guard lg(_mutex); + std::lock_guard lg(_mutex); + + for (auto it = _consumer.rbegin(); it != _consumer.rend(); ++it) + unregister_consumer(const_cast(**it)); + _logger.clear(); _rules.clear(); _consumer.clear(); @@ -323,3 +328,45 @@ logger_impl& manager::init_logger(logger_impl& logger) } return logger; } + +manager& manager::instance() +{ + if (!_instance) + _singleton.reset(new manager()); + return *_instance; +} + +std::unique_ptr manager::default_() +{ + std::unique_ptr m(new manager()); + + /* consumers */ + m->register_consumer( + std::make_unique("cout", std::cout, + "$n[${runtime:-016.6f}] " + "${level:5S} " + "${name} ${sender_type}(0x${sender:-016x})@0x${thread:-016x} " + "${filename}:${line}$n" + "${message}")); + m->register_consumer( + std::make_unique("cerr", std::cerr, + "$n[${runtime:-016.6f}] " + "${level:5S} " + "${sender_type}(0x${sender:-016x})@0x${thread:-016x} " + "${filename}:${line}$n" + "${message}")); + + /* log rules */ + m->define_rule( + std::make_unique(), + std::make_unique("^cout$"), + log_level::debug, + log_level::warn); + m->define_rule( + std::make_unique(), + std::make_unique("^cerr$"), + log_level::error, + log_level::error); + + return m; +} diff --git a/test/cpplogging/cpplogging_tests.cpp b/test/cpplogging/cpplogging_tests.cpp index b3b26bd..47e35f0 100644 --- a/test/cpplogging/cpplogging_tests.cpp +++ b/test/cpplogging/cpplogging_tests.cpp @@ -6,21 +6,15 @@ using namespace ::testing; using namespace ::cpplogging; -struct LoggingReset -{ - ~LoggingReset() - { manager::reset(); } -}; - struct consumer_mock : public consumer { MOCK_CONST_METHOD1(write_entry, void (const log_entry_ptr_s& entry)); - consumer_mock(const std::string& n) : + consumer_mock(manager& mngr, const std::string& n) : consumer(n) { - manager::register_consumer(*this); + mngr.register_consumer(*this); } }; @@ -39,7 +33,8 @@ MATCHER_P5(MatchLogData, level, sender, thread, name, message, "") TEST(LoggingTests, matcher_all) { - LoggingReset loggingReset; + manager mngr; + consumer_stream c0("TestConsumer1", std::cout); consumer_stream c1("TestConsumer2", std::cout); @@ -56,7 +51,8 @@ TEST(LoggingTests, matcher_all) TEST(LoggingTests, matcher_default) { - LoggingReset loggingReset; + manager mngr; + consumer_stream c0("TestConsumer1", std::cout); consumer_stream c1("TestConsumer2", std::cout); @@ -73,7 +69,8 @@ TEST(LoggingTests, matcher_default) TEST(LoggingTests, matcher_regex) { - LoggingReset loggingReset; + manager mngr; + consumer_stream c0("TestConsumer1", std::cout); consumer_stream c1("TestConsumer2", std::cout); @@ -91,9 +88,10 @@ TEST(LoggingTests, matcher_regex) TEST(LoggingTests, log_base) { - LoggingReset loggingReset; - StrictMock c0("consumer0"); - StrictMock c1("Consumer1"); + manager mngr; + + StrictMock c0(mngr, "consumer0"); + StrictMock c1(mngr, "Consumer1"); EXPECT_CALL(c0, write_entry(MatchLogData( log_level::info, @@ -109,7 +107,7 @@ TEST(LoggingTests, log_base) std::string("logger0"), std::string("test1 warn")))); - manager::define_rule( + mngr.define_rule( std::make_unique("logger0"), std::make_unique("consumer0"), log_level::info, @@ -131,8 +129,9 @@ TEST(LoggingTests, log_base) TEST(LoggingTests, log_inverted_regex) { - LoggingReset loggingReset; - StrictMock c0("consumer0"); + manager mngr; + + StrictMock c0(mngr, "consumer0"); EXPECT_CALL(c0, write_entry(MatchLogData( log_level::debug, @@ -141,7 +140,7 @@ TEST(LoggingTests, log_inverted_regex) std::string("logger2"), std::string("test2")))); - manager::define_rule( + mngr.define_rule( std::make_unique("^logger[0-1]+$", true), std::make_unique("consumer0"), log_level::debug, @@ -159,6 +158,8 @@ TEST(LoggingTests, log_inverted_regex) #ifdef CPPLOGGING_HAS_LOAD_CONFIG TEST(LoggingTests, load) { - manager::load("./cpplogging/test_config.json"); + manager mngr; + + mngr.load("./cpplogging/test_config.json"); } #endif