From f3b61c70ba4a00712dcec18e9ceef6aea4c79674 Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Mon, 17 Oct 2022 16:04:49 -0400 Subject: [PATCH 1/8] Catch exception by reference to fix -Wcatch-value warning --- include/spdlog/sinks/mongo_sink.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index 34f4ee337..d239e9284 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -37,7 +37,7 @@ class mongo_sink : public base_sink db_name_ = db_name; coll_name_ = collection_name; } - catch (const std::exception) + catch (const std::exception&) { throw spdlog_ex("Error opening database"); } From 0145223be1c3b115e562603881af2832505fe496 Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Mon, 17 Oct 2022 16:15:23 -0400 Subject: [PATCH 2/8] Add numerical level to Mongo sink for easier queries Filtering to a certain log level or above, a useful operation, can now be done with an integer comparison as opposed to comparing to a list of strings in the database query. --- include/spdlog/sinks/mongo_sink.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index d239e9284..1338983c5 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -56,7 +56,8 @@ class mongo_sink : public base_sink if (client_ != nullptr) { - auto doc = document{} << "timestamp" << bsoncxx::types::b_date(msg.time) << "level" << level::to_string_view(msg.level).data() + auto doc = document{} << "timestamp" << bsoncxx::types::b_date(msg.time) + << "level" << level::to_string_view(msg.level).data() << "level_num" << msg.level << "message" << std::string(msg.payload.begin(), msg.payload.end()) << "logger_name" << std::string(msg.logger_name.begin(), msg.logger_name.end()) << "thread_id" << static_cast(msg.thread_id) << finalize; From a3c47cc6823d017056b4790db88769fc6f6f40de Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Mon, 17 Oct 2022 17:32:08 -0400 Subject: [PATCH 3/8] Don't force Mongo sink to own MongoCXX instance There can only be one instance in the whole program, so programs that use the Mongo sink and also separately use MongoCXX may have problems if the Mongo sink owns the instance. MongoCXX recommends that the main application manage its own instance so configuration parameters can be passed to the constructor: http://mongocxx.org/api/current/classmongocxx_1_1instance.html However, this commit is not a breaking change. If no instance has been created at construction time, the Mongo sink will still create and own the instance. --- include/spdlog/sinks/mongo_sink.h | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index 1338983c5..ab8a5b4db 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -29,10 +30,23 @@ template class mongo_sink : public base_sink { public: - mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") + mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017", + bool create_instance = true) { try { + if (create_instance && !instance_) + { + try + { + instance_ = std::make_shared(); + } + catch (const mongocxx::logic_error&) + { + // A MongoCXX instance already exists, so this object doesn't need to own it + instance_ = nullptr; + } + } client_ = spdlog::details::make_unique(mongocxx::uri{uri}); db_name_ = db_name; coll_name_ = collection_name; @@ -68,13 +82,13 @@ class mongo_sink : public base_sink void flush_() override {} private: - static mongocxx::instance instance_; + static std::shared_ptr instance_; std::string db_name_; std::string coll_name_; std::unique_ptr client_ = nullptr; }; template<> -mongocxx::instance mongo_sink::instance_{}; +std::shared_ptr mongo_sink::instance_{}; #include "spdlog/details/null_mutex.h" #include From 1bb1f05d73abc99f5c245052c997cd0bda2a31d6 Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Tue, 18 Oct 2022 20:13:17 -0400 Subject: [PATCH 4/8] Adjust MongoCXX instance handling in mongo_sink Changes suggested by @gabime on #2519 --- include/spdlog/sinks/mongo_sink.h | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index ab8a5b4db..b8928d677 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -30,26 +30,18 @@ template class mongo_sink : public base_sink { public: - mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017", - bool create_instance = true) + mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") + try : mongo_sink(std::make_shared(), db_name, collection_name, uri) {} + catch (...) {} // Re-throws exception + + mongo_sink(const std::shared_ptr &instance, const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") + : instance_(instance) + , db_name_(db_name) + , coll_name_(collection_name) { try { - if (create_instance && !instance_) - { - try - { - instance_ = std::make_shared(); - } - catch (const mongocxx::logic_error&) - { - // A MongoCXX instance already exists, so this object doesn't need to own it - instance_ = nullptr; - } - } client_ = spdlog::details::make_unique(mongocxx::uri{uri}); - db_name_ = db_name; - coll_name_ = collection_name; } catch (const std::exception&) { @@ -82,13 +74,11 @@ class mongo_sink : public base_sink void flush_() override {} private: - static std::shared_ptr instance_; + std::shared_ptr instance_; std::string db_name_; std::string coll_name_; std::unique_ptr client_ = nullptr; }; -template<> -std::shared_ptr mongo_sink::instance_{}; #include "spdlog/details/null_mutex.h" #include From 5f67ef4d6f2c1760562f2931bee13990da11d43e Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Tue, 18 Oct 2022 20:25:32 -0400 Subject: [PATCH 5/8] Remove pointless try block in mongo_sink --- include/spdlog/sinks/mongo_sink.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index b8928d677..36201f5c7 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -31,8 +31,7 @@ class mongo_sink : public base_sink { public: mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") - try : mongo_sink(std::make_shared(), db_name, collection_name, uri) {} - catch (...) {} // Re-throws exception + : mongo_sink(std::make_shared(), db_name, collection_name, uri) {} mongo_sink(const std::shared_ptr &instance, const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") : instance_(instance) From 0674e79066fbf9eaa0b9f65a3baa7a1e59b5e78d Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Wed, 19 Oct 2022 09:53:33 -0400 Subject: [PATCH 6/8] Improve arg passing and exceptions in mongo_sink --- include/spdlog/sinks/mongo_sink.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index 36201f5c7..ca16d665c 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -20,7 +20,7 @@ #include #include -#include +#include #include #include @@ -31,10 +31,16 @@ class mongo_sink : public base_sink { public: mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") - : mongo_sink(std::make_shared(), db_name, collection_name, uri) {} + try + : mongo_sink(std::make_shared(), db_name, collection_name, uri) + {} + catch (const mongocxx::exception &e) + { + throw_spdlog_ex(fmt_lib::format("Error opening database: {}", e.what())); + } - mongo_sink(const std::shared_ptr &instance, const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") - : instance_(instance) + mongo_sink(std::shared_ptr instance, const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") + : instance_(std::move(instance)) , db_name_(db_name) , coll_name_(collection_name) { @@ -42,9 +48,9 @@ class mongo_sink : public base_sink { client_ = spdlog::details::make_unique(mongocxx::uri{uri}); } - catch (const std::exception&) + catch (const std::exception &e) { - throw spdlog_ex("Error opening database"); + throw_spdlog_ex(fmt_lib::format("Error opening database: {}", e.what())); } } From b5d361fc218e37028692bd30163766f091fdbbcc Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Wed, 19 Oct 2022 10:08:54 -0400 Subject: [PATCH 7/8] clang-format mongo_sink.h --- include/spdlog/sinks/mongo_sink.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index ca16d665c..73184a9d2 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -31,15 +31,15 @@ class mongo_sink : public base_sink { public: mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") - try - : mongo_sink(std::make_shared(), db_name, collection_name, uri) + try : mongo_sink(std::make_shared(), db_name, collection_name, uri) {} catch (const mongocxx::exception &e) { throw_spdlog_ex(fmt_lib::format("Error opening database: {}", e.what())); } - mongo_sink(std::shared_ptr instance, const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") + mongo_sink(std::shared_ptr instance, const std::string &db_name, const std::string &collection_name, + const std::string &uri = "mongodb://localhost:27017") : instance_(std::move(instance)) , db_name_(db_name) , coll_name_(collection_name) @@ -67,10 +67,9 @@ class mongo_sink : public base_sink if (client_ != nullptr) { - auto doc = document{} << "timestamp" << bsoncxx::types::b_date(msg.time) - << "level" << level::to_string_view(msg.level).data() << "level_num" << msg.level - << "message" << std::string(msg.payload.begin(), msg.payload.end()) << "logger_name" - << std::string(msg.logger_name.begin(), msg.logger_name.end()) << "thread_id" + auto doc = document{} << "timestamp" << bsoncxx::types::b_date(msg.time) << "level" << level::to_string_view(msg.level).data() + << "level_num" << msg.level << "message" << std::string(msg.payload.begin(), msg.payload.end()) + << "logger_name" << std::string(msg.logger_name.begin(), msg.logger_name.end()) << "thread_id" << static_cast(msg.thread_id) << finalize; client_->database(db_name_).collection(coll_name_).insert_one(doc.view()); } From 5fba2867f5180b3e6b582bb92cbab59ee64fc620 Mon Sep 17 00:00:00 2001 From: Sandor Magyar Date: Wed, 19 Oct 2022 14:02:21 -0400 Subject: [PATCH 8/8] Change mongocxx::exception handler to std::exception --- include/spdlog/sinks/mongo_sink.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/spdlog/sinks/mongo_sink.h b/include/spdlog/sinks/mongo_sink.h index 73184a9d2..6a8927f54 100644 --- a/include/spdlog/sinks/mongo_sink.h +++ b/include/spdlog/sinks/mongo_sink.h @@ -20,7 +20,6 @@ #include #include -#include #include #include @@ -33,7 +32,7 @@ class mongo_sink : public base_sink mongo_sink(const std::string &db_name, const std::string &collection_name, const std::string &uri = "mongodb://localhost:27017") try : mongo_sink(std::make_shared(), db_name, collection_name, uri) {} - catch (const mongocxx::exception &e) + catch (const std::exception &e) { throw_spdlog_ex(fmt_lib::format("Error opening database: {}", e.what())); }