From e187c6ba1cfb9d449594547056bcc8ca7da2aa73 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Tue, 7 Jan 2020 11:40:26 +0100 Subject: [PATCH 01/43] Removed typedef. --- include/graylog_logger/LogUtil.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/graylog_logger/LogUtil.hpp b/include/graylog_logger/LogUtil.hpp index 4fec4b7..41af262 100644 --- a/include/graylog_logger/LogUtil.hpp +++ b/include/graylog_logger/LogUtil.hpp @@ -18,7 +18,7 @@ namespace Log { -typedef std::chrono::time_point system_time; +using system_time = std::chrono::time_point; /// \brief Severity levels known by the system. /// @@ -147,6 +147,6 @@ class BaseLogHandler { std::string messageToString(const LogMessage &Message); }; -typedef std::shared_ptr LogHandler_P; +using LogHandler_P = std::shared_ptr; } // namespace Log From 6d5742c73a7d9ec0b291f9d1473b05a179ab0893 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Tue, 7 Jan 2020 13:49:54 +0100 Subject: [PATCH 02/43] =?UTF-8?q?[WIP]=C2=A0Added=20benchmarking.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CMakeLists.txt | 1 + cmake/FindGoogleBenchmark.cmake | 24 ++++++++++++++++++++++++ conanfile.txt | 1 + include/graylog_logger/Log.hpp | 9 +++++++++ performance_test/CMakeLists.txt | 7 +++++++ performance_test/DummyLogHandler.cpp | 6 ++++++ performance_test/DummyLogHandler.h | 12 ++++++++++++ performance_test/PerformanceTest.cpp | 25 +++++++++++++++++++++++++ src/Log.cpp | 4 ++++ 9 files changed, 89 insertions(+) create mode 100644 cmake/FindGoogleBenchmark.cmake create mode 100644 performance_test/CMakeLists.txt create mode 100644 performance_test/DummyLogHandler.cpp create mode 100644 performance_test/DummyLogHandler.h create mode 100644 performance_test/PerformanceTest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 000625a..3f617ba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,4 +47,5 @@ option(BUILD_EVERYTHING "Build UnitTests & Console Logger" ON) IF(BUILD_EVERYTHING) add_subdirectory(unit_tests) add_subdirectory(console_logger) + add_subdirectory(performance_test) ENDIF() diff --git a/cmake/FindGoogleBenchmark.cmake b/cmake/FindGoogleBenchmark.cmake new file mode 100644 index 0000000..ff97d62 --- /dev/null +++ b/cmake/FindGoogleBenchmark.cmake @@ -0,0 +1,24 @@ +find_path(GoogleBenchmark_ROOT_DIR + NAMES include/benchmark/benchmark.h +) + +find_path(GoogleBenchmark_INCLUDE_DIR + NAMES benchmark/benchmark.h + HINTS ${GoogleBenchmark_ROOT_DIR}/include +) + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(GoogleBenchmark FOUND_VAR GoogleBenchmark_FOUND REQUIRED_VARS + GoogleBenchmark_ROOT_DIR + GoogleBenchmark_INCLUDE_DIR +) + +mark_as_advanced( + GoogleBenchmark_ROOT_DIR + GoogleBenchmark_INCLUDE_DIR +) + +find_library(GoogleBenchmark_LIB + NAMES benchmark + HINTS ${GoogleBenchmark_ROOT_DIR}/lib + ) \ No newline at end of file diff --git a/conanfile.txt b/conanfile.txt index 08de515..fe231ee 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -2,6 +2,7 @@ gtest/1.8.1@bincrafters/stable asio/1.13.0@bincrafters/stable jsonformoderncpp/3.6.1@vthiery/stable +google-benchmark/1.4.1-dm2@ess-dmsc/testing [options] gtest:shared=False diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 1391295..8cf57e1 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -105,6 +105,15 @@ void Msg( const int Level, const std::string &Message, const std::vector> &ExtraFields); +/// \brief Flush log messages in the queues of the log handlers. +/// +/// \note Requires that the relevant log handlers implements flushing +/// functionality. +/// \param[in] Timeout The amount of time to wait for the queued messages to +/// be flushed. +/// \return Returns true if messages were flushed, false otherwise. +bool Flush(std::chrono::system_clock::duration Timeout); + /// \brief Add a default field of meta-data to every message. /// /// It is possible to override the value of the default message by passing diff --git a/performance_test/CMakeLists.txt b/performance_test/CMakeLists.txt new file mode 100644 index 0000000..be6b154 --- /dev/null +++ b/performance_test/CMakeLists.txt @@ -0,0 +1,7 @@ +find_package(GoogleBenchmark) + +add_executable(performance_test PerformanceTest.cpp DummyLogHandler.cpp DummyLogHandler.h) + +target_link_libraries(performance_test GraylogLogger::graylog_logger_static ${GoogleBenchmark_LIB}) + +target_include_directories(performance_test PRIVATE ${GoogleBenchmark_INCLUDE_DIR}) \ No newline at end of file diff --git a/performance_test/DummyLogHandler.cpp b/performance_test/DummyLogHandler.cpp new file mode 100644 index 0000000..e4d4dde --- /dev/null +++ b/performance_test/DummyLogHandler.cpp @@ -0,0 +1,6 @@ +#include "DummyLogHandler.h" + +void DummyLogHandler::addMessage(const Log::LogMessage &Message) { + Executor.SendWork([=]() { + }); +} \ No newline at end of file diff --git a/performance_test/DummyLogHandler.h b/performance_test/DummyLogHandler.h new file mode 100644 index 0000000..10fd524 --- /dev/null +++ b/performance_test/DummyLogHandler.h @@ -0,0 +1,12 @@ +#pragma once + +#include +#include + +class DummyLogHandler : public Log::BaseLogHandler { +public: + DummyLogHandler() = default; + void addMessage(const Log::LogMessage &Message) override; +private: + Log::ThreadedExecutor Executor; +}; \ No newline at end of file diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp new file mode 100644 index 0000000..c1d8170 --- /dev/null +++ b/performance_test/PerformanceTest.cpp @@ -0,0 +1,25 @@ +#include +#include +#include +#include +#include "DummyLogHandler.h" + +static void BM_LogMessageGenerationOnly(benchmark::State& state) { + Log::LoggingBase Logger; + for (auto _ : state) { + Logger.log(Log::Severity::Error, "Some message."); + } +} +BENCHMARK(BM_LogMessageGenerationOnly); + +static void BM_LogMessageGenerationWithDummySink(benchmark::State& state) { + Log::LoggingBase Logger; + auto Handler = std::make_shared(); + Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); + for (auto _ : state) { + Logger.log(Log::Severity::Error, "Some message."); + } +} +BENCHMARK(BM_LogMessageGenerationWithDummySink); + +BENCHMARK_MAIN(); diff --git a/src/Log.cpp b/src/Log.cpp index 307dd96..65436d8 100644 --- a/src/Log.cpp +++ b/src/Log.cpp @@ -42,6 +42,10 @@ void Msg( Logger::Inst().log(Severity(Level), Message, ExtraFields); } +bool Flush(std::chrono::system_clock::duration Timeout){ + +} + void AddField(const std::string &Key, const AdditionalField &Value) { Logger::Inst().addField(Key, Value); } From 83f3af71c02ae7b00085ed8df355cd289f2a7b09 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Tue, 7 Jan 2020 15:22:53 +0100 Subject: [PATCH 03/43] =?UTF-8?q?[WIP]=C2=A0Improved=20performance=20a=20b?= =?UTF-8?q?it.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- conanfile.txt | 3 +- include/graylog_logger/Logger.hpp | 2 +- include/graylog_logger/LoggingBase.hpp | 47 +++++++++++--- performance_test/CMakeLists.txt | 5 +- performance_test/PerformanceTest.cpp | 15 ++++- src/Log.cpp | 2 +- src/Logger.cpp | 27 +++++---- src/LoggingBase.cpp | 84 ++++++++------------------ 8 files changed, 97 insertions(+), 88 deletions(-) diff --git a/conanfile.txt b/conanfile.txt index fe231ee..63dedf8 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -2,7 +2,8 @@ gtest/1.8.1@bincrafters/stable asio/1.13.0@bincrafters/stable jsonformoderncpp/3.6.1@vthiery/stable -google-benchmark/1.4.1-dm2@ess-dmsc/testing +google-benchmark/1.4.1-dm3@ess-dmsc/testing +fmt/6.0.0@bincrafters/stable [options] gtest:shared=False diff --git a/include/graylog_logger/Logger.hpp b/include/graylog_logger/Logger.hpp index 1ade6f3..b7dc030 100644 --- a/include/graylog_logger/Logger.hpp +++ b/include/graylog_logger/Logger.hpp @@ -18,7 +18,7 @@ class Logger : private LoggingBase { static Logger &Inst(); Logger(const Logger &) = delete; Logger &operator=(const Logger &) = delete; - virtual void addLogHandler(const LogHandler_P &Handler) override; + virtual void addLogHandler(const LogHandler_P Handler) override; using LoggingBase::addField; using LoggingBase::getHandlers; using LoggingBase::log; diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index e868514..a06b09b 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -10,9 +10,10 @@ #pragma once #include "graylog_logger/LogUtil.hpp" -#include #include #include +#include "graylog_logger/ThreadedExecutor.hpp" +#include namespace Log { @@ -20,28 +21,56 @@ class LoggingBase { public: LoggingBase(); virtual ~LoggingBase(); - virtual void log(const Severity Level, const std::string &Message); + virtual void log(const Severity Level, const std::string &Message) { + log(Level, Message, std::vector>()); + } virtual void log(const Severity Level, const std::string &Message, - const std::vector> &ExtraFields); + const std::vector> &ExtraFields) { + auto ThreadId = std::this_thread::get_id(); + Executor.SendWork([=](){ + if (int(Level) > int(MinSeverity)) { + return; + } + LogMessage cMsg(BaseMsg); + for (auto &fld : ExtraFields) { + cMsg.addField(fld.first, fld.second); + } + cMsg.Timestamp = std::chrono::system_clock::now(); + cMsg.MessageString = Message; + cMsg.SeverityLevel = Level; + std::ostringstream ss; + ss << ThreadId; + cMsg.ThreadId = ss.str(); + for (auto &ptr : Handlers) { + ptr->addMessage(cMsg); + } + }); + } virtual void log(const Severity Level, const std::string &Message, - const std::pair &ExtraField); - virtual void addLogHandler(const LogHandler_P &Handler); + const std::pair &ExtraField) { + log(Level, Message, std::vector>{ + ExtraField, + }); + } + virtual void addLogHandler(const LogHandler_P Handler); + template void addField(std::string Key, const valueType &Value) { - std::lock_guard Guard(BaseMsgMutex); - BaseMsg.addField(Key, Value); + Executor.SendWork([=](){ + BaseMsg.addField(Key, Value); + }); }; virtual void removeAllHandlers(); virtual void setMinSeverity(Severity Level); virtual std::vector getHandlers(); protected: - Severity MinSeverity; - std::mutex VectorMutex; + Severity MinSeverity{Severity::Notice}; std::mutex BaseMsgMutex; std::vector Handlers; LogMessage BaseMsg; + ThreadedExecutor Executor; }; } // namespace Log diff --git a/performance_test/CMakeLists.txt b/performance_test/CMakeLists.txt index be6b154..0797fee 100644 --- a/performance_test/CMakeLists.txt +++ b/performance_test/CMakeLists.txt @@ -1,7 +1,8 @@ -find_package(GoogleBenchmark) +find_package(GoogleBenchmark REQUIRED) +find_package(fmt REQUIRED) add_executable(performance_test PerformanceTest.cpp DummyLogHandler.cpp DummyLogHandler.h) -target_link_libraries(performance_test GraylogLogger::graylog_logger_static ${GoogleBenchmark_LIB}) +target_link_libraries(performance_test GraylogLogger::graylog_logger_static fmt::fmt ${GoogleBenchmark_LIB}) target_include_directories(performance_test PRIVATE ${GoogleBenchmark_INCLUDE_DIR}) \ No newline at end of file diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index c1d8170..e60fe43 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -1,14 +1,15 @@ #include #include #include -#include #include "DummyLogHandler.h" +#include static void BM_LogMessageGenerationOnly(benchmark::State& state) { Log::LoggingBase Logger; for (auto _ : state) { Logger.log(Log::Severity::Error, "Some message."); } + state.SetItemsProcessed(state.iterations()); } BENCHMARK(BM_LogMessageGenerationOnly); @@ -19,7 +20,19 @@ static void BM_LogMessageGenerationWithDummySink(benchmark::State& state) { for (auto _ : state) { Logger.log(Log::Severity::Error, "Some message."); } + state.SetItemsProcessed(state.iterations()); } BENCHMARK(BM_LogMessageGenerationWithDummySink); +static void BM_LogMessageGenerationWithFmtFormatting(benchmark::State& state) { + Log::LoggingBase Logger; + auto Handler = std::make_shared(); + Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); + for (auto _ : state) { + Logger.log(Log::Severity::Error, fmt::format("Some format example: {} : {}.", 3.14, 2.72)); + } + state.SetItemsProcessed(state.iterations()); +} +BENCHMARK(BM_LogMessageGenerationWithFmtFormatting); + BENCHMARK_MAIN(); diff --git a/src/Log.cpp b/src/Log.cpp index 65436d8..cd52847 100644 --- a/src/Log.cpp +++ b/src/Log.cpp @@ -43,7 +43,7 @@ void Msg( } bool Flush(std::chrono::system_clock::duration Timeout){ - + return false; } void AddField(const std::string &Key, const AdditionalField &Value) { diff --git a/src/Logger.cpp b/src/Logger.cpp index d998a01..002a3da 100644 --- a/src/Logger.cpp +++ b/src/Logger.cpp @@ -24,22 +24,23 @@ Logger::Logger() { Logger::addLogHandler(std::make_shared()); } -void Logger::addLogHandler(const LogHandler_P &Handler) { - std::lock_guard guard(VectorMutex); - if (dynamic_cast(Handler.get()) != nullptr) { - bool replaced = false; - for (auto ptr : Handlers) { - if (dynamic_cast(ptr.get()) != nullptr) { - ptr = Handler; - replaced = true; +void Logger::addLogHandler(const LogHandler_P Handler) { + Executor.SendWork([=]() { + if (dynamic_cast(Handler.get()) != nullptr) { + bool replaced = false; + for (auto ptr : Handlers) { + if (dynamic_cast(ptr.get()) != nullptr) { + ptr = Handler; + replaced = true; + } } - } - if (not replaced) { + if (not replaced) { + Handlers.push_back(Handler); + } + } else { Handlers.push_back(Handler); } - } else { - Handlers.push_back(Handler); - } + }); } } // namespace Log diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index 8997759..705bf42 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -10,8 +10,6 @@ #include "graylog_logger/LoggingBase.hpp" #include #include -#include -#include #include #include @@ -96,79 +94,45 @@ std::string get_process_name() { #endif LoggingBase::LoggingBase() { - std::lock_guard guard1(VectorMutex); - MinSeverity = Severity::Notice; - const int stringBufferSize = 100; - char stringBuffer[stringBufferSize]; - int res; - res = gethostname(static_cast(stringBuffer), stringBufferSize); - std::lock_guard guard2(BaseMsgMutex); - if (0 == res) { - BaseMsg.Host = std::string(static_cast(stringBuffer)); - } - BaseMsg.ProcessId = getpid(); - BaseMsg.ProcessName = get_process_name(); + Executor.SendWork([=](){ + const int stringBufferSize = 100; + char stringBuffer[stringBufferSize]; + int res; + res = gethostname(static_cast(stringBuffer), stringBufferSize); + if (0 == res) { + BaseMsg.Host = std::string(static_cast(stringBuffer)); + } + BaseMsg.ProcessId = getpid(); + BaseMsg.ProcessName = get_process_name(); + // We want to wait for this to finnish, make it so + }); } LoggingBase::~LoggingBase() { - std::lock_guard guard(VectorMutex); - Handlers.clear(); -} - -void LoggingBase::log(const Severity Level, const std::string &Message) { - log(Level, Message, std::vector>()); -} - -void LoggingBase::log( - const Severity Level, const std::string &Message, - const std::pair &ExtraField) { - log(Level, Message, std::vector>{ - ExtraField, - }); -} - -void LoggingBase::log( - const Severity Level, const std::string &Message, - const std::vector> &ExtraFields) { - if (int(Level) > int(MinSeverity)) { - return; - } - BaseMsgMutex.lock(); - LogMessage cMsg(BaseMsg); - BaseMsgMutex.unlock(); - for (auto &fld : ExtraFields) { - cMsg.addField(fld.first, fld.second); - } - cMsg.Timestamp = std::chrono::system_clock::now(); - cMsg.MessageString = Message; - cMsg.SeverityLevel = Level; - std::ostringstream ss; - ss << std::this_thread::get_id(); - cMsg.ThreadId = ss.str(); - std::lock_guard guard(VectorMutex); - for (auto &ptr : Handlers) { - ptr->addMessage(cMsg); - } + removeAllHandlers(); + // Flushing here? } -void LoggingBase::addLogHandler(const LogHandler_P &Handler) { - std::lock_guard guard(VectorMutex); - Handlers.push_back(Handler); +void LoggingBase::addLogHandler(const LogHandler_P Handler) { + Executor.SendWork([=]() { + Handlers.push_back(Handler); + }); } void LoggingBase::removeAllHandlers() { - std::lock_guard guard(VectorMutex); - Handlers.clear(); + Executor.SendWork([=]() { + Handlers.clear(); + }); } std::vector LoggingBase::getHandlers() { - std::lock_guard guard(VectorMutex); return Handlers; } void LoggingBase::setMinSeverity(Severity Level) { - std::lock_guard guard(VectorMutex); - MinSeverity = Level; + Executor.SendWork([=]() { + MinSeverity = Level; + }); } } // namespace Log From 075cfd85c10fdffba897fb339dabf83efcf10f8f Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Tue, 7 Jan 2020 16:11:10 +0100 Subject: [PATCH 04/43] [WIP] Implement use of moodycamel::concurrentqueue. --- CMakeLists.txt | 2 +- cmake/FindConcurrentQueue.cmake | 31 +++++++++++++++++++++ conanfile.txt | 1 + include/graylog_logger/ThreadedExecutor.hpp | 13 ++++++--- src/CMakeLists.txt | 3 ++ src/ThreadedExecutor.cpp | 2 ++ 6 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 cmake/FindConcurrentQueue.cmake create mode 100644 src/ThreadedExecutor.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f617ba..d8a9c40 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,7 +42,7 @@ find_package(ASIO REQUIRED MODULE) add_subdirectory(src) -option(BUILD_EVERYTHING "Build UnitTests & Console Logger" ON) +option(BUILD_EVERYTHING "Build UnitTests, Console Logger and Benchmarks" ON) IF(BUILD_EVERYTHING) add_subdirectory(unit_tests) diff --git a/cmake/FindConcurrentQueue.cmake b/cmake/FindConcurrentQueue.cmake new file mode 100644 index 0000000..86cc5a3 --- /dev/null +++ b/cmake/FindConcurrentQueue.cmake @@ -0,0 +1,31 @@ +find_path(ConcurrentQueue_ROOT_DIR + NAMES include/concurrentqueue/concurrentqueue.h + PATHS /usr/local /opt/local/ +) + +find_path(ConcurrentQueue_INCLUDE_DIR + NAMES concurrentqueue/concurrentqueue.h + HINTS ${ConcurrentQueue_ROOT_DIR}/include +) + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(ConcurrentQueue FOUND_VAR ConcurrentQueue_FOUND REQUIRED_VARS + ConcurrentQueue_ROOT_DIR + ConcurrentQueue_INCLUDE_DIR +) + +mark_as_advanced( + ConcurrentQueue_ROOT_DIR + ConcurrentQueue_INCLUDE_DIR +) + +if(ConcurrentQueue_FOUND) + set(ConcurrentQueue_INCLUDE_DIRS ${ConcurrentQueue_INCLUDE_DIR}) +endif() + +if(ConcurrentQueue_FOUND AND NOT TARGET ConcurrentQueue::ConcurrentQueue) + add_library(ConcurrentQueue::ConcurrentQueue INTERFACE IMPORTED) + set_target_properties(ConcurrentQueue::ConcurrentQueue PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${ConcurrentQueue_INCLUDE_DIR}" + ) +endif() diff --git a/conanfile.txt b/conanfile.txt index 63dedf8..cd0f939 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -3,6 +3,7 @@ gtest/1.8.1@bincrafters/stable asio/1.13.0@bincrafters/stable jsonformoderncpp/3.6.1@vthiery/stable google-benchmark/1.4.1-dm3@ess-dmsc/testing +concurrentqueue/8f7e861@ess-dmsc/stable fmt/6.0.0@bincrafters/stable [options] diff --git a/include/graylog_logger/ThreadedExecutor.hpp b/include/graylog_logger/ThreadedExecutor.hpp index b7a44c7..0808943 100644 --- a/include/graylog_logger/ThreadedExecutor.hpp +++ b/include/graylog_logger/ThreadedExecutor.hpp @@ -13,6 +13,7 @@ #include "graylog_logger/ConcurrentQueue.hpp" #include #include +#include namespace Log { class ThreadedExecutor { @@ -24,18 +25,22 @@ class ThreadedExecutor { SendWork([=]() { RunThread = false; }); WorkerThread.join(); } - void SendWork(WorkMessage Message) { MessageQueue.push(Message); } + void SendWork(WorkMessage Message) { MessageQueue.enqueue(Message); } private: bool RunThread{true}; std::function ThreadFunction{[=]() { while (RunThread) { WorkMessage CurrentMessage; - MessageQueue.wait_and_pop(CurrentMessage); - CurrentMessage(); + if (MessageQueue.try_dequeue(CurrentMessage)) { + CurrentMessage(); + } else { + using namespace std::chrono_literals; + std::this_thread::sleep_for(5ms); + } } }}; - ConcurrentQueue MessageQueue; + moodycamel::ConcurrentQueue MessageQueue; std::thread WorkerThread; }; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 22c1428..b5f6f8e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,4 +1,5 @@ find_package(Threads REQUIRED) +find_package(ConcurrentQueue REQUIRED) find_package(nlohmann_json REQUIRED) set(Graylog_SRC @@ -10,6 +11,7 @@ set(Graylog_SRC Logger.cpp LoggingBase.cpp LogUtil.cpp + ThreadedExecutor.cpp ) set(Graylog_INC @@ -28,6 +30,7 @@ set(Graylog_INC set(common_libs PUBLIC Threads::Threads + ConcurrentQueue::ConcurrentQueue ) get_target_property(JSON_INCLUDE_DIR nlohmann_json::nlohmann_json INTERFACE_INCLUDE_DIRECTORIES) diff --git a/src/ThreadedExecutor.cpp b/src/ThreadedExecutor.cpp new file mode 100644 index 0000000..7c70cdd --- /dev/null +++ b/src/ThreadedExecutor.cpp @@ -0,0 +1,2 @@ +#include "graylog_logger/ThreadedExecutor.hpp" + From 79faf2efe1365ed2645739d2e3f3aecf80de9c7e Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Wed, 8 Jan 2020 11:56:55 +0100 Subject: [PATCH 05/43] =?UTF-8?q?[WIP]=C2=A0Added=20libfmt=20support,=20re?= =?UTF-8?q?moved=20old=20concurrent=20queue.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- include/graylog_logger/ConcurrentQueue.hpp | 120 -------------------- include/graylog_logger/GraylogInterface.hpp | 4 +- include/graylog_logger/Log.hpp | 4 + include/graylog_logger/LogUtil.hpp | 27 ++--- include/graylog_logger/LoggingBase.hpp | 29 +++++ include/graylog_logger/MinimalApply.h | 22 ++++ include/graylog_logger/ThreadedExecutor.hpp | 1 - performance_test/PerformanceTest.cpp | 14 ++- src/CMakeLists.txt | 10 +- src/ConsoleInterface.cpp | 2 +- src/FileInterface.cpp | 2 +- src/GraylogConnection.cpp | 17 +-- src/GraylogConnection.hpp | 8 +- src/GraylogInterface.cpp | 18 +-- src/LogUtil.cpp | 13 --- 15 files changed, 90 insertions(+), 201 deletions(-) delete mode 100644 include/graylog_logger/ConcurrentQueue.hpp create mode 100644 include/graylog_logger/MinimalApply.h diff --git a/include/graylog_logger/ConcurrentQueue.hpp b/include/graylog_logger/ConcurrentQueue.hpp deleted file mode 100644 index eeb372d..0000000 --- a/include/graylog_logger/ConcurrentQueue.hpp +++ /dev/null @@ -1,120 +0,0 @@ -/* Copyright (C) 2018 European Spallation Source, ERIC. See LICENSE file */ -//===----------------------------------------------------------------------===// -/// -/// \file -/// -/// \brief Simple thread-safe message queue. -/// -//===----------------------------------------------------------------------===// - -#pragma once - -#include -#include -#include - -template class ConcurrentQueue { -private: - std::queue the_queue; - mutable std::mutex the_mutex; - std::condition_variable the_condition_variable; - -public: - void push(Data const &data) { - { - std::lock_guard lock(the_mutex); - the_queue.push(data); - } - the_condition_variable.notify_one(); - } - - unsigned long size() { - unsigned long retVal; - std::lock_guard lock(the_mutex); - retVal = the_queue.size(); - return retVal; - } - - bool empty() const { - std::lock_guard lock(the_mutex); - return the_queue.empty(); - } - - bool try_pop(Data &popped_value) { - std::lock_guard lock(the_mutex); - if (the_queue.empty()) { - return false; - } - - popped_value = the_queue.front(); - the_queue.pop(); - return true; - } - - bool try_pop() { - std::lock_guard lock(the_mutex); - if (the_queue.empty()) { - return false; - } - the_queue.pop(); - return true; - } - - bool try_peek(Data &peek_value) { - std::lock_guard lock(the_mutex); - if (the_queue.empty()) { - return false; - } - - peek_value = the_queue.front(); - return true; - } - - void wait_and_pop(Data &popped_value) { - std::unique_lock lock(the_mutex); - while (the_queue.empty()) { - the_condition_variable.wait(lock); - } - - popped_value = the_queue.front(); - the_queue.pop(); - } - - void wait_and_peek(Data &peek_value) { - std::unique_lock lock(the_mutex); - while (the_queue.empty()) { - the_condition_variable.wait(lock); - } - - peek_value = the_queue.front(); - } - - bool time_out_pop(Data &popped_value, int mSec = 500) { - std::unique_lock lock(the_mutex); - if (the_queue.empty()) { - the_condition_variable.wait_for(lock, std::chrono::milliseconds(mSec)); - } - - if (the_queue.empty()) { - return false; - } - - popped_value = the_queue.front(); - the_queue.pop(); - return true; - } - - bool time_out_peek(Data &peek_value, int mSec = 500) { - std::unique_lock lock(the_mutex); - if (the_queue.empty()) { - the_condition_variable.wait_for(lock, std::chrono::milliseconds(mSec)); - } - - if (the_queue.empty()) { - return false; - } - - peek_value = the_queue.front(); - return true; - } -}; diff --git a/include/graylog_logger/GraylogInterface.hpp b/include/graylog_logger/GraylogInterface.hpp index 7c30ae7..06c1e81 100644 --- a/include/graylog_logger/GraylogInterface.hpp +++ b/include/graylog_logger/GraylogInterface.hpp @@ -33,8 +33,8 @@ class GraylogInterface : public BaseLogHandler, public GraylogConnection { size_t MaxQueueLength = 100); ~GraylogInterface() override = default; void addMessage(const LogMessage &Message) override; - bool emptyQueue() override; - size_t queueSize() override; +// bool emptyQueue() override; +// size_t queueSize() override; protected: std::string logMsgToJSON(const LogMessage &Message); diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 8cf57e1..6b5e6da 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -109,6 +109,10 @@ void Msg( /// /// \note Requires that the relevant log handlers implements flushing /// functionality. +/// \note For the built in log handlers, flush only guarantees that log +/// messages created before the call to Flush() have been handled. Log messages +/// created after the call to Flush() can not be guaranteed to have been +/// handled. /// \param[in] Timeout The amount of time to wait for the queued messages to /// be flushed. /// \return Returns true if messages were flushed, false otherwise. diff --git a/include/graylog_logger/LogUtil.hpp b/include/graylog_logger/LogUtil.hpp index 41af262..e80a29a 100644 --- a/include/graylog_logger/LogUtil.hpp +++ b/include/graylog_logger/LogUtil.hpp @@ -10,11 +10,11 @@ #pragma once -#include "graylog_logger/ConcurrentQueue.hpp" #include #include #include #include +#include namespace Log { @@ -108,24 +108,15 @@ class BaseLogHandler { public: /// \brief Does minimal set-up of the this class. /// \param[in] MaxQueueLength The maximum number of log messages stored. - explicit BaseLogHandler(const size_t MaxQueueLength = 100); + BaseLogHandler() = default; virtual ~BaseLogHandler() = default; + /// \brief Called by the logging library when a new log message is created. - /// \note If the queue of messages is full, any new messages are discarded - /// without any indication that this has been done. + /// + /// Must be implement by any derived classes. /// \param[in] Message The log message. - virtual void addMessage(const LogMessage &Message); - /// \brief Are there messages in the queue? - /// \note As messages can be added and removed by several different threads, - /// expect that the return value will change between two calls. - /// \return true if there are no messages in the queue, otherwise - /// false. - virtual bool emptyQueue(); - /// \brief The number of messages in the queue. - /// \note As messages can be added and removed by several different threads, - /// expect that the return value will change between two calls. - /// \return The number of messages in the queue. - virtual size_t queueSize(); + virtual void addMessage(const LogMessage &Message) = 0; + /// \brief Used to set a custom log message to std::string formatting /// function. /// @@ -137,10 +128,6 @@ class BaseLogHandler { std::function ParserFunction); protected: - size_t QueueLength; - /// \brief Pop messages from this queue if implementing a log message - /// consumer (handler). - ConcurrentQueue MessageQueue; /// \brief Can be used to create strings from messages if set. std::function MessageParser{nullptr}; /// \brief The default log message to std::string function. diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index a06b09b..49670e7 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -14,6 +14,9 @@ #include #include "graylog_logger/ThreadedExecutor.hpp" #include +#include +#include +#include "graylog_logger/MinimalApply.h" namespace Log { @@ -53,6 +56,32 @@ class LoggingBase { ExtraField, }); } + + template + void fmt_log(const Severity Level, std::string Format, Args... args) { + auto ThreadId = std::this_thread::get_id(); + auto UsedArguments = std::make_tuple(args...); + Executor.SendWork([=](){ + if (int(Level) > int(MinSeverity)) { + return; + } + LogMessage cMsg(BaseMsg); + cMsg.Timestamp = std::chrono::system_clock::now(); + auto format_message = [=](const auto&... args) { + return fmt::format(Format, args...); + }; + + cMsg.MessageString = minimal::apply(format_message, UsedArguments); + cMsg.SeverityLevel = Level; + std::ostringstream ss; + ss << ThreadId; + cMsg.ThreadId = ss.str(); + for (auto &ptr : Handlers) { + ptr->addMessage(cMsg); + } + }); + } + virtual void addLogHandler(const LogHandler_P Handler); template diff --git a/include/graylog_logger/MinimalApply.h b/include/graylog_logger/MinimalApply.h new file mode 100644 index 0000000..41b12ed --- /dev/null +++ b/include/graylog_logger/MinimalApply.h @@ -0,0 +1,22 @@ +#include + +namespace minimal { +template +constexpr decltype(auto) invoke(F &&f, Args &&... args) { + return std::forward(f)(std::forward(args)...); +} + +namespace detail { +template +constexpr decltype(auto) apply_impl(F &&f, Tuple &&t, std::index_sequence) { + return invoke(std::forward(f), std::get(std::forward(t))...); +} +} // namespace detail + +template +constexpr decltype(auto) apply(F &&f, Tuple &&t) { + return detail::apply_impl( + std::forward(f), std::forward(t), + std::make_index_sequence>::value>()); +} +} // namespace minimal \ No newline at end of file diff --git a/include/graylog_logger/ThreadedExecutor.hpp b/include/graylog_logger/ThreadedExecutor.hpp index 0808943..7b0f8ca 100644 --- a/include/graylog_logger/ThreadedExecutor.hpp +++ b/include/graylog_logger/ThreadedExecutor.hpp @@ -10,7 +10,6 @@ #pragma once -#include "graylog_logger/ConcurrentQueue.hpp" #include #include #include diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index e60fe43..a214165 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -29,10 +29,22 @@ static void BM_LogMessageGenerationWithFmtFormatting(benchmark::State& state) { auto Handler = std::make_shared(); Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); for (auto _ : state) { - Logger.log(Log::Severity::Error, fmt::format("Some format example: {} : {}.", 3.14, 2.72)); + Logger.log(Log::Severity::Error, fmt::format("Some format example: {} : {} : {}.", 3.14, 2.72, "some_string")); } state.SetItemsProcessed(state.iterations()); } BENCHMARK(BM_LogMessageGenerationWithFmtFormatting); +static void BM_LogMessageGenerationWithDeferredFmtFormatting(benchmark::State& state) { + Log::LoggingBase Logger; + auto Handler = std::make_shared(); + Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); + for (auto _ : state) { + Logger.fmt_log(Log::Severity::Error, "Some format example: {} : {} : {}.", 3.14, 2.72, "some_string"); + + } + state.SetItemsProcessed(state.iterations()); +} +BENCHMARK(BM_LogMessageGenerationWithDeferredFmtFormatting); + BENCHMARK_MAIN(); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index b5f6f8e..6bdd1e9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,6 +1,7 @@ find_package(Threads REQUIRED) find_package(ConcurrentQueue REQUIRED) find_package(nlohmann_json REQUIRED) +find_package(fmt REQUIRED) set(Graylog_SRC ConsoleInterface.cpp @@ -15,7 +16,6 @@ set(Graylog_SRC ) set(Graylog_INC - ../include/graylog_logger/ConcurrentQueue.hpp ../include/graylog_logger/ConsoleInterface.hpp ../include/graylog_logger/FileInterface.hpp GraylogConnection.hpp @@ -25,12 +25,13 @@ set(Graylog_INC ../include/graylog_logger/LoggingBase.hpp ../include/graylog_logger/LogUtil.hpp ../include/graylog_logger/ThreadedExecutor.hpp - ../include/graylog_logger/ConnectionStatus.hpp) + ../include/graylog_logger/ConnectionStatus.hpp ../include/graylog_logger/MinimalApply.h) set(common_libs PUBLIC Threads::Threads ConcurrentQueue::ConcurrentQueue + fmt::fmt ) get_target_property(JSON_INCLUDE_DIR nlohmann_json::nlohmann_json INTERFACE_INCLUDE_DIRECTORIES) @@ -65,10 +66,7 @@ set_target_properties(graylog_logger PROPERTIES VERSION ${PROJECT_VERSION}) set_target_properties(graylog_logger_static PROPERTIES VERSION ${PROJECT_VERSION} POSITION_INDEPENDENT_CODE ON) if (NOT WIN32) - add_cppcheck_target( - graylog_logger - "../include/graylog_logger/ConcurrentQueue.hpp" - ) + add_cppcheck_target(graylog_logger) endif(NOT WIN32) diff --git a/src/ConsoleInterface.cpp b/src/ConsoleInterface.cpp index 4eb1a92..25388d2 100644 --- a/src/ConsoleInterface.cpp +++ b/src/ConsoleInterface.cpp @@ -23,7 +23,7 @@ std::string ConsoleStringCreator(const LogMessage &Message) { } ConsoleInterface::ConsoleInterface(size_t MaxQueueLength) - : BaseLogHandler(MaxQueueLength) { + : BaseLogHandler() { BaseLogHandler::setMessageStringCreatorFunction(ConsoleStringCreator); } void ConsoleInterface::addMessage(const LogMessage &Message) { diff --git a/src/FileInterface.cpp b/src/FileInterface.cpp index 4000d4e..bb8d35d 100644 --- a/src/FileInterface.cpp +++ b/src/FileInterface.cpp @@ -16,7 +16,7 @@ namespace Log { FileInterface::FileInterface(std::string const &Name, const size_t MaxQueueLength) - : BaseLogHandler(MaxQueueLength), FileStream(Name, std::ios::app) { + : BaseLogHandler(), FileStream(Name, std::ios::app) { if (FileStream.is_open() and FileStream.good()) { Log::Msg(Severity::Info, "Started logging to log file: \"" + Name + "\""); } else { diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index 52a5c21..fac0efc 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -135,8 +135,8 @@ void GraylogConnection::Impl::trySendMessage() { return; } std::string NewMessage; - bool PopResult = LogMessages.try_pop(NewMessage); - if (PopResult) { + using namespace std::chrono_literals; + if (LogMessages.wait_dequeue_timed(NewMessage, 10ms)) { std::copy(NewMessage.begin(), NewMessage.end(), std::back_inserter(MessageBuffer)); MessageBuffer.push_back('\0'); @@ -166,19 +166,6 @@ void GraylogConnection::Impl::sentMessageHandler(const asio::error_code &Error, trySendMessage(); } -void GraylogConnection::Impl::waitForMessage() { - if (not Socket.is_open()) { - return; - } - const int WaitTimeMS = 20; - std::string ThrowAwayMessage; - if (LogMessages.time_out_peek(ThrowAwayMessage, WaitTimeMS)) { - trySendMessage(); - return; - } - Service.post([this]() { this->waitForMessage(); }); -} - void GraylogConnection::Impl::doAddressQuery() { setState(Status::ADDR_LOOKUP); asio::ip::tcp::resolver::query Query(HostAddress, HostPort); diff --git a/src/GraylogConnection.hpp b/src/GraylogConnection.hpp index ec0541f..b510452 100644 --- a/src/GraylogConnection.hpp +++ b/src/GraylogConnection.hpp @@ -9,9 +9,9 @@ #pragma once -#include "graylog_logger/ConcurrentQueue.hpp" #include "graylog_logger/ConnectionStatus.hpp" #include "graylog_logger/GraylogInterface.hpp" +#include #include #include #include @@ -31,10 +31,8 @@ class GraylogConnection::Impl { using Status = Log::Status; Impl(std::string Host, int Port); virtual ~Impl(); - virtual void sendMessage(std::string Msg) { LogMessages.push(Msg); }; + virtual void sendMessage(std::string Msg) { LogMessages.enqueue(Msg); }; Status getConnectionStatus() const; - bool messageQueueEmpty() { return LogMessages.empty(); } - size_t messageQueueSize() { return LogMessages.size(); } protected: enum class ReconnectDelay { LONG, SHORT }; @@ -52,7 +50,7 @@ class GraylogConnection::Impl { std::string HostPort; std::thread AsioThread; - ConcurrentQueue LogMessages; + moodycamel::BlockingConcurrentQueue LogMessages; private: const size_t MessageAdditionLimit{3000}; diff --git a/src/GraylogInterface.cpp b/src/GraylogInterface.cpp index 174c0b0..4a0ddfe 100644 --- a/src/GraylogInterface.cpp +++ b/src/GraylogInterface.cpp @@ -27,28 +27,14 @@ Status GraylogConnection::getConnectionStatus() const { return Pimpl->getConnectionStatus(); } -bool GraylogConnection::messageQueueEmpty() { - return Pimpl->messageQueueEmpty(); -} - -size_t GraylogConnection::messageQueueSize() { - return Pimpl->messageQueueSize(); -} - GraylogConnection::~GraylogConnection() {} GraylogInterface::GraylogInterface(const std::string &Host, const int Port, const size_t MaxQueueLength) - : BaseLogHandler(MaxQueueLength), GraylogConnection(Host, Port) {} - -bool GraylogInterface::emptyQueue() { return messageQueueEmpty(); } - -size_t GraylogInterface::queueSize() { return messageQueueSize(); } + : BaseLogHandler(), GraylogConnection(Host, Port) {} void GraylogInterface::addMessage(const LogMessage &Message) { - if (messageQueueSize() < BaseLogHandler::QueueLength) { - sendMessage(logMsgToJSON(Message)); - } + sendMessage(logMsgToJSON(Message)); } std::string GraylogInterface::logMsgToJSON(const LogMessage &Message) { diff --git a/src/LogUtil.cpp b/src/LogUtil.cpp index be82640..11f2b3a 100644 --- a/src/LogUtil.cpp +++ b/src/LogUtil.cpp @@ -15,24 +15,11 @@ namespace Log { -BaseLogHandler::BaseLogHandler(const size_t MaxQueueLength) - : QueueLength(MaxQueueLength) {} - void BaseLogHandler::setMessageStringCreatorFunction( std::function ParserFunction) { BaseLogHandler::MessageParser = std::move(ParserFunction); } -void BaseLogHandler::addMessage(const LogMessage &Message) { - if (MessageQueue.size() < QueueLength) { - MessageQueue.push(Message); - } -} - -bool BaseLogHandler::emptyQueue() { return MessageQueue.empty(); } - -size_t BaseLogHandler::queueSize() { return MessageQueue.size(); } - std::string BaseLogHandler::messageToString(const LogMessage &Message) { if (nullptr != MessageParser) { return MessageParser(Message); From 8c98a0d58a3fa606dc17f1e5168a5bf25f11473e Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Wed, 8 Jan 2020 17:50:22 +0100 Subject: [PATCH 06/43] =?UTF-8?q?[WIP]=20Most=20of=20the=C2=A0flushing=20f?= =?UTF-8?q?unctionality.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- include/graylog_logger/ConsoleInterface.hpp | 19 +++++++++++++- include/graylog_logger/FileInterface.hpp | 18 +++++++++++++ include/graylog_logger/GraylogInterface.hpp | 19 ++++++++++++-- include/graylog_logger/Log.hpp | 10 ++++---- include/graylog_logger/LogUtil.hpp | 19 ++++++++++++++ include/graylog_logger/Logger.hpp | 3 ++- include/graylog_logger/LoggingBase.hpp | 28 ++++++++++++++++++++- include/graylog_logger/MinimalApply.h | 10 ++++++++ include/graylog_logger/ThreadedExecutor.hpp | 10 ++++++++ performance_test/DummyLogHandler.h | 3 +++ src/ConsoleInterface.cpp | 12 ++++++++- src/FileInterface.cpp | 10 ++++++++ src/Log.cpp | 4 +-- src/Logger.cpp | 2 +- src/LoggingBase.cpp | 2 +- 15 files changed, 154 insertions(+), 15 deletions(-) diff --git a/include/graylog_logger/ConsoleInterface.hpp b/include/graylog_logger/ConsoleInterface.hpp index 08d5145..f9202e6 100644 --- a/include/graylog_logger/ConsoleInterface.hpp +++ b/include/graylog_logger/ConsoleInterface.hpp @@ -16,8 +16,25 @@ namespace Log { class ConsoleInterface : public BaseLogHandler { public: - explicit ConsoleInterface(size_t MaxQueueLength = 100); + explicit ConsoleInterface(); void addMessage(const LogMessage &Message) override; + /// \brief Waits for all messages created before the call to flush to be + /// printed and then flushes the output stream. + /// \param[in] TimeOut Amount of time to wait for messages to be written. + /// \return Returns true if queue was emptied and stream flushed before the + /// time out. Returns false otherwise. + bool flush(std::chrono::system_clock::duration TimeOut) override; + + /// \brief Are there any more queued messages? + /// \note The message queue will show as empty before the last message in + /// the queue has been written to console. + /// \return Returns true if message queue is empty. + bool emptyQueue() override; + + /// \brief Number of queued messages. + /// \return Due to multiple threads accessing this queue, shows approximate + /// number of messages in the queue. + size_t queueSize() override; protected: ThreadedExecutor Executor; // Must be last diff --git a/include/graylog_logger/FileInterface.hpp b/include/graylog_logger/FileInterface.hpp index f17ee29..70d4510 100644 --- a/include/graylog_logger/FileInterface.hpp +++ b/include/graylog_logger/FileInterface.hpp @@ -22,6 +22,24 @@ class FileInterface : public BaseLogHandler { const size_t MaxQueueLength = 100); void addMessage(const LogMessage &Message) override; + /// \brief Waits for all messages created before the call to flush to be + /// printed and then flushes the file stream. + /// \param[in] TimeOut Amount of time to wait for messages to be written. + /// \return Returns true if queue was emptied and stream flushed before the + /// time out. Returns false otherwise. + bool flush(std::chrono::system_clock::duration TimeOut) override; + + /// \brief Are there any more queued messages? + /// \note The message queue will show as empty before the last message in + /// the queue has been written. + /// \return Returns true if message queue is empty. + bool emptyQueue() override; + + /// \brief Number of queued messages. + /// \return Due to multiple threads accessing this queue, shows approximate + /// number of messages in the queue. + size_t queueSize() override; + protected: std::ofstream FileStream; ThreadedExecutor Executor; // Must be last diff --git a/include/graylog_logger/GraylogInterface.hpp b/include/graylog_logger/GraylogInterface.hpp index 06c1e81..e3c73b3 100644 --- a/include/graylog_logger/GraylogInterface.hpp +++ b/include/graylog_logger/GraylogInterface.hpp @@ -33,8 +33,23 @@ class GraylogInterface : public BaseLogHandler, public GraylogConnection { size_t MaxQueueLength = 100); ~GraylogInterface() override = default; void addMessage(const LogMessage &Message) override; -// bool emptyQueue() override; -// size_t queueSize() override; + /// \brief Waits for all messages created before the call to flush to be + /// transmitted. + /// \param[in] TimeOut Amount of time to wait for messages to be transmitted. + /// \return Returns true if messages were transmitted before the time out. + /// Returns false otherwise. + bool flush(std::chrono::system_clock::duration TimeOut) override; + + /// \brief Are there any more queued messages? + /// \note The message queue will show as empty before the last message in + /// the queue has been transmitted. + /// \return Returns true if message queue is empty. + bool emptyQueue() override; + + /// \brief Number of queued messages. + /// \return Due to multiple threads accessing this queue, shows approximate + /// number of messages in the queue. + size_t queueSize() override; protected: std::string logMsgToJSON(const LogMessage &Message); diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 6b5e6da..20aa9dd 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -107,16 +107,16 @@ void Msg( /// \brief Flush log messages in the queues of the log handlers. /// -/// \note Requires that the relevant log handlers implements flushing -/// functionality. -/// \note For the built in log handlers, flush only guarantees that log +/// \note Exact implementation depends on that of the currently used log +/// handlers. +/// \note For the built-in log handlers, flush only guarantees that log /// messages created before the call to Flush() have been handled. Log messages /// created after the call to Flush() can not be guaranteed to have been /// handled. -/// \param[in] Timeout The amount of time to wait for the queued messages to +/// \param[in] TimeOut The amount of time to wait for the queued messages to /// be flushed. /// \return Returns true if messages were flushed, false otherwise. -bool Flush(std::chrono::system_clock::duration Timeout); +bool Flush(std::chrono::system_clock::duration TimeOut); /// \brief Add a default field of meta-data to every message. /// diff --git a/include/graylog_logger/LogUtil.hpp b/include/graylog_logger/LogUtil.hpp index e80a29a..85c99a5 100644 --- a/include/graylog_logger/LogUtil.hpp +++ b/include/graylog_logger/LogUtil.hpp @@ -117,6 +117,23 @@ class BaseLogHandler { /// \param[in] Message The log message. virtual void addMessage(const LogMessage &Message) = 0; + /// \brief Empty the queue of messages. Might do nothing. See documentation + /// of derived classes for details. + /// \param[in] TimeOut Amount of time to wait queue to empty. + /// \return Returns true if queue was emptied before time out occurred. + virtual bool flush(std::chrono::system_clock::duration TimeOut) = 0; + + /// \brief Are there messages in the queue? + /// \note See derived classes for implementation details. + /// \return true if there are no messages in the queue, otherwise + /// false. + virtual bool emptyQueue() = 0; + + /// \brief The number of messages in the queue. + /// \note See derived class for implementation details. + /// \return The number of messages in the queue. + virtual size_t queueSize() = 0; + /// \brief Used to set a custom log message to std::string formatting /// function. /// @@ -127,6 +144,8 @@ class BaseLogHandler { void setMessageStringCreatorFunction( std::function ParserFunction); + + protected: /// \brief Can be used to create strings from messages if set. std::function MessageParser{nullptr}; diff --git a/include/graylog_logger/Logger.hpp b/include/graylog_logger/Logger.hpp index b7dc030..6d9e4e6 100644 --- a/include/graylog_logger/Logger.hpp +++ b/include/graylog_logger/Logger.hpp @@ -18,12 +18,13 @@ class Logger : private LoggingBase { static Logger &Inst(); Logger(const Logger &) = delete; Logger &operator=(const Logger &) = delete; - virtual void addLogHandler(const LogHandler_P Handler) override; + virtual void addLogHandler(const LogHandler_P& Handler) override; using LoggingBase::addField; using LoggingBase::getHandlers; using LoggingBase::log; using LoggingBase::removeAllHandlers; using LoggingBase::setMinSeverity; + using LoggingBase::flush; private: Logger(); diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 49670e7..7dedeb3 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -17,6 +17,8 @@ #include #include #include "graylog_logger/MinimalApply.h" +#include +#include namespace Log { @@ -82,7 +84,7 @@ class LoggingBase { }); } - virtual void addLogHandler(const LogHandler_P Handler); + virtual void addLogHandler(const LogHandler_P& Handler); template void addField(std::string Key, const valueType &Value) { @@ -94,6 +96,30 @@ class LoggingBase { virtual void setMinSeverity(Severity Level); virtual std::vector getHandlers(); + virtual bool flush(std::chrono::system_clock::duration TimeOut) { + auto FlushCompleted = std::make_shared>(); + auto FlushCompletedValue = FlushCompleted->get_future(); + Executor.SendWork([=,FlushCompleted{std::move(FlushCompleted)}](){ + std::vector> FlushResults; + for (auto& CHandler : Handlers) { + FlushResults.push_back(std::async(std::launch::async, [=](){ + return CHandler->flush(TimeOut);} + )); + } + bool ReturnValue{true}; + for (auto& CFlushResult: FlushResults) { + CFlushResult.wait(); + if (not CFlushResult.get()) { + ReturnValue = false; + } + } + FlushCompleted->set_value(ReturnValue); + }); + std::vector> FlushResult; + FlushCompletedValue.wait(); + return FlushCompletedValue.get(); + } + protected: Severity MinSeverity{Severity::Notice}; std::mutex BaseMsgMutex; diff --git a/include/graylog_logger/MinimalApply.h b/include/graylog_logger/MinimalApply.h index 41b12ed..043e573 100644 --- a/include/graylog_logger/MinimalApply.h +++ b/include/graylog_logger/MinimalApply.h @@ -1,3 +1,13 @@ +/* Copyright (C) 2020 European Spallation Source, ERIC. See LICENSE file */ +//===----------------------------------------------------------------------===// +/// +/// \file +/// +/// \brief Some functions from C++17 for using std::tuple with variadic +/// functions. +/// +//===----------------------------------------------------------------------===// + #include namespace minimal { diff --git a/include/graylog_logger/ThreadedExecutor.hpp b/include/graylog_logger/ThreadedExecutor.hpp index 7b0f8ca..b69bb76 100644 --- a/include/graylog_logger/ThreadedExecutor.hpp +++ b/include/graylog_logger/ThreadedExecutor.hpp @@ -13,6 +13,8 @@ #include #include #include +#include +#include namespace Log { class ThreadedExecutor { @@ -25,6 +27,14 @@ class ThreadedExecutor { WorkerThread.join(); } void SendWork(WorkMessage Message) { MessageQueue.enqueue(Message); } + bool WaitForWorkSemaphore(std::chrono::system_clock::duration WaitTime) { + auto WorkDone = std::make_shared>(); + auto WorkDoneFuture = WorkDone->get_future(); + SendWork([WorkDone = std::move(WorkDone)](){ + WorkDone->set_value(true); + }); + return std::future_status::ready == WorkDoneFuture.wait_for(WaitTime); + } private: bool RunThread{true}; diff --git a/performance_test/DummyLogHandler.h b/performance_test/DummyLogHandler.h index 10fd524..bb41da0 100644 --- a/performance_test/DummyLogHandler.h +++ b/performance_test/DummyLogHandler.h @@ -7,6 +7,9 @@ class DummyLogHandler : public Log::BaseLogHandler { public: DummyLogHandler() = default; void addMessage(const Log::LogMessage &Message) override; + bool emptyQueue() override {return true;} + size_t queueSize() override {return 0;} + bool flush(std::chrono::system_clock::duration) override {return true;} private: Log::ThreadedExecutor Executor; }; \ No newline at end of file diff --git a/src/ConsoleInterface.cpp b/src/ConsoleInterface.cpp index 25388d2..f2c499e 100644 --- a/src/ConsoleInterface.cpp +++ b/src/ConsoleInterface.cpp @@ -22,7 +22,7 @@ std::string ConsoleStringCreator(const LogMessage &Message) { Message.MessageString; } -ConsoleInterface::ConsoleInterface(size_t MaxQueueLength) +ConsoleInterface::ConsoleInterface() : BaseLogHandler() { BaseLogHandler::setMessageStringCreatorFunction(ConsoleStringCreator); } @@ -31,4 +31,14 @@ void ConsoleInterface::addMessage(const LogMessage &Message) { [=]() { std::cout << BaseLogHandler::MessageParser(Message) << "\n"; }); } +bool ConsoleInterface::flush(std::chrono::system_clock::duration TimeOut) { + auto WorkDone = std::make_shared>(); + auto WorkDoneFuture = WorkDone->get_future(); + Executor.SendWork([=, WorkDone{std::move(WorkDone)}](){ + std::cout.flush(); + WorkDone->set_value(); + }); + return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); +} + } // namespace Log diff --git a/src/FileInterface.cpp b/src/FileInterface.cpp index bb8d35d..79254a5 100644 --- a/src/FileInterface.cpp +++ b/src/FileInterface.cpp @@ -33,4 +33,14 @@ void FileInterface::addMessage(const LogMessage &Message) { }); } +bool FileInterface::flush(std::chrono::system_clock::duration TimeOut) { + auto WorkDone = std::make_shared>(); + auto WorkDoneFuture = WorkDone->get_future(); + Executor.SendWork([=, WorkDone{std::move(WorkDone)}](){ + FileStream.flush(); + WorkDone->set_value(); + }); + return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); +} + } // namespace Log diff --git a/src/Log.cpp b/src/Log.cpp index cd52847..aa80d31 100644 --- a/src/Log.cpp +++ b/src/Log.cpp @@ -42,8 +42,8 @@ void Msg( Logger::Inst().log(Severity(Level), Message, ExtraFields); } -bool Flush(std::chrono::system_clock::duration Timeout){ - return false; +bool Flush(std::chrono::system_clock::duration TimeOut) { + return Logger::Inst().flush(TimeOut); } void AddField(const std::string &Key, const AdditionalField &Value) { diff --git a/src/Logger.cpp b/src/Logger.cpp index 002a3da..ebead88 100644 --- a/src/Logger.cpp +++ b/src/Logger.cpp @@ -24,7 +24,7 @@ Logger::Logger() { Logger::addLogHandler(std::make_shared()); } -void Logger::addLogHandler(const LogHandler_P Handler) { +void Logger::addLogHandler(const LogHandler_P& Handler) { Executor.SendWork([=]() { if (dynamic_cast(Handler.get()) != nullptr) { bool replaced = false; diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index 705bf42..3811eae 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -113,7 +113,7 @@ LoggingBase::~LoggingBase() { // Flushing here? } -void LoggingBase::addLogHandler(const LogHandler_P Handler) { +void LoggingBase::addLogHandler(const LogHandler_P& Handler) { //Is this correct? (Use after return?) Executor.SendWork([=]() { Handlers.push_back(Handler); }); From ad4f17c2b58faaa1dd79d2217331ea06150e4921 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Wed, 8 Jan 2020 19:00:00 +0100 Subject: [PATCH 07/43] =?UTF-8?q?[WIP]=C2=A0Added=20missing=20implementati?= =?UTF-8?q?ons=20for=20flush=20and=20get=20queue=20size.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- include/graylog_logger/GraylogInterface.hpp | 3 +++ include/graylog_logger/ThreadedExecutor.hpp | 9 ++----- src/ConsoleInterface.cpp | 8 +++++++ src/FileInterface.cpp | 8 +++++++ src/GraylogConnection.cpp | 26 +++++++++++++++++---- src/GraylogConnection.hpp | 12 ++++++++-- src/GraylogInterface.cpp | 24 +++++++++++++++++++ unit_tests/BaseLogHandlerStandIn.hpp | 3 +++ unit_tests/FileInterfaceTest.cpp | 1 - unit_tests/QueueLengthTest.cpp | 5 ++-- 10 files changed, 82 insertions(+), 17 deletions(-) diff --git a/include/graylog_logger/GraylogInterface.hpp b/include/graylog_logger/GraylogInterface.hpp index e3c73b3..64a0a81 100644 --- a/include/graylog_logger/GraylogInterface.hpp +++ b/include/graylog_logger/GraylogInterface.hpp @@ -22,6 +22,7 @@ class GraylogConnection { virtual Status getConnectionStatus() const; virtual bool messageQueueEmpty(); virtual size_t messageQueueSize(); + virtual bool flush(std::chrono::system_clock::duration TimeOut); private: class Impl; @@ -38,6 +39,8 @@ class GraylogInterface : public BaseLogHandler, public GraylogConnection { /// \param[in] TimeOut Amount of time to wait for messages to be transmitted. /// \return Returns true if messages were transmitted before the time out. /// Returns false otherwise. + /// \note It is possible (though unlikely) that not all log messages have + /// been transmitted even if flush() returns true. bool flush(std::chrono::system_clock::duration TimeOut) override; /// \brief Are there any more queued messages? diff --git a/include/graylog_logger/ThreadedExecutor.hpp b/include/graylog_logger/ThreadedExecutor.hpp index b69bb76..53c9c69 100644 --- a/include/graylog_logger/ThreadedExecutor.hpp +++ b/include/graylog_logger/ThreadedExecutor.hpp @@ -27,13 +27,8 @@ class ThreadedExecutor { WorkerThread.join(); } void SendWork(WorkMessage Message) { MessageQueue.enqueue(Message); } - bool WaitForWorkSemaphore(std::chrono::system_clock::duration WaitTime) { - auto WorkDone = std::make_shared>(); - auto WorkDoneFuture = WorkDone->get_future(); - SendWork([WorkDone = std::move(WorkDone)](){ - WorkDone->set_value(true); - }); - return std::future_status::ready == WorkDoneFuture.wait_for(WaitTime); + size_t size_approx() { + return MessageQueue.size_approx(); } private: diff --git a/src/ConsoleInterface.cpp b/src/ConsoleInterface.cpp index f2c499e..4db8bd1 100644 --- a/src/ConsoleInterface.cpp +++ b/src/ConsoleInterface.cpp @@ -41,4 +41,12 @@ bool ConsoleInterface::flush(std::chrono::system_clock::duration TimeOut) { return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); } +bool ConsoleInterface::emptyQueue() { + return Executor.size_approx() == 0; +} + +size_t ConsoleInterface::queueSize() { + return Executor.size_approx(); +} + } // namespace Log diff --git a/src/FileInterface.cpp b/src/FileInterface.cpp index 79254a5..7d83b75 100644 --- a/src/FileInterface.cpp +++ b/src/FileInterface.cpp @@ -43,4 +43,12 @@ bool FileInterface::flush(std::chrono::system_clock::duration TimeOut) { return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); } +bool FileInterface::emptyQueue() { + return Executor.size_approx() == 0; +} + +size_t FileInterface::queueSize() { + return Executor.size_approx(); +} + } // namespace Log diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index fac0efc..22d52dd 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -134,18 +134,26 @@ void GraylogConnection::Impl::trySendMessage() { asio::async_write(Socket, asio::buffer(MessageBuffer), HandlerGlue); return; } - std::string NewMessage; + std::function NewMessageFunc; using namespace std::chrono_literals; - if (LogMessages.wait_dequeue_timed(NewMessage, 10ms)) { + if (LogMessages.wait_dequeue_timed(NewMessageFunc, 10ms)) { + auto NewMessage = NewMessageFunc(); + if (NewMessage.empty()) { + if (!MessageBuffer.empty()) { + asio::async_write(Socket, asio::buffer(MessageBuffer), HandlerGlue); + } else { + Service.post([this]() { this->trySendMessage(); }); + } + return; + } std::copy(NewMessage.begin(), NewMessage.end(), std::back_inserter(MessageBuffer)); MessageBuffer.push_back('\0'); asio::async_write(Socket, asio::buffer(MessageBuffer), HandlerGlue); } else if (!MessageBuffer.empty()) { asio::async_write(Socket, asio::buffer(MessageBuffer), HandlerGlue); - return; } else { - Service.post([this]() { this->waitForMessage(); }); + Service.post([this]() { this->trySendMessage(); }); } } @@ -197,4 +205,14 @@ void GraylogConnection::Impl::setState( ConnectionState = NewState; } +bool GraylogConnection::Impl::flush(std::chrono::system_clock::duration TimeOut) { + auto WorkDone = std::make_shared>(); + auto WorkDoneFuture = WorkDone->get_future(); + LogMessages.enqueue([WorkDone = std::move(WorkDone)]() -> std::string { + WorkDone->set_value(); + return {}; + }); + return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); +} + } // namespace Log diff --git a/src/GraylogConnection.hpp b/src/GraylogConnection.hpp index b510452..b3cd479 100644 --- a/src/GraylogConnection.hpp +++ b/src/GraylogConnection.hpp @@ -18,6 +18,7 @@ #include #include #include +#include namespace Log { @@ -31,8 +32,15 @@ class GraylogConnection::Impl { using Status = Log::Status; Impl(std::string Host, int Port); virtual ~Impl(); - virtual void sendMessage(std::string Msg) { LogMessages.enqueue(Msg); }; + virtual void sendMessage(std::string Msg) { + auto MsgFunc = [=](){ + return Msg;}; + LogMessages.enqueue(MsgFunc);}; Status getConnectionStatus() const; + virtual bool flush(std::chrono::system_clock::duration TimeOut); + virtual size_t queueSize() { + return LogMessages.size_approx(); + } protected: enum class ReconnectDelay { LONG, SHORT }; @@ -50,7 +58,7 @@ class GraylogConnection::Impl { std::string HostPort; std::thread AsioThread; - moodycamel::BlockingConcurrentQueue LogMessages; + moodycamel::BlockingConcurrentQueue> LogMessages; private: const size_t MessageAdditionLimit{3000}; diff --git a/src/GraylogInterface.cpp b/src/GraylogInterface.cpp index 4a0ddfe..624a273 100644 --- a/src/GraylogInterface.cpp +++ b/src/GraylogInterface.cpp @@ -23,10 +23,22 @@ void GraylogConnection::sendMessage(std::string Msg) { Pimpl->sendMessage(Msg); } +bool GraylogConnection::flush(std::chrono::system_clock::duration TimeOut) { + return Pimpl->flush(TimeOut); +} + Status GraylogConnection::getConnectionStatus() const { return Pimpl->getConnectionStatus(); } +bool GraylogConnection::messageQueueEmpty() { + return Pimpl->queueSize() == 0; +} + +size_t GraylogConnection::messageQueueSize() { + return Pimpl->queueSize(); +} + GraylogConnection::~GraylogConnection() {} GraylogInterface::GraylogInterface(const std::string &Host, const int Port, @@ -66,4 +78,16 @@ std::string GraylogInterface::logMsgToJSON(const LogMessage &Message) { return JsonObject.dump(); } +bool GraylogInterface::flush(std::chrono::system_clock::duration TimeOut) { + return GraylogConnection::flush(TimeOut); +} + +bool GraylogInterface::emptyQueue() { + return messageQueueEmpty(); +} + +size_t GraylogInterface::queueSize() { + return messageQueueSize(); +} + } // namespace Log diff --git a/unit_tests/BaseLogHandlerStandIn.hpp b/unit_tests/BaseLogHandlerStandIn.hpp index 5afa866..82b9da3 100644 --- a/unit_tests/BaseLogHandlerStandIn.hpp +++ b/unit_tests/BaseLogHandlerStandIn.hpp @@ -21,4 +21,7 @@ class BaseLogHandlerStandIn : public BaseLogHandler { LogMessage CurrentMessage; using BaseLogHandler::MessageParser; using BaseLogHandler::messageToString; + bool flush(std::chrono::system_clock::duration) override {return true;} + bool emptyQueue() override {return true;} + size_t queueSize() override {return 0;} }; diff --git a/unit_tests/FileInterfaceTest.cpp b/unit_tests/FileInterfaceTest.cpp index d5c4cae..e1e70e1 100644 --- a/unit_tests/FileInterfaceTest.cpp +++ b/unit_tests/FileInterfaceTest.cpp @@ -42,7 +42,6 @@ class FileInterfaceStandIn : public FileInterface { explicit FileInterfaceStandIn(const std::string &fileName) : FileInterface(fileName){}; ~FileInterfaceStandIn() override = default; - using FileInterface::MessageQueue; }; class FileInterfaceTest : public ::testing::Test { diff --git a/unit_tests/QueueLengthTest.cpp b/unit_tests/QueueLengthTest.cpp index 1771369..f991d85 100644 --- a/unit_tests/QueueLengthTest.cpp +++ b/unit_tests/QueueLengthTest.cpp @@ -18,7 +18,7 @@ using namespace Log; class ConsoleInterfaceStandIn : public ConsoleInterface { public: - ConsoleInterfaceStandIn(int queueSize) : ConsoleInterface(queueSize){}; + ConsoleInterfaceStandIn() : ConsoleInterface(){}; }; class FileInterfaceStandIn : public FileInterface { @@ -53,11 +53,10 @@ LogMessage GetLogMsg() { TEST_F(QueueLength, ConsoleInterfaceTest) { std::atomic_int MsgCounter{0}; - int QueueLength = 50; int TestLimit{50}; testing::internal::CaptureStdout(); { - ConsoleInterfaceStandIn CLogger(QueueLength); + ConsoleInterfaceStandIn CLogger; CLogger.setMessageStringCreatorFunction([&MsgCounter](auto Msg) { MsgCounter++; return ""; From 003fd36de2d0b8a2472daeef9c4cf67c023d2096 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Wed, 8 Jan 2020 19:27:49 +0100 Subject: [PATCH 08/43] Fixed unit tests. --- include/graylog_logger/GraylogInterface.hpp | 4 ++-- src/GraylogConnection.cpp | 6 +++--- src/GraylogConnection.hpp | 4 ++-- src/GraylogInterface.cpp | 6 +++--- src/LoggingBase.cpp | 5 +++-- unit_tests/GraylogInterfaceTest.cpp | 6 +++--- unit_tests/LoggingBaseTest.cpp | 19 +++++++++++++++++++ 7 files changed, 35 insertions(+), 15 deletions(-) diff --git a/include/graylog_logger/GraylogInterface.hpp b/include/graylog_logger/GraylogInterface.hpp index 64a0a81..257ab08 100644 --- a/include/graylog_logger/GraylogInterface.hpp +++ b/include/graylog_logger/GraylogInterface.hpp @@ -16,7 +16,7 @@ namespace Log { class GraylogConnection { public: using Status = Log::Status; - GraylogConnection(std::string Host, int Port); + GraylogConnection(std::string Host, int Port, size_t MaxQueueSize); virtual ~GraylogConnection(); virtual void sendMessage(std::string Msg); virtual Status getConnectionStatus() const; @@ -31,7 +31,7 @@ class GraylogConnection { class GraylogInterface : public BaseLogHandler, public GraylogConnection { public: GraylogInterface(const std::string &Host, int Port, - size_t MaxQueueLength = 100); + size_t MaxQueueLength = 1000); ~GraylogInterface() override = default; void addMessage(const LogMessage &Message) override; /// \brief Waits for all messages created before the call to flush to be diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index 22d52dd..bc28b03 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -55,10 +55,10 @@ void GraylogConnection::Impl::tryConnect(QueryResult AllEndpoints) { setState(Status::CONNECT); } -GraylogConnection::Impl::Impl(std::string Host, int Port) +GraylogConnection::Impl::Impl(std::string Host, int Port, size_t MaxQueueLength) : HostAddress(std::move(Host)), HostPort(std::to_string(Port)), Service(), Work(std::make_unique(Service)), Socket(Service), - Resolver(Service), ReconnectTimeout(Service, 10s) { + Resolver(Service), ReconnectTimeout(Service, 10s), LogMessages(MaxQueueLength) { doAddressQuery(); AsioThread = std::thread(&GraylogConnection::Impl::threadFunction, this); } @@ -208,7 +208,7 @@ void GraylogConnection::Impl::setState( bool GraylogConnection::Impl::flush(std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - LogMessages.enqueue([WorkDone = std::move(WorkDone)]() -> std::string { + LogMessages.try_enqueue([WorkDone = std::move(WorkDone)]() -> std::string { WorkDone->set_value(); return {}; }); diff --git a/src/GraylogConnection.hpp b/src/GraylogConnection.hpp index b3cd479..ad580ee 100644 --- a/src/GraylogConnection.hpp +++ b/src/GraylogConnection.hpp @@ -30,12 +30,12 @@ struct QueryResult; class GraylogConnection::Impl { public: using Status = Log::Status; - Impl(std::string Host, int Port); + Impl(std::string Host, int Port, size_t MaxQueueLength); virtual ~Impl(); virtual void sendMessage(std::string Msg) { auto MsgFunc = [=](){ return Msg;}; - LogMessages.enqueue(MsgFunc);}; + LogMessages.try_enqueue(MsgFunc);}; Status getConnectionStatus() const; virtual bool flush(std::chrono::system_clock::duration TimeOut); virtual size_t queueSize() { diff --git a/src/GraylogInterface.cpp b/src/GraylogInterface.cpp index 624a273..e8fd4f1 100644 --- a/src/GraylogInterface.cpp +++ b/src/GraylogInterface.cpp @@ -16,8 +16,8 @@ namespace Log { -GraylogConnection::GraylogConnection(std::string Host, int Port) - : Pimpl(std::make_unique(std::move(Host), Port)) {} +GraylogConnection::GraylogConnection(std::string Host, int Port, size_t MaxQueueSize) + : Pimpl(std::make_unique(std::move(Host), Port, MaxQueueSize)) {} void GraylogConnection::sendMessage(std::string Msg) { Pimpl->sendMessage(Msg); @@ -43,7 +43,7 @@ GraylogConnection::~GraylogConnection() {} GraylogInterface::GraylogInterface(const std::string &Host, const int Port, const size_t MaxQueueLength) - : BaseLogHandler(), GraylogConnection(Host, Port) {} + : BaseLogHandler(), GraylogConnection(Host, Port, MaxQueueLength) {} void GraylogInterface::addMessage(const LogMessage &Message) { sendMessage(logMsgToJSON(Message)); diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index 3811eae..2c247f6 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -113,9 +113,10 @@ LoggingBase::~LoggingBase() { // Flushing here? } -void LoggingBase::addLogHandler(const LogHandler_P& Handler) { //Is this correct? (Use after return?) +void LoggingBase::addLogHandler(const LogHandler_P& Handler) { + LogHandler_P TempHandler{Handler}; Executor.SendWork([=]() { - Handlers.push_back(Handler); + Handlers.push_back(TempHandler); }); } diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index 3a7fdd7..09ccd03 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -38,7 +38,7 @@ class GraylogInterfaceStandIn : public GraylogInterface { class GraylogConnectionStandIn : public GraylogConnection { public: GraylogConnectionStandIn(std::string host, int port, int queueLength = 100) - : GraylogConnection(host, port){}; + : GraylogConnection(host, port, queueLength){}; ~GraylogConnectionStandIn(){}; }; @@ -313,7 +313,7 @@ TEST(GraylogInterfaceCom, TestQueueSizeLimit) { for (int i = 0; i < MaxNrOfMessages + 10; ++i) { con.addMessage(testMsg); } - EXPECT_EQ(con.queueSize(), MaxNrOfMessages); + EXPECT_NEAR(con.queueSize(), MaxNrOfMessages, 20); } using std::chrono_literals::operator""ms; @@ -328,7 +328,7 @@ TEST_F(GraylogConnectionCom, DISABLED_MultipleCloseConnectionTest) { logServer->CloseAllConnections(); std::this_thread::sleep_for(20ms); } - std::this_thread::sleep_for(1000ms); + con.flush(10000ms); EXPECT_EQ(logServer->GetNrOfMessages(), NrOfMessages); } } diff --git a/unit_tests/LoggingBaseTest.cpp b/unit_tests/LoggingBaseTest.cpp index 562852c..24f4081 100644 --- a/unit_tests/LoggingBaseTest.cpp +++ b/unit_tests/LoggingBaseTest.cpp @@ -20,6 +20,8 @@ class LoggingBaseStandIn : public LoggingBase { using LoggingBase::BaseMsg; }; +using namespace std::chrono_literals; + TEST(LoggingBase, InitTest) { LoggingBase log; ASSERT_EQ(log.getHandlers().size(), 0); @@ -29,6 +31,7 @@ TEST(LoggingBase, AddHandlerTest) { LoggingBase log; auto standIn = std::make_shared(); log.addLogHandler(standIn); + log.flush(10s); ASSERT_EQ(log.getHandlers().size(), 1); auto handlers = log.getHandlers(); ASSERT_EQ(handlers[0].get(), standIn.get()); @@ -38,8 +41,10 @@ TEST(LoggingBase, ClearHandlersTest) { LoggingBase log; auto standIn = std::make_shared(); log.addLogHandler(standIn); + log.flush(10s); ASSERT_EQ(log.getHandlers().size(), 1); log.removeAllHandlers(); + log.flush(10s); ASSERT_EQ(log.getHandlers().size(), 0); } @@ -48,12 +53,14 @@ TEST(LoggingBase, LogSeveritiesTest) { log.setMinSeverity(Severity::Debug); auto standIn = std::make_shared(); log.addLogHandler(standIn); + log.flush(10s); std::vector testSeverities = { Severity::Alert, Severity::Critical, Severity::Debug, Severity::Emergency, Severity::Error, Severity::Informational, Severity::Notice, Severity::Warning}; for (auto sev : testSeverities) { log.log(sev, ""); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.SeverityLevel, sev); } } @@ -69,11 +76,13 @@ TEST(LoggingBase, LogIntSeveritiesTest) { Severity::Notice, Severity::Warning}; for (auto sev : testSeverities) { log.log(Severity(int(sev)), ""); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.SeverityLevel, sev); } int testIntSev = -7; auto testSev = Severity(testIntSev); log.log(testSev, ""); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.SeverityLevel, Severity(testIntSev)); } @@ -86,6 +95,7 @@ TEST(LoggingBase, LogMessageTest) { for (int i = 0; i < 100; i++) { tmpStr += baseStr; log.log(Severity::Critical, tmpStr); + log.flush(10s); ASSERT_EQ(tmpStr, standIn->CurrentMessage.MessageString); } } @@ -95,6 +105,7 @@ TEST(LoggingBase, SetExtraField) { std::string someKey = "yet_another_key"; double someValue = -13.543462; log.addField(someKey, someValue); + log.flush(10s); ASSERT_EQ(log.BaseMsg.AdditionalFields.size(), 1); ASSERT_EQ(log.BaseMsg.AdditionalFields[0].first, someKey); ASSERT_EQ(log.BaseMsg.AdditionalFields[0].second.FieldType, @@ -107,6 +118,7 @@ TEST(LoggingBase, LogMsgWithoutStaticExtraField) { auto standIn = std::make_shared(); log.addLogHandler(standIn); log.log(Severity::Alert, "Some message"); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields.size(), 0); } @@ -118,6 +130,7 @@ TEST(LoggingBase, LogMsgWithStaticExtraField) { std::int64_t someStaticExtraValue = -42344093; log.addField(someStaticExtraField, someStaticExtraValue); log.log(Severity::Alert, "Some message"); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields.size(), 1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].first, someStaticExtraField); @@ -135,6 +148,7 @@ TEST(LoggingBase, LogMsgWithDynamicExtraField) { std::int64_t someStaticExtraValue = -42344093; log.log(Severity::Alert, "Some message", {someStaticExtraField, someStaticExtraValue}); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields.size(), 1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].first, someStaticExtraField); @@ -153,6 +167,7 @@ TEST(LoggingBase, LogMsgWithTwoDynamicExtraFields) { std::int64_t v1 = -4234324123; std::string v2 = "value2"; log.log(Severity::Alert, "Some message", {{f1, v1}, {f2, v2}}); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields.size(), 2); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].first, f1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].second.FieldType, @@ -173,6 +188,7 @@ TEST(LoggingBase, LogMsgWithTwoDynamicOverlappingExtraFields) { std::int64_t v1 = -4234324123; std::string v2 = "value2"; log.log(Severity::Alert, "Some message", {{f1, v1}, {f1, v2}}); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields.size(), 1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].first, f1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].second.FieldType, @@ -189,6 +205,7 @@ TEST(LoggingBase, LogMsgWithOverlappingStatDynExtraFields) { std::string v2 = "value2"; log.addField(f1, v2); log.log(Severity::Alert, "Some message", {f1, v1}); + log.flush(10s); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields.size(), 1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].first, f1); ASSERT_EQ(standIn->CurrentMessage.AdditionalFields[0].second.FieldType, @@ -201,6 +218,7 @@ TEST(LoggingBase, MachineInfoTest) { auto standIn = std::make_shared(); log.addLogHandler(standIn); log.log(Severity::Critical, "No message"); + log.flush(10s); LogMessage msg = standIn->CurrentMessage; ASSERT_EQ(msg.Host, asio::ip::host_name()) << "Incorrect host name."; std::ostringstream ss; @@ -214,6 +232,7 @@ TEST(LoggingBase, TimestampTest) { auto standIn = std::make_shared(); log.addLogHandler(standIn); log.log(Severity::Critical, "No message"); + log.flush(10s); LogMessage msg = standIn->CurrentMessage; std::chrono::duration time_diff = std::chrono::system_clock::now() - msg.Timestamp; From 223ab777b70823d73222a43360680e5c93ffec59 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Wed, 8 Jan 2020 20:05:23 +0100 Subject: [PATCH 09/43] Better fix of unit test. --- unit_tests/GraylogInterfaceTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index 09ccd03..eb13769 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -307,10 +307,10 @@ TEST(GraylogInterfaceCom, TestQueueSize) { } TEST(GraylogInterfaceCom, TestQueueSizeLimit) { - int MaxNrOfMessages = 100; + int MaxNrOfMessages = 64; GraylogInterface con("localhost", testPort, MaxNrOfMessages); LogMessage testMsg = GetPopulatedLogMsg(); - for (int i = 0; i < MaxNrOfMessages + 10; ++i) { + for (int i = 0; i < MaxNrOfMessages * 4; ++i) { con.addMessage(testMsg); } EXPECT_NEAR(con.queueSize(), MaxNrOfMessages, 20); From 8401a1807eb978a6e1aab90ab4316d8cb289a94d Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Wed, 8 Jan 2020 23:24:20 +0100 Subject: [PATCH 10/43] clang-format --- include/graylog_logger/Log.hpp | 2 +- include/graylog_logger/MinimalApply.h | 9 +++++---- performance_test/DummyLogHandler.cpp | 3 +-- performance_test/DummyLogHandler.h | 7 ++++--- performance_test/PerformanceTest.cpp | 23 +++++++++++++---------- src/ConsoleInterface.cpp | 13 ++++--------- src/FileInterface.cpp | 10 +++------- src/GraylogConnection.cpp | 6 ++++-- src/GraylogInterface.cpp | 22 ++++++++-------------- src/Logger.cpp | 2 +- src/LoggingBase.cpp | 20 ++++++-------------- src/ThreadedExecutor.cpp | 1 - unit_tests/GraylogInterfaceTest.cpp | 2 +- unit_tests/LoggingBaseTest.cpp | 2 +- 14 files changed, 52 insertions(+), 70 deletions(-) diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 20aa9dd..0dc24e7 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -176,7 +176,7 @@ void AddLogHandler(const BaseLogHandler *Handler); /// \brief Remove all log message handlers. /// /// Clears the vector of handlers. If no other shared pointer for a handler -/// existst, the handler is de-allocated. +/// exists, the handler is de-allocated. void RemoveAllHandlers(); /// \brief Get a copy of the vector containing the handlers. diff --git a/include/graylog_logger/MinimalApply.h b/include/graylog_logger/MinimalApply.h index 043e573..99e82fc 100644 --- a/include/graylog_logger/MinimalApply.h +++ b/include/graylog_logger/MinimalApply.h @@ -11,19 +11,20 @@ #include namespace minimal { -template +template constexpr decltype(auto) invoke(F &&f, Args &&... args) { return std::forward(f)(std::forward(args)...); } namespace detail { -template -constexpr decltype(auto) apply_impl(F &&f, Tuple &&t, std::index_sequence) { +template +constexpr decltype(auto) apply_impl(F &&f, Tuple &&t, + std::index_sequence) { return invoke(std::forward(f), std::get(std::forward(t))...); } } // namespace detail -template +template constexpr decltype(auto) apply(F &&f, Tuple &&t) { return detail::apply_impl( std::forward(f), std::forward(t), diff --git a/performance_test/DummyLogHandler.cpp b/performance_test/DummyLogHandler.cpp index e4d4dde..6bd8336 100644 --- a/performance_test/DummyLogHandler.cpp +++ b/performance_test/DummyLogHandler.cpp @@ -1,6 +1,5 @@ #include "DummyLogHandler.h" void DummyLogHandler::addMessage(const Log::LogMessage &Message) { - Executor.SendWork([=]() { - }); + Executor.SendWork([=]() {}); } \ No newline at end of file diff --git a/performance_test/DummyLogHandler.h b/performance_test/DummyLogHandler.h index bb41da0..77d2894 100644 --- a/performance_test/DummyLogHandler.h +++ b/performance_test/DummyLogHandler.h @@ -7,9 +7,10 @@ class DummyLogHandler : public Log::BaseLogHandler { public: DummyLogHandler() = default; void addMessage(const Log::LogMessage &Message) override; - bool emptyQueue() override {return true;} - size_t queueSize() override {return 0;} - bool flush(std::chrono::system_clock::duration) override {return true;} + bool emptyQueue() override { return true; } + size_t queueSize() override { return 0; } + bool flush(std::chrono::system_clock::duration) override { return true; } + private: Log::ThreadedExecutor Executor; }; \ No newline at end of file diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index a214165..8168b0f 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -1,10 +1,10 @@ -#include -#include -#include #include "DummyLogHandler.h" +#include #include +#include +#include -static void BM_LogMessageGenerationOnly(benchmark::State& state) { +static void BM_LogMessageGenerationOnly(benchmark::State &state) { Log::LoggingBase Logger; for (auto _ : state) { Logger.log(Log::Severity::Error, "Some message."); @@ -13,7 +13,7 @@ static void BM_LogMessageGenerationOnly(benchmark::State& state) { } BENCHMARK(BM_LogMessageGenerationOnly); -static void BM_LogMessageGenerationWithDummySink(benchmark::State& state) { +static void BM_LogMessageGenerationWithDummySink(benchmark::State &state) { Log::LoggingBase Logger; auto Handler = std::make_shared(); Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); @@ -24,24 +24,27 @@ static void BM_LogMessageGenerationWithDummySink(benchmark::State& state) { } BENCHMARK(BM_LogMessageGenerationWithDummySink); -static void BM_LogMessageGenerationWithFmtFormatting(benchmark::State& state) { +static void BM_LogMessageGenerationWithFmtFormatting(benchmark::State &state) { Log::LoggingBase Logger; auto Handler = std::make_shared(); Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); for (auto _ : state) { - Logger.log(Log::Severity::Error, fmt::format("Some format example: {} : {} : {}.", 3.14, 2.72, "some_string")); + Logger.log(Log::Severity::Error, + fmt::format("Some format example: {} : {} : {}.", 3.14, 2.72, + "some_string")); } state.SetItemsProcessed(state.iterations()); } BENCHMARK(BM_LogMessageGenerationWithFmtFormatting); -static void BM_LogMessageGenerationWithDeferredFmtFormatting(benchmark::State& state) { +static void +BM_LogMessageGenerationWithDeferredFmtFormatting(benchmark::State &state) { Log::LoggingBase Logger; auto Handler = std::make_shared(); Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); for (auto _ : state) { - Logger.fmt_log(Log::Severity::Error, "Some format example: {} : {} : {}.", 3.14, 2.72, "some_string"); - + Logger.fmt_log(Log::Severity::Error, "Some format example: {} : {} : {}.", + 3.14, 2.72, "some_string"); } state.SetItemsProcessed(state.iterations()); } diff --git a/src/ConsoleInterface.cpp b/src/ConsoleInterface.cpp index 4db8bd1..0f402ae 100644 --- a/src/ConsoleInterface.cpp +++ b/src/ConsoleInterface.cpp @@ -22,8 +22,7 @@ std::string ConsoleStringCreator(const LogMessage &Message) { Message.MessageString; } -ConsoleInterface::ConsoleInterface() - : BaseLogHandler() { +ConsoleInterface::ConsoleInterface() : BaseLogHandler() { BaseLogHandler::setMessageStringCreatorFunction(ConsoleStringCreator); } void ConsoleInterface::addMessage(const LogMessage &Message) { @@ -34,19 +33,15 @@ void ConsoleInterface::addMessage(const LogMessage &Message) { bool ConsoleInterface::flush(std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - Executor.SendWork([=, WorkDone{std::move(WorkDone)}](){ + Executor.SendWork([=, WorkDone{std::move(WorkDone)}]() { std::cout.flush(); WorkDone->set_value(); }); return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); } -bool ConsoleInterface::emptyQueue() { - return Executor.size_approx() == 0; -} +bool ConsoleInterface::emptyQueue() { return Executor.size_approx() == 0; } -size_t ConsoleInterface::queueSize() { - return Executor.size_approx(); -} +size_t ConsoleInterface::queueSize() { return Executor.size_approx(); } } // namespace Log diff --git a/src/FileInterface.cpp b/src/FileInterface.cpp index 7d83b75..4d111dc 100644 --- a/src/FileInterface.cpp +++ b/src/FileInterface.cpp @@ -36,19 +36,15 @@ void FileInterface::addMessage(const LogMessage &Message) { bool FileInterface::flush(std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - Executor.SendWork([=, WorkDone{std::move(WorkDone)}](){ + Executor.SendWork([=, WorkDone{std::move(WorkDone)}]() { FileStream.flush(); WorkDone->set_value(); }); return std::future_status::ready == WorkDoneFuture.wait_for(TimeOut); } -bool FileInterface::emptyQueue() { - return Executor.size_approx() == 0; -} +bool FileInterface::emptyQueue() { return Executor.size_approx() == 0; } -size_t FileInterface::queueSize() { - return Executor.size_approx(); -} +size_t FileInterface::queueSize() { return Executor.size_approx(); } } // namespace Log diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index bc28b03..58545de 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -58,7 +58,8 @@ void GraylogConnection::Impl::tryConnect(QueryResult AllEndpoints) { GraylogConnection::Impl::Impl(std::string Host, int Port, size_t MaxQueueLength) : HostAddress(std::move(Host)), HostPort(std::to_string(Port)), Service(), Work(std::make_unique(Service)), Socket(Service), - Resolver(Service), ReconnectTimeout(Service, 10s), LogMessages(MaxQueueLength) { + Resolver(Service), ReconnectTimeout(Service, 10s), + LogMessages(MaxQueueLength) { doAddressQuery(); AsioThread = std::thread(&GraylogConnection::Impl::threadFunction, this); } @@ -205,7 +206,8 @@ void GraylogConnection::Impl::setState( ConnectionState = NewState; } -bool GraylogConnection::Impl::flush(std::chrono::system_clock::duration TimeOut) { +bool GraylogConnection::Impl::flush( + std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); LogMessages.try_enqueue([WorkDone = std::move(WorkDone)]() -> std::string { diff --git a/src/GraylogInterface.cpp b/src/GraylogInterface.cpp index e8fd4f1..412515a 100644 --- a/src/GraylogInterface.cpp +++ b/src/GraylogInterface.cpp @@ -16,8 +16,10 @@ namespace Log { -GraylogConnection::GraylogConnection(std::string Host, int Port, size_t MaxQueueSize) - : Pimpl(std::make_unique(std::move(Host), Port, MaxQueueSize)) {} +GraylogConnection::GraylogConnection(std::string Host, int Port, + size_t MaxQueueSize) + : Pimpl(std::make_unique(std::move(Host), Port, + MaxQueueSize)) {} void GraylogConnection::sendMessage(std::string Msg) { Pimpl->sendMessage(Msg); @@ -31,13 +33,9 @@ Status GraylogConnection::getConnectionStatus() const { return Pimpl->getConnectionStatus(); } -bool GraylogConnection::messageQueueEmpty() { - return Pimpl->queueSize() == 0; -} +bool GraylogConnection::messageQueueEmpty() { return Pimpl->queueSize() == 0; } -size_t GraylogConnection::messageQueueSize() { - return Pimpl->queueSize(); -} +size_t GraylogConnection::messageQueueSize() { return Pimpl->queueSize(); } GraylogConnection::~GraylogConnection() {} @@ -82,12 +80,8 @@ bool GraylogInterface::flush(std::chrono::system_clock::duration TimeOut) { return GraylogConnection::flush(TimeOut); } -bool GraylogInterface::emptyQueue() { - return messageQueueEmpty(); -} +bool GraylogInterface::emptyQueue() { return messageQueueEmpty(); } -size_t GraylogInterface::queueSize() { - return messageQueueSize(); -} +size_t GraylogInterface::queueSize() { return messageQueueSize(); } } // namespace Log diff --git a/src/Logger.cpp b/src/Logger.cpp index ebead88..3e86713 100644 --- a/src/Logger.cpp +++ b/src/Logger.cpp @@ -24,7 +24,7 @@ Logger::Logger() { Logger::addLogHandler(std::make_shared()); } -void Logger::addLogHandler(const LogHandler_P& Handler) { +void Logger::addLogHandler(const LogHandler_P &Handler) { Executor.SendWork([=]() { if (dynamic_cast(Handler.get()) != nullptr) { bool replaced = false; diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index 2c247f6..be1434e 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -94,7 +94,7 @@ std::string get_process_name() { #endif LoggingBase::LoggingBase() { - Executor.SendWork([=](){ + Executor.SendWork([=]() { const int stringBufferSize = 100; char stringBuffer[stringBufferSize]; int res; @@ -113,27 +113,19 @@ LoggingBase::~LoggingBase() { // Flushing here? } -void LoggingBase::addLogHandler(const LogHandler_P& Handler) { +void LoggingBase::addLogHandler(const LogHandler_P &Handler) { LogHandler_P TempHandler{Handler}; - Executor.SendWork([=]() { - Handlers.push_back(TempHandler); - }); + Executor.SendWork([=]() { Handlers.push_back(TempHandler); }); } void LoggingBase::removeAllHandlers() { - Executor.SendWork([=]() { - Handlers.clear(); - }); + Executor.SendWork([=]() { Handlers.clear(); }); } -std::vector LoggingBase::getHandlers() { - return Handlers; -} +std::vector LoggingBase::getHandlers() { return Handlers; } void LoggingBase::setMinSeverity(Severity Level) { - Executor.SendWork([=]() { - MinSeverity = Level; - }); + Executor.SendWork([=]() { MinSeverity = Level; }); } } // namespace Log diff --git a/src/ThreadedExecutor.cpp b/src/ThreadedExecutor.cpp index 7c70cdd..a27e2ff 100644 --- a/src/ThreadedExecutor.cpp +++ b/src/ThreadedExecutor.cpp @@ -1,2 +1 @@ #include "graylog_logger/ThreadedExecutor.hpp" - diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index eb13769..8ef3a04 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -6,8 +6,8 @@ // Copyright © 2016 European Spallation Source. All rights reserved. // -#include "LogTestServer.hpp" #include "graylog_logger/GraylogInterface.hpp" +#include "LogTestServer.hpp" #include #include #include diff --git a/unit_tests/LoggingBaseTest.cpp b/unit_tests/LoggingBaseTest.cpp index 24f4081..b13bc97 100644 --- a/unit_tests/LoggingBaseTest.cpp +++ b/unit_tests/LoggingBaseTest.cpp @@ -6,9 +6,9 @@ // Copyright © 2016 European Spallation Source. All rights reserved. // +#include "graylog_logger/LoggingBase.hpp" #include "BaseLogHandlerStandIn.hpp" #include "graylog_logger/LogUtil.hpp" -#include "graylog_logger/LoggingBase.hpp" #include #include #include From 75df8094d758e9623978138e53013f6ddf1723bc Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 01:24:01 +0100 Subject: [PATCH 11/43] Updated cmake-code and fixed build issues. --- CMakeLists.txt | 34 ++++++-- README.md | 38 +++++++-- cmake/FindConcurrentQueue.cmake | 9 +- cmake/FindGMock.cmake | 128 +++++++++++++++++++++++++++++ performance_test/CMakeLists.txt | 3 - performance_test/DummyLogHandler.h | 2 +- src/CMakeLists.txt | 29 +++---- src/ThreadedExecutor.cpp | 1 - unit_tests/CMakeLists.txt | 10 ++- 9 files changed, 207 insertions(+), 47 deletions(-) create mode 100644 cmake/FindGMock.cmake delete mode 100644 src/ThreadedExecutor.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d8a9c40..e7f3df4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.7) project("graylog-logger" - VERSION 1.2.0 + VERSION 2.0.0 DESCRIPTION "A simple logging library." LANGUAGES CXX ) @@ -27,8 +27,10 @@ elseif(${CONAN} MATCHES "MANUAL") else() message(FATAL_ERROR "CONAN set to MANUAL but no file named conanbuildinfo.cmake found in build directory") endif() +elseif(${CONAN} MATCHES "DISABLE") + message(STATUS "***Deactivating Conan support.***") else() - message(FATAL_ERROR "Unrecognised option for CONAN, use AUTO or MANUAL") + message(FATAL_ERROR "Unrecognised option for CONAN, use AUTO, MANUAL or DISABLE") endif() if (NOT WIN32) @@ -39,13 +41,29 @@ else() endif(NOT WIN32) find_package(ASIO REQUIRED MODULE) +find_package(nlohmann_json REQUIRED) +find_package(Threads REQUIRED) +find_package(ConcurrentQueue REQUIRED) -add_subdirectory(src) +find_package(GTest) +find_package(GMock) +find_package(fmt 6) +find_package(GoogleBenchmark) -option(BUILD_EVERYTHING "Build UnitTests, Console Logger and Benchmarks" ON) +add_subdirectory(src) +add_subdirectory(console_logger) -IF(BUILD_EVERYTHING) +if(GTest_FOUND AND GMock_FOUND) + message(STATUS "GTest and GMock found, adding unit tests.") add_subdirectory(unit_tests) - add_subdirectory(console_logger) - add_subdirectory(performance_test) -ENDIF() +else() + message(STATUS "Unable to find GTest and/or GMock. Skipping unit tests.") +endif() + +if(GoogleBenchmark_FOUND AND fmt_FOUND) + message(STATUS "google-benchmark and fmtlib found, adding performance tests.") + add_subdirectory(performance_test) +else() + message(STATUS "Unable to find google-benchmark and/or fmtlib. Skipping performance tests.") +endif() + diff --git a/README.md b/README.md index 959be54..9ef6a1e 100644 --- a/README.md +++ b/README.md @@ -2,13 +2,14 @@ # Graylog logger -This is a simple logging library which can be used to send log messages to a Graylog server. This is done by creating messages in the [GELF](http://docs.graylog.org/en/2.1/pages/gelf.html) format and sending them to a Graylog server via TCP. For testing purposes a [Vagrant](https://www.vagrantup.com/) machine running Graylog can be used. A simple Vagrantfile for creating this set-up can be [found here](https://github.com/ess-dmsc/graylog-machine). The argument for creating yet another logging library instead of writing a plugin/sink/handler for an already existing one is that a relatively light weight solution was desired. The library has functionality for writing log messages to console and file as well and by default the library will only write log messages to console. +This is a simple logging library which can be used to send log messages to a Graylog server. This is done by creating messages in the [GELF](http://docs.graylog.org/en/2.1/pages/gelf.html) format and sending them to a Graylog server via TCP. For testing purposes a [Vagrant](https://www.vagrantup.com/) machine running Graylog can be used. A simple Vagrantfile for creating this set-up can be [found here](https://github.com/ess-dmsc/graylog-machine). The argument for creating yet another logging library instead of writing a plugin/sink/handler for an already existing one is that a relatively light weight solution was desired. The library has functionality for writing log messages to console and file as well. By default the library will only write log messages to console. -The repository is split into three parts: +The repository is split into four parts: * The logging library. * Unit tests of the logging library which are completely self contained (i.e. does not require a Graylog server). * A simple console application which uses the logging library. +* Some benchmarking code which is used for profiling and optimising the code as well as test for performance regression. A sister project to this library is the Python logging handler [GraylogHandler](https://github.com/ess-dmsc/graylog-handler). @@ -21,11 +22,17 @@ These instructions will get you a copy of the project up and running on your loc ### Prerequisites -The logging library depends on several external libraries: +The logging library has the following (requried) external dependencies: -* [GTest](https://github.com/google/googletest) (For testing.) -* [JSONForModernCPP](https://github.com/nlohmann/json) -* [ASIO](http://think-async.com) (For networking.) +* [moodycamel::ConcurrentQueue](https://github.com/cameron314/concurrentqueue) For message passing to and within the logging library. +* [JSONForModernCPP](https://github.com/nlohmann/json) For generating graylog GELF messages. +* [ASIO](http://think-async.com) For networking. + +This library can also make use of the following (optional) dependencies: + +* [libfmt](https://github.com/fmtlib/fmt) For doing fast (multi-threaded) string formatting. +* [GTest](https://github.com/google/googletest) For unit testing. +* [google-benchmark](https://github.com/google/benchmark) For benchmarking. You will also need CMake (version ≥ 3.9) to build the project. The project makes use of library and language features provided by C++14. It might be possible to link to the library using compilers that only supports C++11 though this has not been tested. @@ -33,6 +40,12 @@ Due to the use of ASIO, the library should compile on most \*nix systems and Win ### Installing +There are two methods for building this library. They are described below. + +#### Using conan +When using conan to provide the dependencies, all the optional and required dependencies are provided automatically. Follow the instructions below. + +First, add the required conan repositories: ``` conan remote add community https://api.bintray.com/conan/conan-community/conan @@ -52,6 +65,19 @@ cmake .. make install ``` +#### System installed dependencies +If using conan is not an option, it is possible to build the library using system installed dependencies. This requires a bit more work though and might not be as reliable. + +Run the following commands: + +``` +git clone https://github.com/ess-dmsc/graylog-logger.git +cd graylog-logger +mkdir build +cd build +cmake .. -DCONAN=DISABLE -DCMAKE_PREFIX_PATH=/path/to/dir/containing/the/concurrentqueue/directory/ +make install +``` #### Documentation The code has some documentation. To generate it, run _doxygen_ in the root of the repository i.e.: diff --git a/cmake/FindConcurrentQueue.cmake b/cmake/FindConcurrentQueue.cmake index 86cc5a3..eaf50ab 100644 --- a/cmake/FindConcurrentQueue.cmake +++ b/cmake/FindConcurrentQueue.cmake @@ -1,21 +1,14 @@ -find_path(ConcurrentQueue_ROOT_DIR - NAMES include/concurrentqueue/concurrentqueue.h - PATHS /usr/local /opt/local/ -) - find_path(ConcurrentQueue_INCLUDE_DIR NAMES concurrentqueue/concurrentqueue.h - HINTS ${ConcurrentQueue_ROOT_DIR}/include + PATHS /usr/local/include/ /opt/local/include/ ) include(FindPackageHandleStandardArgs) find_package_handle_standard_args(ConcurrentQueue FOUND_VAR ConcurrentQueue_FOUND REQUIRED_VARS - ConcurrentQueue_ROOT_DIR ConcurrentQueue_INCLUDE_DIR ) mark_as_advanced( - ConcurrentQueue_ROOT_DIR ConcurrentQueue_INCLUDE_DIR ) diff --git a/cmake/FindGMock.cmake b/cmake/FindGMock.cmake new file mode 100644 index 0000000..2f5baec --- /dev/null +++ b/cmake/FindGMock.cmake @@ -0,0 +1,128 @@ +# Locate the Google C++ Mocking Framework. +# (This file is almost an identical copy of the original FindGTest.cmake file, +# feel free to use it as it is or modify it for your own needs.) +# +# +# Defines the following variables: +# +# GMOCK_FOUND - Found the Google Testing framework +# GMOCK_INCLUDE_DIRS - Include directories +# +# Also defines the library variables below as normal +# variables. These contain debug/optimized keywords when +# a debugging library is found. +# +# GMOCK_BOTH_LIBRARIES - Both libgmock & libgmock-main +# GMOCK_LIBRARIES - libgmock +# GMOCK_MAIN_LIBRARIES - libgmock-main +# +# Accepts the following variables as input: +# +# GMOCK_ROOT - (as a CMake or environment variable) +# The root directory of the gmock install prefix +# +# GMOCK_MSVC_SEARCH - If compiling with MSVC, this variable can be set to +# "MD" or "MT" to enable searching a gmock build tree +# (defaults: "MD") +# +#----------------------- +# Example Usage: +# +# find_package(GMock REQUIRED) +# include_directories(${GMOCK_INCLUDE_DIRS}) +# +# add_executable(foo foo.cc) +# target_link_libraries(foo ${GMOCK_BOTH_LIBRARIES}) +# +#============================================================================= +# This file is released under the MIT licence: +# +# Copyright (c) 2011 Matej Svec +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to +# deal in the Software without restriction, including without limitation the +# rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +# sell copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +#============================================================================= + + +function(_gmock_append_debugs _endvar _library) + if(${_library} AND ${_library}_DEBUG) + set(_output optimized ${${_library}} debug ${${_library}_DEBUG}) + else() + set(_output ${${_library}}) + endif() + set(${_endvar} ${_output} PARENT_SCOPE) +endfunction() + +function(_gmock_find_library _name) + find_library(${_name} + NAMES ${ARGN} + HINTS + $ENV{GMOCK_ROOT} + ${GMOCK_ROOT} + PATH_SUFFIXES ${_gmock_libpath_suffixes} + ) + mark_as_advanced(${_name}) +endfunction() + + +if(NOT DEFINED GMOCK_MSVC_SEARCH) + set(GMOCK_MSVC_SEARCH MD) +endif() + +set(_gmock_libpath_suffixes lib) +if(MSVC) + if(GMOCK_MSVC_SEARCH STREQUAL "MD") + list(APPEND _gmock_libpath_suffixes + msvc/gmock-md/Debug + msvc/gmock-md/Release) + elseif(GMOCK_MSVC_SEARCH STREQUAL "MT") + list(APPEND _gmock_libpath_suffixes + msvc/gmock/Debug + msvc/gmock/Release) + endif() +endif() + +find_path(GMOCK_INCLUDE_DIR gmock/gmock.h + HINTS + $ENV{GMOCK_ROOT}/include + ${GMOCK_ROOT}/include +) +mark_as_advanced(GMOCK_INCLUDE_DIR) + +if(MSVC AND GMOCK_MSVC_SEARCH STREQUAL "MD") + # The provided /MD project files for Google Mock add -md suffixes to the + # library names. + _gmock_find_library(GMOCK_LIBRARY gmock-md gmock) + _gmock_find_library(GMOCK_LIBRARY_DEBUG gmock-mdd gmockd) + _gmock_find_library(GMOCK_MAIN_LIBRARY gmock_main-md gmock_main) + _gmock_find_library(GMOCK_MAIN_LIBRARY_DEBUG gmock_main-mdd gmock_maind) +else() + _gmock_find_library(GMOCK_LIBRARY gmock) + _gmock_find_library(GMOCK_LIBRARY_DEBUG gmockd) + _gmock_find_library(GMOCK_MAIN_LIBRARY gmock_main) + _gmock_find_library(GMOCK_MAIN_LIBRARY_DEBUG gmock_maind) +endif() + +FIND_PACKAGE_HANDLE_STANDARD_ARGS(GMock DEFAULT_MSG GMOCK_LIBRARY GMOCK_INCLUDE_DIR GMOCK_MAIN_LIBRARY) + +if(GMOCK_FOUND) + set(GMOCK_INCLUDE_DIRS ${GMOCK_INCLUDE_DIR}) + _gmock_append_debugs(GMOCK_LIBRARIES GMOCK_LIBRARY) + _gmock_append_debugs(GMOCK_MAIN_LIBRARIES GMOCK_MAIN_LIBRARY) + set(GMOCK_BOTH_LIBRARIES ${GMOCK_LIBRARIES} ${GMOCK_MAIN_LIBRARIES}) +endif() diff --git a/performance_test/CMakeLists.txt b/performance_test/CMakeLists.txt index 0797fee..3656933 100644 --- a/performance_test/CMakeLists.txt +++ b/performance_test/CMakeLists.txt @@ -1,6 +1,3 @@ -find_package(GoogleBenchmark REQUIRED) -find_package(fmt REQUIRED) - add_executable(performance_test PerformanceTest.cpp DummyLogHandler.cpp DummyLogHandler.h) target_link_libraries(performance_test GraylogLogger::graylog_logger_static fmt::fmt ${GoogleBenchmark_LIB}) diff --git a/performance_test/DummyLogHandler.h b/performance_test/DummyLogHandler.h index 77d2894..2585ad8 100644 --- a/performance_test/DummyLogHandler.h +++ b/performance_test/DummyLogHandler.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include class DummyLogHandler : public Log::BaseLogHandler { diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6bdd1e9..21d017d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,7 +1,16 @@ -find_package(Threads REQUIRED) -find_package(ConcurrentQueue REQUIRED) -find_package(nlohmann_json REQUIRED) -find_package(fmt REQUIRED) +set(common_libs PUBLIC + Threads::Threads + ConcurrentQueue::ConcurrentQueue + ASIO::ASIO + nlohmann_json::nlohmann_json + ) + +if(fmt_FOUND) + message(STATUS "Found fmtlib, adding support for threaded formatting.") + list(APPEND common_libs fmt::fmt) +else() + message(STATUS "Unable to find fmtlib. There will be no support for threaded formatting.") +endif() set(Graylog_SRC ConsoleInterface.cpp @@ -12,7 +21,6 @@ set(Graylog_SRC Logger.cpp LoggingBase.cpp LogUtil.cpp - ThreadedExecutor.cpp ) set(Graylog_INC @@ -26,13 +34,6 @@ set(Graylog_INC ../include/graylog_logger/LogUtil.hpp ../include/graylog_logger/ThreadedExecutor.hpp ../include/graylog_logger/ConnectionStatus.hpp ../include/graylog_logger/MinimalApply.h) - -set(common_libs - PUBLIC - Threads::Threads - ConcurrentQueue::ConcurrentQueue - fmt::fmt - ) get_target_property(JSON_INCLUDE_DIR nlohmann_json::nlohmann_json INTERFACE_INCLUDE_DIRECTORIES) @@ -45,8 +46,6 @@ target_include_directories(graylog_logger $ PRIVATE "." - ${ASIO_INCLUDE_DIR} - ${JSON_INCLUDE_DIR} ) add_library(graylog_logger_static STATIC ${Graylog_SRC} ${Graylog_INC} ${Graylog_private_INC}) @@ -58,8 +57,6 @@ target_include_directories(graylog_logger_static $ PRIVATE "." - ${ASIO_INCLUDE_DIR} - ${JSON_INCLUDE_DIR} ) set_target_properties(graylog_logger PROPERTIES VERSION ${PROJECT_VERSION}) diff --git a/src/ThreadedExecutor.cpp b/src/ThreadedExecutor.cpp deleted file mode 100644 index a27e2ff..0000000 --- a/src/ThreadedExecutor.cpp +++ /dev/null @@ -1 +0,0 @@ -#include "graylog_logger/ThreadedExecutor.hpp" diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index 6a1d32e..d9503ef 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -28,9 +28,11 @@ set(UnitTest_INC add_executable(unit_tests ${UnitTest_SRC} ${UnitTest_INC}) target_link_libraries(unit_tests - GraylogLogger::graylog_logger_static - CONAN_PKG::jsonformoderncpp - CONAN_PKG::gtest - ASIO::ASIO) + GraylogLogger::graylog_logger_static + nlohmann_json::nlohmann_json + GTest::GTest + ${GMOCK_BOTH_LIBRARIES} + ASIO::ASIO + ) add_test(TestAll unit_tests) From 029945f7102d7b20d6d3ced6bda01913f6e33b40 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 01:36:09 +0100 Subject: [PATCH 12/43] Minor changes. --- performance_test/CMakeLists.txt | 2 +- performance_test/PerformanceTest.cpp | 1 - src/ConsoleInterface.cpp | 2 +- src/FileInterface.cpp | 2 +- src/GraylogConnection.cpp | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/performance_test/CMakeLists.txt b/performance_test/CMakeLists.txt index 3656933..ee4b859 100644 --- a/performance_test/CMakeLists.txt +++ b/performance_test/CMakeLists.txt @@ -1,4 +1,4 @@ -add_executable(performance_test PerformanceTest.cpp DummyLogHandler.cpp DummyLogHandler.h) +add_executable(performance_test PerformanceTest.cpp DummyLogHandler.h DummyLogHandler.cpp) target_link_libraries(performance_test GraylogLogger::graylog_logger_static fmt::fmt ${GoogleBenchmark_LIB}) diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index 8168b0f..283db51 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -1,7 +1,6 @@ #include "DummyLogHandler.h" #include #include -#include #include static void BM_LogMessageGenerationOnly(benchmark::State &state) { diff --git a/src/ConsoleInterface.cpp b/src/ConsoleInterface.cpp index 0f402ae..3125ac9 100644 --- a/src/ConsoleInterface.cpp +++ b/src/ConsoleInterface.cpp @@ -33,7 +33,7 @@ void ConsoleInterface::addMessage(const LogMessage &Message) { bool ConsoleInterface::flush(std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - Executor.SendWork([=, WorkDone{std::move(WorkDone)}]() { + Executor.SendWork([ =, WorkDone{std::move(WorkDone)} ]() { std::cout.flush(); WorkDone->set_value(); }); diff --git a/src/FileInterface.cpp b/src/FileInterface.cpp index 4d111dc..a26b9bf 100644 --- a/src/FileInterface.cpp +++ b/src/FileInterface.cpp @@ -36,7 +36,7 @@ void FileInterface::addMessage(const LogMessage &Message) { bool FileInterface::flush(std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - Executor.SendWork([=, WorkDone{std::move(WorkDone)}]() { + Executor.SendWork([ =, WorkDone{std::move(WorkDone)} ]() { FileStream.flush(); WorkDone->set_value(); }); diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index 58545de..03f9cce 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -210,7 +210,7 @@ bool GraylogConnection::Impl::flush( std::chrono::system_clock::duration TimeOut) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - LogMessages.try_enqueue([WorkDone = std::move(WorkDone)]() -> std::string { + LogMessages.try_enqueue([WorkDone = std::move(WorkDone)]()->std::string { WorkDone->set_value(); return {}; }); From d961bbc8518634375fadfa527c3b4a28625b6916 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 01:46:54 +0100 Subject: [PATCH 13/43] Disable clang-format. --- Jenkinsfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 046e1df..7867b28 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -77,7 +77,7 @@ builders = pipeline_builder.createBuilders { container -> } // stage } // if - if (container.key == clangformat_os) { +/* if (container.key == clangformat_os) { pipeline_builder.stage("${container.key}: check formatting") { container.sh """ clang-format -version @@ -85,7 +85,7 @@ builders = pipeline_builder.createBuilders { container -> find . \\\\( -name '*.cpp' -or -name '*.cxx' -or -name '*.h' -or -name '*.hpp' \\\\) \\ -exec clangformatdiff.sh {} + """ - } // stage + }*/// stage pipeline_builder.stage("${container.key}: cppcheck") { def test_output = "cppcheck.txt" From f16fdcc6b9b746fe5ed288431236920c2739ae25 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 01:54:47 +0100 Subject: [PATCH 14/43] again --- Jenkinsfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 7867b28..b8e5109 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -77,15 +77,15 @@ builders = pipeline_builder.createBuilders { container -> } // stage } // if -/* if (container.key == clangformat_os) { - pipeline_builder.stage("${container.key}: check formatting") { + if (container.key == clangformat_os) { +/* pipeline_builder.stage("${container.key}: check formatting") { container.sh """ clang-format -version cd ${pipeline_builder.project} find . \\\\( -name '*.cpp' -or -name '*.cxx' -or -name '*.h' -or -name '*.hpp' \\\\) \\ -exec clangformatdiff.sh {} + """ - }*/// stage + }*/ // stage pipeline_builder.stage("${container.key}: cppcheck") { def test_output = "cppcheck.txt" From 21ee7023a52ff6636984927fbbe0d7ba19a7b4f8 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 12:15:00 +0100 Subject: [PATCH 15/43] [WIP] Better handling of fat. --- CMakeLists.txt | 6 ++++++ include/graylog_logger/LibConfig.hpp.in | 3 +++ include/graylog_logger/LoggingBase.hpp | 5 +++++ src/CMakeLists.txt | 16 ++++++++++++---- 4 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 include/graylog_logger/LibConfig.hpp.in diff --git a/CMakeLists.txt b/CMakeLists.txt index e7f3df4..4684f5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,6 +53,12 @@ find_package(GoogleBenchmark) add_subdirectory(src) add_subdirectory(console_logger) +if(fmt_FOUND) + set(WITH_FMT 1) +endif() + +configure_file(include/graylog_logger/LibConfig.hpp.in include/graylog_logger/LibConfig.hpp) + if(GTest_FOUND AND GMock_FOUND) message(STATUS "GTest and GMock found, adding unit tests.") add_subdirectory(unit_tests) diff --git a/include/graylog_logger/LibConfig.hpp.in b/include/graylog_logger/LibConfig.hpp.in new file mode 100644 index 0000000..174ba55 --- /dev/null +++ b/include/graylog_logger/LibConfig.hpp.in @@ -0,0 +1,3 @@ +#pragma once + +#cmakedefine WITH_FMT diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 7dedeb3..912fd31 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -10,12 +10,15 @@ #pragma once #include "graylog_logger/LogUtil.hpp" +#include "graylog_logger/LibConfig.hpp" #include #include #include "graylog_logger/ThreadedExecutor.hpp" #include +#ifdef WITH_FMT #include #include +#endif #include "graylog_logger/MinimalApply.h" #include #include @@ -59,6 +62,7 @@ class LoggingBase { }); } +#ifdef WITH_FMT template void fmt_log(const Severity Level, std::string Format, Args... args) { auto ThreadId = std::this_thread::get_id(); @@ -83,6 +87,7 @@ class LoggingBase { } }); } +#endif virtual void addLogHandler(const LogHandler_P& Handler); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 21d017d..39eb947 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,6 +12,8 @@ else() message(STATUS "Unable to find fmtlib. There will be no support for threaded formatting.") endif() + + set(Graylog_SRC ConsoleInterface.cpp FileInterface.cpp @@ -33,7 +35,10 @@ set(Graylog_INC ../include/graylog_logger/LoggingBase.hpp ../include/graylog_logger/LogUtil.hpp ../include/graylog_logger/ThreadedExecutor.hpp - ../include/graylog_logger/ConnectionStatus.hpp ../include/graylog_logger/MinimalApply.h) + ../include/graylog_logger/ConnectionStatus.hpp + ../include/graylog_logger/MinimalApply.h + ${CMAKE_BINARY_DIR}/include/graylog_logger/LibConfig.hpp +) get_target_property(JSON_INCLUDE_DIR nlohmann_json::nlohmann_json INTERFACE_INCLUDE_DIRECTORIES) @@ -44,6 +49,7 @@ target_include_directories(graylog_logger PUBLIC $ $ + $ PRIVATE "." ) @@ -55,6 +61,7 @@ target_include_directories(graylog_logger_static PUBLIC $ $ + $ PRIVATE "." ) @@ -71,12 +78,13 @@ include(GNUInstallDirs) set(INSTALL_CONFIGDIR ${CMAKE_INSTALL_LIBDIR}/cmake/GraylogLogger) install(TARGETS graylog_logger graylog_logger_static - EXPORT graylog_logger-targets - LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} - ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + EXPORT graylog_logger-targets + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} ) install(DIRECTORY ../include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) +install(FILES ${CMAKE_BINARY_DIR}/include/graylog_logger/LibConfig.hpp DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/graylog_logger/LibConfig.hpp) install(EXPORT graylog_logger-targets FILE From 0fc978d0a2f016e1abb01aaeb1bf75f8eb89b93f Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 17:24:10 +0100 Subject: [PATCH 16/43] Add FmtMsg. --- include/graylog_logger/Log.hpp | 24 ++++++++++++++++++++++++ include/graylog_logger/Logger.hpp | 1 + include/graylog_logger/LoggingBase.hpp | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 0dc24e7..62bd208 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -12,6 +12,30 @@ #include "graylog_logger/LogUtil.hpp" #include +#include "graylog_logger/LibConfig.hpp" + +#ifdef WITH_FMT +#include "graylog_logger/Logger.hpp" +namespace Log { +template +/// \brief Submit a formatted message to the logging library. +/// +/// The following fields will be added to the message by the function: +/// - Timestamp +/// - Process name +/// - Process id +/// - Thread id +/// - Host name +/// +/// \param[in] Level The severity level of the message. +/// \param[in] Format The (fmtlib) format of the text message. +/// \param[in] args The variables to be inserted into the format string. +void FmtMsg(const Severity Level, const std::string Format, Args... args) { + Logger::Inst().fmt_log(Level, Format, args...); +} +} +#endif + namespace Log { /// \brief Submit a log message to the logging library. diff --git a/include/graylog_logger/Logger.hpp b/include/graylog_logger/Logger.hpp index 6d9e4e6..594edfc 100644 --- a/include/graylog_logger/Logger.hpp +++ b/include/graylog_logger/Logger.hpp @@ -25,6 +25,7 @@ class Logger : private LoggingBase { using LoggingBase::removeAllHandlers; using LoggingBase::setMinSeverity; using LoggingBase::flush; + using LoggingBase::fmt_log; private: Logger(); diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 912fd31..adf2d00 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -22,6 +22,7 @@ #include "graylog_logger/MinimalApply.h" #include #include +#include namespace Log { @@ -127,7 +128,6 @@ class LoggingBase { protected: Severity MinSeverity{Severity::Notice}; - std::mutex BaseMsgMutex; std::vector Handlers; LogMessage BaseMsg; ThreadedExecutor Executor; From a812c84fb19dd22185bab54799cb944d1cceb1eb Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 17:55:31 +0100 Subject: [PATCH 17/43] Build changes. --- Jenkinsfile | 6 +++--- console_logger/CMakeLists.txt | 2 +- performance_test/CMakeLists.txt | 2 +- unit_tests/CMakeLists.txt | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index b8e5109..0a79143 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -58,7 +58,7 @@ builders = pipeline_builder.createBuilders { container -> container.sh """ cd build . ./activate_run.sh - make VERBOSE=1 all > ${container.key}-build.log + make all unit_tests > ${container.key}-build.log """ container.copyFrom("build/${container.key}-build.log", "${container.key}-build.log") archiveArtifacts "${container.key}-build.log" @@ -160,7 +160,7 @@ def get_macos_pipeline() } try { - sh "source activate_run.sh && make all" + sh "source activate_run.sh && make all unit_tests" sh "source activate_run.sh && ./unit_tests/unit_tests" } catch (e) { failure_function(e, 'MacOSX / build+test failed') @@ -201,7 +201,7 @@ def get_win10_pipeline() { stage("win10: Build") { bat """cd _build cmake .. -G \"Visual Studio 15 2017 Win64\" -DCMAKE_BUILD_TYPE=Release -DCONAN=MANUAL -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=TRUE - cmake --build . --config Release + cmake --build . -t all unit_tests --config Release """ } // stage diff --git a/console_logger/CMakeLists.txt b/console_logger/CMakeLists.txt index f6477ea..75ec10b 100644 --- a/console_logger/CMakeLists.txt +++ b/console_logger/CMakeLists.txt @@ -1,4 +1,4 @@ -add_executable(console_logger ConsoleLogger.cpp getopt.h) +add_executable(console_logger EXCLUDE_FROM_ALL ConsoleLogger.cpp getopt.h) target_link_libraries(console_logger GraylogLogger::graylog_logger_static) diff --git a/performance_test/CMakeLists.txt b/performance_test/CMakeLists.txt index ee4b859..4ac421e 100644 --- a/performance_test/CMakeLists.txt +++ b/performance_test/CMakeLists.txt @@ -1,4 +1,4 @@ -add_executable(performance_test PerformanceTest.cpp DummyLogHandler.h DummyLogHandler.cpp) +add_executable(performance_test EXCLUDE_FROM_ALL PerformanceTest.cpp DummyLogHandler.h DummyLogHandler.cpp) target_link_libraries(performance_test GraylogLogger::graylog_logger_static fmt::fmt ${GoogleBenchmark_LIB}) diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index d9503ef..fb98038 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -25,7 +25,7 @@ set(UnitTest_INC LogTestServer.hpp ) -add_executable(unit_tests ${UnitTest_SRC} ${UnitTest_INC}) +add_executable(unit_tests EXCLUDE_FROM_ALL ${UnitTest_SRC} ${UnitTest_INC}) target_link_libraries(unit_tests GraylogLogger::graylog_logger_static From cc8115cb829c86c4a20ef9f780d292c31622effb Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 17:59:18 +0100 Subject: [PATCH 18/43] Windows fix. --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 0a79143..30e63ec 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -201,7 +201,7 @@ def get_win10_pipeline() { stage("win10: Build") { bat """cd _build cmake .. -G \"Visual Studio 15 2017 Win64\" -DCMAKE_BUILD_TYPE=Release -DCONAN=MANUAL -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=TRUE - cmake --build . -t all unit_tests --config Release + cmake --build . --target all unit_tests --config Release """ } // stage From 8bb4772e76e30374a914edfa1e0801a4cad16dbe Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 18:02:40 +0100 Subject: [PATCH 19/43] Another windows fix. --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 30e63ec..2fccacb 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -201,7 +201,7 @@ def get_win10_pipeline() { stage("win10: Build") { bat """cd _build cmake .. -G \"Visual Studio 15 2017 Win64\" -DCMAKE_BUILD_TYPE=Release -DCONAN=MANUAL -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=TRUE - cmake --build . --target all unit_tests --config Release + cmake --build . --target all --target unit_tests --config Release """ } // stage From fa2d11307223eb00212f10d4e0e6cfded584f7c8 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Thu, 9 Jan 2020 18:04:57 +0100 Subject: [PATCH 20/43] winfix --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 2fccacb..dfb6b05 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -201,7 +201,7 @@ def get_win10_pipeline() { stage("win10: Build") { bat """cd _build cmake .. -G \"Visual Studio 15 2017 Win64\" -DCMAKE_BUILD_TYPE=Release -DCONAN=MANUAL -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=TRUE - cmake --build . --target all --target unit_tests --config Release + cmake --build . --target unit_tests --config Release """ } // stage From 8b721f10288141a244c5d90b4df5390f3b889e19 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Fri, 10 Jan 2020 14:54:49 +0100 Subject: [PATCH 21/43] =?UTF-8?q?[WIP]=C2=A0Mostly=20documentation=20impro?= =?UTF-8?q?vements.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Doxyfile | 2 +- README.md | 9 ++-- TODO.md | 3 -- documentation/changes.md | 41 +++++++++++++++++++ include/graylog_logger/Logger.hpp | 2 + include/graylog_logger/LoggingBase.hpp | 2 +- .../{MinimalApply.h => MinimalApply.hpp} | 0 performance_test/DummyLogHandler.cpp | 12 ++++++ performance_test/DummyLogHandler.h | 10 +++++ performance_test/PerformanceTest.cpp | 10 +++++ src/CMakeLists.txt | 2 +- 11 files changed, 84 insertions(+), 9 deletions(-) rename include/graylog_logger/{MinimalApply.h => MinimalApply.hpp} (100%) diff --git a/Doxyfile b/Doxyfile index daee976..706642b 100644 --- a/Doxyfile +++ b/Doxyfile @@ -2080,7 +2080,7 @@ INCLUDE_FILE_PATTERNS = # recursively expanded use the := operator instead of the = operator. # This tag requires that the tag ENABLE_PREPROCESSING is set to YES. -PREDEFINED = +PREDEFINED = WITH_FMT # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this # tag can be used to specify a list of macro names that should be expanded. The diff --git a/README.md b/README.md index 9ef6a1e..f1f81f2 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ The repository is split into four parts: A sister project to this library is the Python logging handler [GraylogHandler](https://github.com/ess-dmsc/graylog-handler). -- [Further documentation](documentation/README.md) +- Further documentation can be [found here.](documentation/README.md) ## Getting Started @@ -99,7 +99,8 @@ Examples illustrating how the library can be used can be found in the [examples. * [Conan](https://conan.io/) - Package manager for C++ ## Contributing -TBD + +Get in contact by [creating an issue](https://github.com/ess-dmsc/graylog-logger/issues). I will be happy to assist you making the changes that you want. ## Versioning @@ -110,8 +111,10 @@ We use [SemVer](http://semver.org/) for versioning. For the versions available, ## Authors * **Jonas Nilsson** - *Initial work* - [SkyToGround](https://github.com/SkyToGround) +* **Afonos Mukai** - Provided much assistance in setting up the continous integration system. +* **Matthew Jones** - Improved the CMake code and suggested improvements to the interface. -See also the list of [contributors](https://github.com/ess-dmsc/graylog-logger/graphs/contributors) who participated in this project. +See also the list of [contributors](https://github.com/ess-dmsc/graylog-logger/graphs/contributors) to this project. ## License diff --git a/TODO.md b/TODO.md index 05e3708..378766b 100644 --- a/TODO.md +++ b/TODO.md @@ -1,10 +1,7 @@ # To do The items in the following list is in no particular order. Suggestions and/or patches are welcome. -* Improved threading model -* Performance testing and improvements * Version number from git tag * Log file rotation * UDP Messages -* Integrate the fmt library? * Add example logging macros diff --git a/documentation/changes.md b/documentation/changes.md index 8c3f7ba..b28727e 100644 --- a/documentation/changes.md +++ b/documentation/changes.md @@ -1,8 +1,49 @@ ## Changes +### Version 2.0.0 +* Added performance tests. +* Replaced the home-brewed concurrent queue with a *much* faster open source one. +* Made the logger run in a separate thread. +* Added flushing functionality, see documentation for details in how it works. +* Added support for the fmtlib (for formatting) if present. *Note:* The formatting is done in a separate thread which should in most cases be faster than doing it in the thread that calls the logging library. +* Improved/fixed the README-file. +* Made it easier/possible to build the library without using Conan for providing dependencies. +* Added and fixed examples. +* Fixed the non-working `emptyQueue()` and `queueSize()` implementations. +* Fixed/improved code documentation. +* De-activated the use of clang-format in our continuous integration system. It will be re-activated when we have updated clang-format on our build nodes to a newer version. +* Removed the message queue size parameter from log message handlers as it was deemed unnecessary. +* Fixed and added unit tests. + ### Version 1.2.0 * Made the library compile with MSVC on Windows. +### Version 1.1.8 +* Fixed a bug that prevented the library from being included as a conan package in other projects. +* Moved some of the code from externally facing header files to internal header files. This reduces the number of includes. +* Other minor CMake and code fixes. + +### Version 1.1.7 +* Updated the JSON for Modern C++ from 3.1.0 to 3.6.1. + +### Version 1.1.6 +* ASIO has been updated from 1.12.0 to 1.13.0. + +### Version 1.1.5 +* Improved CMake/Conan interaction. + +### Version 11.4 +* Build system fixes. + +### Version 1.1.3 +* Fixed serious issue that would cause the library to repeatedly cause new connections to spawn. +* Made minor changes to the documentation. +* Minor other changes. + +### Version 1.1.2 +* Some indication of the result of opening a log file was required and that has now been added. +* The threading model for the ConsoleLogger and FileLogger handlers has been updated and is now much improved. + ### Version 1.1.1 * Made the static version of the library compile with position independent code (`-fPIC`). diff --git a/include/graylog_logger/Logger.hpp b/include/graylog_logger/Logger.hpp index 594edfc..af4b4e8 100644 --- a/include/graylog_logger/Logger.hpp +++ b/include/graylog_logger/Logger.hpp @@ -25,7 +25,9 @@ class Logger : private LoggingBase { using LoggingBase::removeAllHandlers; using LoggingBase::setMinSeverity; using LoggingBase::flush; +#ifdef WITH_FMT using LoggingBase::fmt_log; +#endif private: Logger(); diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index adf2d00..0028f96 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -19,7 +19,7 @@ #include #include #endif -#include "graylog_logger/MinimalApply.h" +#include "graylog_logger/MinimalApply.hpp" #include #include #include diff --git a/include/graylog_logger/MinimalApply.h b/include/graylog_logger/MinimalApply.hpp similarity index 100% rename from include/graylog_logger/MinimalApply.h rename to include/graylog_logger/MinimalApply.hpp diff --git a/performance_test/DummyLogHandler.cpp b/performance_test/DummyLogHandler.cpp index 6bd8336..fa6f0fd 100644 --- a/performance_test/DummyLogHandler.cpp +++ b/performance_test/DummyLogHandler.cpp @@ -1,5 +1,17 @@ +/* Copyright (C) 2020 European Spallation Source, ERIC. See LICENSE file */ +//===----------------------------------------------------------------------===// +/// +/// \file +/// +/// \brief A simple dummy log handler. +/// +//===----------------------------------------------------------------------===// + + #include "DummyLogHandler.h" +// This code is here instead of in the header file to prevent the compiler +// from optimising the code away. void DummyLogHandler::addMessage(const Log::LogMessage &Message) { Executor.SendWork([=]() {}); } \ No newline at end of file diff --git a/performance_test/DummyLogHandler.h b/performance_test/DummyLogHandler.h index 2585ad8..2d5e4f6 100644 --- a/performance_test/DummyLogHandler.h +++ b/performance_test/DummyLogHandler.h @@ -1,3 +1,13 @@ +/* Copyright (C) 2020 European Spallation Source, ERIC. See LICENSE file */ +//===----------------------------------------------------------------------===// +/// +/// \file +/// +/// \brief A simple dummy log handler. +/// +//===----------------------------------------------------------------------===// + + #pragma once #include diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index 283db51..a16c00e 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -1,3 +1,13 @@ +/* Copyright (C) 2020 European Spallation Source, ERIC. See LICENSE file */ +//===----------------------------------------------------------------------===// +/// +/// \file +/// +/// \brief Code for running performance-tests on the library. +/// +//===----------------------------------------------------------------------===// + + #include "DummyLogHandler.h" #include #include diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 39eb947..b618fc4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -36,7 +36,7 @@ set(Graylog_INC ../include/graylog_logger/LogUtil.hpp ../include/graylog_logger/ThreadedExecutor.hpp ../include/graylog_logger/ConnectionStatus.hpp - ../include/graylog_logger/MinimalApply.h + ../include/graylog_logger/MinimalApply.hpp ${CMAKE_BINARY_DIR}/include/graylog_logger/LibConfig.hpp ) From 1ba7de86a4da9568f22fedf7a1ce1794e45ebab3 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Fri, 10 Jan 2020 15:58:25 +0100 Subject: [PATCH 22/43] Various minor fixes. --- README.md | 2 +- console_logger/ConsoleLogger.cpp | 31 +++-------------- documentation/examples.md | 47 ++++++++++++++++---------- include/graylog_logger/Log.hpp | 11 ++++-- include/graylog_logger/LoggingBase.hpp | 12 ++++--- 5 files changed, 52 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index f1f81f2..0a44450 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ We use [SemVer](http://semver.org/) for versioning. For the versions available, ## Authors * **Jonas Nilsson** - *Initial work* - [SkyToGround](https://github.com/SkyToGround) -* **Afonos Mukai** - Provided much assistance in setting up the continous integration system. +* **Afonso Mukai** - Provided much assistance in setting up the continous integration system. * **Matthew Jones** - Improved the CMake code and suggested improvements to the interface. See also the list of [contributors](https://github.com/ess-dmsc/graylog-logger/graphs/contributors) to this project. diff --git a/console_logger/ConsoleLogger.cpp b/console_logger/ConsoleLogger.cpp index d390204..480bf3a 100644 --- a/console_logger/ConsoleLogger.cpp +++ b/console_logger/ConsoleLogger.cpp @@ -160,33 +160,12 @@ int main(int argc, char **argv) { } else { Log::Msg(Severity(sevLevel), msg); } + + Log::FmtMsg(Severity::Info, "A formatted string containing an int ({:s}), a float ({}) and the string \"{}\".", 42, 3.14, "hello"); - std::vector allInterfaces = Log::GetHandlers(); - std::vector graylogInt; - - for (auto &ptr : allInterfaces) { - if (dynamic_cast(ptr.get()) != nullptr) { - graylogInt.push_back(ptr); - } - } - if (not graylogInt.empty()) { - int milisSleep = 20; - int waitLoops = std::lround((timeout * 1000) / milisSleep); - auto sleepTime = std::chrono::milliseconds(milisSleep); - bool continueLoop = true; - for (int i = 0; i < waitLoops and continueLoop; i++) { - std::this_thread::sleep_for(sleepTime); - continueLoop = false; - for (auto &ptr : graylogInt) { - if (not dynamic_cast(ptr.get())->emptyQueue()) { - continueLoop = true; - } - } - } - if (continueLoop) { - std::cout << "Reached timeout when sending message to Graylog-server." - << std::endl; - } + if (not Log::Flush(std::chrono::milliseconds(static_cast(timeout * 1000)))) { + std::cout << "Reached timeout when trying to send message to Graylog-server." + << std::endl; } return 0; diff --git a/documentation/examples.md b/documentation/examples.md index bdc114f..7066f45 100644 --- a/documentation/examples.md +++ b/documentation/examples.md @@ -1,5 +1,5 @@ # graylog-logger example code -Instructions on how to install the C++ library *graylog-logger* can be found on the [repository page](https://github.com/ess-dmsc/graylog-logger). The intent of this document is to give you examples on how the library can be used and extended. There is currently no code documentation except for the examples given here. For more information on implementation and interfaces see the header and implementation files. Most of the relevant interface information can be found in the header files `graylog_logger/Log.hpp` and `graylog_logger/LogUtil.hpp`. +Instructions on how to install the C++ library *graylog-logger* can be found on the [repository page](https://github.com/ess-dmsc/graylog-logger). The intent of this document is to give you examples on how the library can be used and extended. There is also some code documentation in the header files in the */include/graylog_logger* directory. Of those, *Log.hpp* has the most information. ## Basic example By default, the library will log messages to console. @@ -14,7 +14,7 @@ int main() { return 0; } ``` -The compiled application will (should) print the following to console: +The compiled application should print the following to console: ``` WARNING: Some message. @@ -24,8 +24,6 @@ WARNING: Some message. In order to set-up the library to write messages to file, a file writer has to be added. ```c++ -#include -#include #include #include @@ -34,12 +32,12 @@ using namespace Log; int main() { Log::AddLogHandler(std::make_shared("new_log_file.log")); Log::Msg(Severity::Error, "This is an error."); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Log::Flush(); return 0; } ``` -The `sleep_for()` function is added in order to give the file writing thread time to actually write the message before the application exits. The resulting file output should be similar to the following: +The call to `Log::Flush()` will ensure that the message is written to file before the application exits. The resulting file output should be similar to the following: ``` 2017-01-02 18:29:20 (CI0011840) ERROR: This is an error. @@ -49,15 +47,13 @@ The `sleep_for()` function is added in order to give the file writing thread tim To use the library for its original purpose, a Graylog server interface has to be added. This can be done as follows: ```c++ -#include -#include #include #include int main() { Log::AddLogHandler(new Log::GraylogInterface("somehost.com", 12201)); Log::Msg(Log::Severity::Error, "This message will be sent to a Graylog server."); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Log::Flush(); return 0; } ``` @@ -68,8 +64,6 @@ As the default file handler has not been removed this will send a message to con In order to prevent the logger from writing messages to (e.g.) console but still write to file (or Graylog server), existing log handlers must be removed using the `Log::RemoveAllHandlers()` function before adding the log handlers you do want to use. ```c++ -#include -#include #include #include #include @@ -81,7 +75,7 @@ int main() { AddLogHandler(new FileInterface("new_log_file.log")); AddLogHandler(new GraylogInterface("somehost.com", 12201)); Msg(Severity::Error, "This is an error."); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Log::Flush(); return 0; } ``` @@ -200,7 +194,6 @@ int main() { Log::AddField("some_key", "some_value"); Log::AddLogHandler(new Log::GraylogInterface("somehost.com", 12201)); Log::Msg(Log::Severity::Error, "This message will be sent to a Graylog servers."); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); return 0; } ``` @@ -214,7 +207,7 @@ Adding fields on a per message basis is done when calling the `Log::Msg()`-funct int main() { Log::AddLogHandler(new Log::GraylogInterface("somehost.com", 12201)); Log::Msg(Log::Severity::Error, "A message.", {"another_key", 3.14}); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Log::Flush(); return 0; } ``` @@ -226,7 +219,7 @@ int main() { using Log::GraylogInterface; Log::AddLogHandler(new GraylogInterface("somehost.com", 12201)); Log::Msg(Severity::Error, "A message.", {{"another_key", 3.14}, {"a_third_key", "some_other_value"}}); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Log::Flush(); return 0; } ``` @@ -239,9 +232,29 @@ int main() { Log::AddField("some_key", "some_value"); Log::AddLogHandler(new Log::GraylogInterface("somehost.com", 12201)); Log::Msg(Severity::Error, "Yet another message", {"some_key", 42}); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Log::Flush(); return 0; } ``` -This last piece of code will send a log message to the Graylog server containing only a single extra field with the key `some_key` and the integer value `42`. \ No newline at end of file +This last piece of code will send a log message to the Graylog server containing only a single extra field with the key `some_key` and the integer value `42`. + +## Creating formatted log messages using *fmtlib* + +If [fmtlib](https://fmt.dev/latest/index.html) is available (minimum requried version is 6.0), graylog-logger will compile with built in support for formatting text strings using this library. The fmtlib formatting syntax can [be found here](https://fmt.dev/latest/syntax.html). + +Use the formatting functionality as follows: + +```c++ +int main() { + Log::FmtMsg(Severity::Info, "A formatted string containing an int ({}), a float ({}) and the string \"{}\".", 42, 3.14, "hello"); + Log::Flush(); + return 0; +} +``` + +``` +Info: A formatted string containing an int (42), a float (3.14) and the string "hello". +``` + +This will output the following message: \ No newline at end of file diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 62bd208..388f212 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -17,7 +17,7 @@ #ifdef WITH_FMT #include "graylog_logger/Logger.hpp" namespace Log { -template + /// \brief Submit a formatted message to the logging library. /// /// The following fields will be added to the message by the function: @@ -26,10 +26,15 @@ template /// - Process id /// - Thread id /// - Host name +/// +/// \note If an exception is encountered when generating the formatted text +/// string, a log message showing the `Format` string as well as the error +/// message will be generated in its place. /// /// \param[in] Level The severity level of the message. /// \param[in] Format The (fmtlib) format of the text message. /// \param[in] args The variables to be inserted into the format string. +template void FmtMsg(const Severity Level, const std::string Format, Args... args) { Logger::Inst().fmt_log(Level, Format, args...); } @@ -138,9 +143,9 @@ void Msg( /// created after the call to Flush() can not be guaranteed to have been /// handled. /// \param[in] TimeOut The amount of time to wait for the queued messages to -/// be flushed. +/// be flushed. Defaults to 500ms. /// \return Returns true if messages were flushed, false otherwise. -bool Flush(std::chrono::system_clock::duration TimeOut); +bool Flush(std::chrono::system_clock::duration TimeOut = std::chrono::milliseconds(500)); /// \brief Add a default field of meta-data to every message. /// diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 0028f96..2787d6f 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -73,13 +73,17 @@ class LoggingBase { return; } LogMessage cMsg(BaseMsg); + cMsg.SeverityLevel = Level; cMsg.Timestamp = std::chrono::system_clock::now(); - auto format_message = [=](const auto&... args) { - return fmt::format(Format, args...); + auto format_message = [=,&cMsg](const auto&... args) { + try { + return fmt::format(Format, args...); + } catch (fmt::format_error &e) { + cMsg.SeverityLevel = Log::Severity::Error; + return fmt::format("graylog-logger internal error. Unable to format the string \"{}\". The error was: \"{}\".", Format, e.what()); + } }; - cMsg.MessageString = minimal::apply(format_message, UsedArguments); - cMsg.SeverityLevel = Level; std::ostringstream ss; ss << ThreadId; cMsg.ThreadId = ss.str(); From 39ca293e48bc3016479bc4b248d1bc9f7fb42f25 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Sat, 11 Jan 2020 00:55:57 +0100 Subject: [PATCH 23/43] clang-tidy and minor fixes. --- documentation/examples.md | 5 +++-- include/graylog_logger/GraylogInterface.hpp | 2 +- src/ConsoleInterface.cpp | 2 +- src/GraylogConnection.cpp | 9 ++------- src/GraylogInterface.cpp | 6 +++--- src/LogUtil.cpp | 9 +++++---- src/LoggingBase.cpp | 14 ++++++-------- unit_tests/BaseLogHandlerStandIn.hpp | 2 +- unit_tests/GraylogInterfaceTest.cpp | 6 +++--- 9 files changed, 25 insertions(+), 30 deletions(-) diff --git a/documentation/examples.md b/documentation/examples.md index 7066f45..3d69890 100644 --- a/documentation/examples.md +++ b/documentation/examples.md @@ -253,8 +253,9 @@ int main() { } ``` +This will output the following message: + ``` Info: A formatted string containing an int (42), a float (3.14) and the string "hello". ``` - -This will output the following message: \ No newline at end of file +There is currently no support for using extra fields together with fmt-formatted strings. diff --git a/include/graylog_logger/GraylogInterface.hpp b/include/graylog_logger/GraylogInterface.hpp index 257ab08..f96441d 100644 --- a/include/graylog_logger/GraylogInterface.hpp +++ b/include/graylog_logger/GraylogInterface.hpp @@ -55,7 +55,7 @@ class GraylogInterface : public BaseLogHandler, public GraylogConnection { size_t queueSize() override; protected: - std::string logMsgToJSON(const LogMessage &Message); + static std::string logMsgToJSON(const LogMessage &Message); }; } // namespace Log diff --git a/src/ConsoleInterface.cpp b/src/ConsoleInterface.cpp index 3125ac9..46a7683 100644 --- a/src/ConsoleInterface.cpp +++ b/src/ConsoleInterface.cpp @@ -22,7 +22,7 @@ std::string ConsoleStringCreator(const LogMessage &Message) { Message.MessageString; } -ConsoleInterface::ConsoleInterface() : BaseLogHandler() { +ConsoleInterface::ConsoleInterface() { BaseLogHandler::setMessageStringCreatorFunction(ConsoleStringCreator); } void ConsoleInterface::addMessage(const LogMessage &Message) { diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index 03f9cce..886f21f 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -13,10 +13,6 @@ #include #include -namespace { -#define UNUSED_ARG(x) (void)x; -} // namespace - namespace Log { using std::chrono_literals::operator""ms; @@ -96,7 +92,7 @@ void GraylogConnection::Impl::connectHandler(const asio::error_code &Error, } void GraylogConnection::Impl::reConnect(ReconnectDelay Delay) { - auto HandlerGlue = [this](auto &Err) { this->doAddressQuery(); }; + auto HandlerGlue = [this](auto &/* Err */) { this->doAddressQuery(); }; switch (Delay) { case ReconnectDelay::SHORT: ReconnectTimeout.expires_after(100ms); @@ -111,8 +107,7 @@ void GraylogConnection::Impl::reConnect(ReconnectDelay Delay) { } void GraylogConnection::Impl::receiveHandler(const asio::error_code &Error, - std::size_t BytesReceived) { - UNUSED_ARG(BytesReceived); + std::size_t /* BytesReceived */) { if (Error) { Socket.close(); reConnect(ReconnectDelay::SHORT); diff --git a/src/GraylogInterface.cpp b/src/GraylogInterface.cpp index 412515a..c8f184a 100644 --- a/src/GraylogInterface.cpp +++ b/src/GraylogInterface.cpp @@ -22,7 +22,7 @@ GraylogConnection::GraylogConnection(std::string Host, int Port, MaxQueueSize)) {} void GraylogConnection::sendMessage(std::string Msg) { - Pimpl->sendMessage(Msg); + Pimpl->sendMessage(std::move(Msg)); } bool GraylogConnection::flush(std::chrono::system_clock::duration TimeOut) { @@ -37,11 +37,11 @@ bool GraylogConnection::messageQueueEmpty() { return Pimpl->queueSize() == 0; } size_t GraylogConnection::messageQueueSize() { return Pimpl->queueSize(); } -GraylogConnection::~GraylogConnection() {} +GraylogConnection::~GraylogConnection() = default; GraylogInterface::GraylogInterface(const std::string &Host, const int Port, const size_t MaxQueueLength) - : BaseLogHandler(), GraylogConnection(Host, Port, MaxQueueLength) {} + : GraylogConnection(Host, Port, MaxQueueLength) {} void GraylogInterface::addMessage(const LogMessage &Message) { sendMessage(logMsgToJSON(Message)); diff --git a/src/LogUtil.cpp b/src/LogUtil.cpp index 11f2b3a..90e517b 100644 --- a/src/LogUtil.cpp +++ b/src/LogUtil.cpp @@ -25,13 +25,14 @@ std::string BaseLogHandler::messageToString(const LogMessage &Message) { return MessageParser(Message); } std::time_t cTime = std::chrono::system_clock::to_time_t(Message.Timestamp); - char timeBuffer[50]; - size_t bytes = std::strftime(static_cast(timeBuffer), 50, "%F %T", - std::localtime(&cTime)); + const size_t TimeBufferSize{50}; + std::array TimeBuffer{}; + size_t BytesWritten = std::strftime(static_cast(TimeBuffer.data()), TimeBufferSize, "%F %T", + std::localtime(&cTime)); std::array sevToStr = {{"EMERGENCY", "ALERT", "CRITICAL", "ERROR", "WARNING", "Notice", "Info", "Debug"}}; - return std::string(static_cast(timeBuffer), bytes) + + return std::string(static_cast(TimeBuffer.data()), BytesWritten) + std::string(" (") + Message.Host + std::string(") ") + sevToStr.at(int(Message.SeverityLevel)) + std::string(": ") + Message.MessageString; diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index be1434e..bff5adb 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -95,12 +95,12 @@ std::string get_process_name() { LoggingBase::LoggingBase() { Executor.SendWork([=]() { - const int stringBufferSize = 100; - char stringBuffer[stringBufferSize]; + const int StringBufferSize = 100; + std::array StringBuffer{}; int res; - res = gethostname(static_cast(stringBuffer), stringBufferSize); + res = gethostname(static_cast(StringBuffer.data()), StringBufferSize); if (0 == res) { - BaseMsg.Host = std::string(static_cast(stringBuffer)); + BaseMsg.Host = std::string(static_cast(StringBuffer.data())); } BaseMsg.ProcessId = getpid(); BaseMsg.ProcessName = get_process_name(); @@ -109,13 +109,11 @@ LoggingBase::LoggingBase() { } LoggingBase::~LoggingBase() { - removeAllHandlers(); - // Flushing here? + LoggingBase::removeAllHandlers(); } void LoggingBase::addLogHandler(const LogHandler_P &Handler) { - LogHandler_P TempHandler{Handler}; - Executor.SendWork([=]() { Handlers.push_back(TempHandler); }); + Executor.SendWork([=]() { Handlers.push_back(Handler); }); } void LoggingBase::removeAllHandlers() { diff --git a/unit_tests/BaseLogHandlerStandIn.hpp b/unit_tests/BaseLogHandlerStandIn.hpp index 82b9da3..1df8830 100644 --- a/unit_tests/BaseLogHandlerStandIn.hpp +++ b/unit_tests/BaseLogHandlerStandIn.hpp @@ -14,7 +14,7 @@ using namespace Log; class BaseLogHandlerStandIn : public BaseLogHandler { public: - BaseLogHandlerStandIn() : BaseLogHandler(){}; + BaseLogHandlerStandIn() {}; void addMessage(const LogMessage &Message) override { CurrentMessage = Message; }; diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index 8ef3a04..e30a52c 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -266,7 +266,7 @@ TEST(GraylogInterfaceCom, TestAdditionalFieldString) { std::string key = "yet_another_key"; std::string value = "yet another value"; testMsg.addField(key, value); - std::string jsonStr = con.logMsgToJSON(testMsg); + std::string jsonStr = GraylogInterfaceStandIn::logMsgToJSON(testMsg); auto JsonObject = nlohmann::json::parse(jsonStr); std::string tempStr; EXPECT_NO_THROW(tempStr = JsonObject["_" + key]); @@ -279,7 +279,7 @@ TEST(GraylogInterfaceCom, TestAdditionalFieldInt) { std::string key = "yet_another_key"; std::int64_t value = -12431454; testMsg.addField(key, value); - std::string jsonStr = con.logMsgToJSON(testMsg); + std::string jsonStr = GraylogInterfaceStandIn::logMsgToJSON(testMsg); auto JsonObject = nlohmann::json::parse(jsonStr); std::int64_t tempVal = 0; EXPECT_NO_THROW(tempVal = JsonObject["_" + key]); @@ -292,7 +292,7 @@ TEST(GraylogInterfaceCom, TestAdditionalFieldDouble) { std::string key = "yet_another_key"; double value = 3.1415926535897932384626433832795028841; testMsg.addField(key, value); - std::string jsonStr = con.logMsgToJSON(testMsg); + std::string jsonStr = GraylogInterfaceStandIn::logMsgToJSON(testMsg); auto JsonObject = nlohmann::json::parse(jsonStr); double tempVal; EXPECT_NO_THROW(tempVal = JsonObject["_" + key]); From 2941740d2b9f5ff7fbf2baf15a0c02f2263a58f8 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Sat, 11 Jan 2020 01:38:31 +0100 Subject: [PATCH 24/43] Multiple fixes. --- console_logger/ConsoleLogger.cpp | 15 +++++--- include/graylog_logger/Log.hpp | 14 +++---- include/graylog_logger/LogUtil.hpp | 2 - include/graylog_logger/Logger.hpp | 4 +- include/graylog_logger/LoggingBase.hpp | 42 ++++++++++----------- include/graylog_logger/ThreadedExecutor.hpp | 8 ++-- performance_test/DummyLogHandler.cpp | 1 - performance_test/PerformanceTest.cpp | 1 - src/GraylogConnection.cpp | 2 +- src/GraylogConnection.hpp | 17 ++++----- src/LogUtil.cpp | 5 ++- src/LoggingBase.cpp | 7 ++-- unit_tests/BaseLogHandlerStandIn.hpp | 8 ++-- unit_tests/LoggingBaseTest.cpp | 26 +++++++++++++ 14 files changed, 88 insertions(+), 64 deletions(-) diff --git a/console_logger/ConsoleLogger.cpp b/console_logger/ConsoleLogger.cpp index 480bf3a..d10dcaf 100644 --- a/console_logger/ConsoleLogger.cpp +++ b/console_logger/ConsoleLogger.cpp @@ -160,12 +160,17 @@ int main(int argc, char **argv) { } else { Log::Msg(Severity(sevLevel), msg); } - - Log::FmtMsg(Severity::Info, "A formatted string containing an int ({:s}), a float ({}) and the string \"{}\".", 42, 3.14, "hello"); - if (not Log::Flush(std::chrono::milliseconds(static_cast(timeout * 1000)))) { - std::cout << "Reached timeout when trying to send message to Graylog-server." - << std::endl; + Log::FmtMsg(Severity::Info, + "A formatted string containing an int ({:s}), a float ({}) and " + "the string \"{}\".", + 42, 3.14, "hello"); + + if (not Log::Flush( + std::chrono::milliseconds(static_cast(timeout * 1000)))) { + std::cout + << "Reached timeout when trying to send message to Graylog-server." + << std::endl; } return 0; diff --git a/include/graylog_logger/Log.hpp b/include/graylog_logger/Log.hpp index 388f212..5818585 100644 --- a/include/graylog_logger/Log.hpp +++ b/include/graylog_logger/Log.hpp @@ -10,9 +10,9 @@ #pragma once +#include "graylog_logger/LibConfig.hpp" #include "graylog_logger/LogUtil.hpp" #include -#include "graylog_logger/LibConfig.hpp" #ifdef WITH_FMT #include "graylog_logger/Logger.hpp" @@ -26,22 +26,21 @@ namespace Log { /// - Process id /// - Thread id /// - Host name -/// +/// /// \note If an exception is encountered when generating the formatted text -/// string, a log message showing the `Format` string as well as the error +/// string, a log message showing the `Format` string as well as the error /// message will be generated in its place. /// /// \param[in] Level The severity level of the message. /// \param[in] Format The (fmtlib) format of the text message. /// \param[in] args The variables to be inserted into the format string. -template +template void FmtMsg(const Severity Level, const std::string Format, Args... args) { Logger::Inst().fmt_log(Level, Format, args...); } -} +} // namespace Log #endif - namespace Log { /// \brief Submit a log message to the logging library. /// @@ -145,7 +144,8 @@ void Msg( /// \param[in] TimeOut The amount of time to wait for the queued messages to /// be flushed. Defaults to 500ms. /// \return Returns true if messages were flushed, false otherwise. -bool Flush(std::chrono::system_clock::duration TimeOut = std::chrono::milliseconds(500)); +bool Flush(std::chrono::system_clock::duration TimeOut = + std::chrono::milliseconds(500)); /// \brief Add a default field of meta-data to every message. /// diff --git a/include/graylog_logger/LogUtil.hpp b/include/graylog_logger/LogUtil.hpp index 85c99a5..f744556 100644 --- a/include/graylog_logger/LogUtil.hpp +++ b/include/graylog_logger/LogUtil.hpp @@ -144,8 +144,6 @@ class BaseLogHandler { void setMessageStringCreatorFunction( std::function ParserFunction); - - protected: /// \brief Can be used to create strings from messages if set. std::function MessageParser{nullptr}; diff --git a/include/graylog_logger/Logger.hpp b/include/graylog_logger/Logger.hpp index af4b4e8..90f867b 100644 --- a/include/graylog_logger/Logger.hpp +++ b/include/graylog_logger/Logger.hpp @@ -18,13 +18,13 @@ class Logger : private LoggingBase { static Logger &Inst(); Logger(const Logger &) = delete; Logger &operator=(const Logger &) = delete; - virtual void addLogHandler(const LogHandler_P& Handler) override; + virtual void addLogHandler(const LogHandler_P &Handler) override; using LoggingBase::addField; + using LoggingBase::flush; using LoggingBase::getHandlers; using LoggingBase::log; using LoggingBase::removeAllHandlers; using LoggingBase::setMinSeverity; - using LoggingBase::flush; #ifdef WITH_FMT using LoggingBase::fmt_log; #endif diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 2787d6f..eea5df3 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -9,20 +9,20 @@ #pragma once -#include "graylog_logger/LogUtil.hpp" #include "graylog_logger/LibConfig.hpp" -#include -#include +#include "graylog_logger/LogUtil.hpp" #include "graylog_logger/ThreadedExecutor.hpp" #include +#include +#include #ifdef WITH_FMT #include #include #endif #include "graylog_logger/MinimalApply.hpp" +#include #include #include -#include namespace Log { @@ -37,7 +37,7 @@ class LoggingBase { log(const Severity Level, const std::string &Message, const std::vector> &ExtraFields) { auto ThreadId = std::this_thread::get_id(); - Executor.SendWork([=](){ + Executor.SendWork([=]() { if (int(Level) > int(MinSeverity)) { return; } @@ -58,9 +58,10 @@ class LoggingBase { } virtual void log(const Severity Level, const std::string &Message, const std::pair &ExtraField) { - log(Level, Message, std::vector>{ - ExtraField, - }); + log(Level, Message, + std::vector>{ + ExtraField, + }); } #ifdef WITH_FMT @@ -68,19 +69,21 @@ class LoggingBase { void fmt_log(const Severity Level, std::string Format, Args... args) { auto ThreadId = std::this_thread::get_id(); auto UsedArguments = std::make_tuple(args...); - Executor.SendWork([=](){ + Executor.SendWork([=]() { if (int(Level) > int(MinSeverity)) { return; } LogMessage cMsg(BaseMsg); cMsg.SeverityLevel = Level; cMsg.Timestamp = std::chrono::system_clock::now(); - auto format_message = [=,&cMsg](const auto&... args) { + auto format_message = [=, &cMsg](const auto &... args) { try { return fmt::format(Format, args...); } catch (fmt::format_error &e) { cMsg.SeverityLevel = Log::Severity::Error; - return fmt::format("graylog-logger internal error. Unable to format the string \"{}\". The error was: \"{}\".", Format, e.what()); + return fmt::format("graylog-logger internal error. Unable to format " + "the string \"{}\". The error was: \"{}\".", + Format, e.what()); } }; cMsg.MessageString = minimal::apply(format_message, UsedArguments); @@ -94,13 +97,11 @@ class LoggingBase { } #endif - virtual void addLogHandler(const LogHandler_P& Handler); + virtual void addLogHandler(const LogHandler_P &Handler); template void addField(std::string Key, const valueType &Value) { - Executor.SendWork([=](){ - BaseMsg.addField(Key, Value); - }); + Executor.SendWork([=]() { BaseMsg.addField(Key, Value); }); }; virtual void removeAllHandlers(); virtual void setMinSeverity(Severity Level); @@ -109,15 +110,14 @@ class LoggingBase { virtual bool flush(std::chrono::system_clock::duration TimeOut) { auto FlushCompleted = std::make_shared>(); auto FlushCompletedValue = FlushCompleted->get_future(); - Executor.SendWork([=,FlushCompleted{std::move(FlushCompleted)}](){ + Executor.SendWork([ =, FlushCompleted{std::move(FlushCompleted)} ]() { std::vector> FlushResults; - for (auto& CHandler : Handlers) { - FlushResults.push_back(std::async(std::launch::async, [=](){ - return CHandler->flush(TimeOut);} - )); + for (auto &CHandler : Handlers) { + FlushResults.push_back(std::async( + std::launch::async, [=]() { return CHandler->flush(TimeOut); })); } bool ReturnValue{true}; - for (auto& CFlushResult: FlushResults) { + for (auto &CFlushResult : FlushResults) { CFlushResult.wait(); if (not CFlushResult.get()) { ReturnValue = false; diff --git a/include/graylog_logger/ThreadedExecutor.hpp b/include/graylog_logger/ThreadedExecutor.hpp index 53c9c69..19dd647 100644 --- a/include/graylog_logger/ThreadedExecutor.hpp +++ b/include/graylog_logger/ThreadedExecutor.hpp @@ -10,11 +10,11 @@ #pragma once -#include -#include #include +#include #include #include +#include namespace Log { class ThreadedExecutor { @@ -27,9 +27,7 @@ class ThreadedExecutor { WorkerThread.join(); } void SendWork(WorkMessage Message) { MessageQueue.enqueue(Message); } - size_t size_approx() { - return MessageQueue.size_approx(); - } + size_t size_approx() { return MessageQueue.size_approx(); } private: bool RunThread{true}; diff --git a/performance_test/DummyLogHandler.cpp b/performance_test/DummyLogHandler.cpp index fa6f0fd..8d3e5d9 100644 --- a/performance_test/DummyLogHandler.cpp +++ b/performance_test/DummyLogHandler.cpp @@ -7,7 +7,6 @@ /// //===----------------------------------------------------------------------===// - #include "DummyLogHandler.h" // This code is here instead of in the header file to prevent the compiler diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index a16c00e..2dfb8ba 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -7,7 +7,6 @@ /// //===----------------------------------------------------------------------===// - #include "DummyLogHandler.h" #include #include diff --git a/src/GraylogConnection.cpp b/src/GraylogConnection.cpp index 886f21f..8a1ab69 100644 --- a/src/GraylogConnection.cpp +++ b/src/GraylogConnection.cpp @@ -92,7 +92,7 @@ void GraylogConnection::Impl::connectHandler(const asio::error_code &Error, } void GraylogConnection::Impl::reConnect(ReconnectDelay Delay) { - auto HandlerGlue = [this](auto &/* Err */) { this->doAddressQuery(); }; + auto HandlerGlue = [this](auto & /* Err */) { this->doAddressQuery(); }; switch (Delay) { case ReconnectDelay::SHORT: ReconnectTimeout.expires_after(100ms); diff --git a/src/GraylogConnection.hpp b/src/GraylogConnection.hpp index ad580ee..27b3813 100644 --- a/src/GraylogConnection.hpp +++ b/src/GraylogConnection.hpp @@ -11,14 +11,14 @@ #include "graylog_logger/ConnectionStatus.hpp" #include "graylog_logger/GraylogInterface.hpp" -#include #include #include #include +#include +#include #include #include #include -#include namespace Log { @@ -33,14 +33,12 @@ class GraylogConnection::Impl { Impl(std::string Host, int Port, size_t MaxQueueLength); virtual ~Impl(); virtual void sendMessage(std::string Msg) { - auto MsgFunc = [=](){ - return Msg;}; - LogMessages.try_enqueue(MsgFunc);}; + auto MsgFunc = [=]() { return Msg; }; + LogMessages.try_enqueue(MsgFunc); + }; Status getConnectionStatus() const; virtual bool flush(std::chrono::system_clock::duration TimeOut); - virtual size_t queueSize() { - return LogMessages.size_approx(); - } + virtual size_t queueSize() { return LogMessages.size_approx(); } protected: enum class ReconnectDelay { LONG, SHORT }; @@ -58,7 +56,8 @@ class GraylogConnection::Impl { std::string HostPort; std::thread AsioThread; - moodycamel::BlockingConcurrentQueue> LogMessages; + moodycamel::BlockingConcurrentQueue> + LogMessages; private: const size_t MessageAdditionLimit{3000}; diff --git a/src/LogUtil.cpp b/src/LogUtil.cpp index 90e517b..9abdb0e 100644 --- a/src/LogUtil.cpp +++ b/src/LogUtil.cpp @@ -27,8 +27,9 @@ std::string BaseLogHandler::messageToString(const LogMessage &Message) { std::time_t cTime = std::chrono::system_clock::to_time_t(Message.Timestamp); const size_t TimeBufferSize{50}; std::array TimeBuffer{}; - size_t BytesWritten = std::strftime(static_cast(TimeBuffer.data()), TimeBufferSize, "%F %T", - std::localtime(&cTime)); + size_t BytesWritten = + std::strftime(static_cast(TimeBuffer.data()), TimeBufferSize, + "%F %T", std::localtime(&cTime)); std::array sevToStr = {{"EMERGENCY", "ALERT", "CRITICAL", "ERROR", "WARNING", "Notice", "Info", "Debug"}}; diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index bff5adb..f5880c9 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -98,7 +98,8 @@ LoggingBase::LoggingBase() { const int StringBufferSize = 100; std::array StringBuffer{}; int res; - res = gethostname(static_cast(StringBuffer.data()), StringBufferSize); + res = + gethostname(static_cast(StringBuffer.data()), StringBufferSize); if (0 == res) { BaseMsg.Host = std::string(static_cast(StringBuffer.data())); } @@ -108,9 +109,7 @@ LoggingBase::LoggingBase() { }); } -LoggingBase::~LoggingBase() { - LoggingBase::removeAllHandlers(); -} +LoggingBase::~LoggingBase() { LoggingBase::removeAllHandlers(); } void LoggingBase::addLogHandler(const LogHandler_P &Handler) { Executor.SendWork([=]() { Handlers.push_back(Handler); }); diff --git a/unit_tests/BaseLogHandlerStandIn.hpp b/unit_tests/BaseLogHandlerStandIn.hpp index 1df8830..384fe57 100644 --- a/unit_tests/BaseLogHandlerStandIn.hpp +++ b/unit_tests/BaseLogHandlerStandIn.hpp @@ -14,14 +14,14 @@ using namespace Log; class BaseLogHandlerStandIn : public BaseLogHandler { public: - BaseLogHandlerStandIn() {}; + BaseLogHandlerStandIn(){}; void addMessage(const LogMessage &Message) override { CurrentMessage = Message; }; LogMessage CurrentMessage; using BaseLogHandler::MessageParser; using BaseLogHandler::messageToString; - bool flush(std::chrono::system_clock::duration) override {return true;} - bool emptyQueue() override {return true;} - size_t queueSize() override {return 0;} + bool flush(std::chrono::system_clock::duration) override { return true; } + bool emptyQueue() override { return true; } + size_t queueSize() override { return 0; } }; diff --git a/unit_tests/LoggingBaseTest.cpp b/unit_tests/LoggingBaseTest.cpp index b13bc97..b091564 100644 --- a/unit_tests/LoggingBaseTest.cpp +++ b/unit_tests/LoggingBaseTest.cpp @@ -8,6 +8,7 @@ #include "graylog_logger/LoggingBase.hpp" #include "BaseLogHandlerStandIn.hpp" +#include "graylog_logger/LibConfig.hpp" #include "graylog_logger/LogUtil.hpp" #include #include @@ -238,3 +239,28 @@ TEST(LoggingBase, TimestampTest) { std::chrono::system_clock::now() - msg.Timestamp; ASSERT_NEAR(time_diff.count(), 0.0, 0.1) << "Time stamp is incorrect."; } + +#ifdef WITH_FMT + +TEST(LoggingBase, FmtLogMessage) { + LoggingBase log; + auto standIn = std::make_shared(); + log.addLogHandler(standIn); + log.fmt_log(Severity::Critical, "A test message {} - {}", 42, "hello"); + log.flush(10s); + LogMessage msg = standIn->CurrentMessage; + ASSERT_EQ(msg.MessageString, "A test message 42 - hello"); +} + +TEST(LoggingBase, FmtLogMessageException) { + LoggingBase log; + auto standIn = std::make_shared(); + log.addLogHandler(standIn); + std::string FormatStr{"A test message {:s} - {}"}; + log.fmt_log(Severity::Critical, FormatStr, 42, "hello"); + log.flush(10s); + LogMessage msg = standIn->CurrentMessage; + ASSERT_TRUE(msg.MessageString.find(FormatStr) != std::string::npos); +} + +#endif From 249088bc00c2813a52d9294b6fc9fa8fae93e9db Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 10:44:33 +0100 Subject: [PATCH 25/43] Added unit tests. --- unit_tests/CMakeLists.txt | 2 +- unit_tests/ConsoleInterfaceTest.cpp | 49 +++++++++++++++++++++++++++++ unit_tests/FileInterfaceTest.cpp | 44 ++++++++++++++++++++++++++ unit_tests/GraylogInterfaceTest.cpp | 31 ++++++++++++++++++ unit_tests/Semaphore.hpp | 25 +++++++++++++++ 5 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 unit_tests/Semaphore.hpp diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index fb98038..2e663b2 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -23,7 +23,7 @@ set(UnitTest_SRC set(UnitTest_INC BaseLogHandlerStandIn.hpp LogTestServer.hpp -) + Semaphore.hpp) add_executable(unit_tests EXCLUDE_FROM_ALL ${UnitTest_SRC} ${UnitTest_INC}) diff --git a/unit_tests/ConsoleInterfaceTest.cpp b/unit_tests/ConsoleInterfaceTest.cpp index e8ef611..0ddbb1d 100644 --- a/unit_tests/ConsoleInterfaceTest.cpp +++ b/unit_tests/ConsoleInterfaceTest.cpp @@ -9,6 +9,7 @@ #include "graylog_logger/ConsoleInterface.hpp" #include #include +#include "Semaphore.hpp" using namespace Log; @@ -42,3 +43,51 @@ TEST(ConsoleInterface, ConsoleStringFunctionTest) { std::string output = testing::internal::GetCapturedStdout(); ASSERT_EQ(std::string("ALERT: Some test string\n"), output); } + +TEST(ConsoleInterface, QueueSizeEmpty) { + ConsoleInterface cInter; + ASSERT_EQ(cInter.queueSize(), 0); + ASSERT_TRUE(cInter.emptyQueue()); +} + +class ConsoleInterfaceStandIn : public ConsoleInterface { +public: + using ConsoleInterface::Executor; +}; + +TEST(ConsoleInterface, QueueSizeOne) { + ConsoleInterfaceStandIn cInter; + Semaphore Signal1, Signal2; + Semaphore Signal3; + cInter.Executor.SendWork([&](){ + Signal1.notify(); + Signal2.wait(); + Signal3.notify(); + }); + cInter.Executor.SendWork([](){}); + Signal1.wait(); + EXPECT_EQ(cInter.queueSize(), 1); + EXPECT_FALSE(cInter.emptyQueue()); + Signal2.notify(); + Signal3.wait(); +} + +using namespace std::chrono_literals; + +TEST(ConsoleInterface, FlushSuccess) { + ConsoleInterfaceStandIn cInter; + EXPECT_TRUE(cInter.flush(50ms)); +} + +TEST(ConsoleInterface, FlushFail) { + ConsoleInterfaceStandIn cInter; + Semaphore Signal1, Signal2; + cInter.Executor.SendWork([&](){ + Signal1.wait(); + Signal2.notify(); + }); + EXPECT_FALSE(cInter.flush(50ms)); + Signal1.notify(); + Signal2.wait(); +} + diff --git a/unit_tests/FileInterfaceTest.cpp b/unit_tests/FileInterfaceTest.cpp index e1e70e1..37caf36 100644 --- a/unit_tests/FileInterfaceTest.cpp +++ b/unit_tests/FileInterfaceTest.cpp @@ -8,6 +8,7 @@ #include "graylog_logger/FileInterface.hpp" #include "graylog_logger/Log.hpp" +#include "Semaphore.hpp" #include #include #include @@ -42,6 +43,7 @@ class FileInterfaceStandIn : public FileInterface { explicit FileInterfaceStandIn(const std::string &fileName) : FileInterface(fileName){}; ~FileInterfaceStandIn() override = default; + using FileInterface::Executor; }; class FileInterfaceTest : public ::testing::Test { @@ -105,3 +107,45 @@ TEST_F(FileInterfaceTest, OpenFileMessages) { EXPECT_NE(StdOutputString.find("Unable to open log file"), std::string::npos) << "Actual std-output was: " << StdOutputString; } + +TEST_F(FileInterfaceTest, QueueSizeEmpty) { + FileInterfaceStandIn cInter(usedFileName); + ASSERT_EQ(cInter.queueSize(), 0); + ASSERT_TRUE(cInter.emptyQueue()); +} + +TEST_F(FileInterfaceTest, QueueSizeOne) { + FileInterfaceStandIn cInter(usedFileName); + Semaphore Signal1, Signal2; + Semaphore Signal3; + cInter.Executor.SendWork([&](){ + Signal1.notify(); + Signal2.wait(); + Signal3.notify(); + }); + cInter.Executor.SendWork([](){}); + Signal1.wait(); + EXPECT_EQ(cInter.queueSize(), 1); + EXPECT_FALSE(cInter.emptyQueue()); + Signal2.notify(); + Signal3.wait(); +} + +using namespace std::chrono_literals; + +TEST_F(FileInterfaceTest, FlushSuccess) { + FileInterfaceStandIn cInter(usedFileName); + EXPECT_TRUE(cInter.flush(50ms)); +} + +TEST_F(FileInterfaceTest, FlushFail) { + FileInterfaceStandIn cInter(usedFileName); + Semaphore Signal1, Signal2; + cInter.Executor.SendWork([&](){ + Signal1.wait(); + Signal2.notify(); + }); + EXPECT_FALSE(cInter.flush(50ms)); + Signal1.notify(); + Signal2.wait(); +} diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index e30a52c..c001090 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -8,6 +8,7 @@ #include "graylog_logger/GraylogInterface.hpp" #include "LogTestServer.hpp" +#include "Semaphore.hpp" #include #include #include @@ -33,6 +34,7 @@ class GraylogInterfaceStandIn : public GraylogInterface { : GraylogInterface(host, port, queueLength){}; MOCK_METHOD1(sendMessage, void(std::string)); using GraylogInterface::logMsgToJSON; + void sendMessageBase(std::string Msg) {GraylogInterface::sendMessage(Msg);} }; class GraylogConnectionStandIn : public GraylogConnection { @@ -332,3 +334,32 @@ TEST_F(GraylogConnectionCom, DISABLED_MultipleCloseConnectionTest) { EXPECT_EQ(logServer->GetNrOfMessages(), NrOfMessages); } } + +TEST(GraylogInterface, QueueSizeEmpty) { + GraylogInterfaceStandIn cInter("localhost", testPort, 10); + ASSERT_EQ(cInter.queueSize(), 0); + ASSERT_TRUE(cInter.emptyQueue()); +} + +TEST(GraylogInterface, QueueSizeOne) { + GraylogInterfaceStandIn cInter("localhost", testPort, 10); + cInter.sendMessageBase("tst_msg"); + cInter.sendMessageBase("tst_msg"); + EXPECT_GE(cInter.queueSize(), 1); + EXPECT_LE(cInter.queueSize(), 2); + EXPECT_FALSE(cInter.emptyQueue()); +} + +using namespace std::chrono_literals; + +TEST(GraylogInterface, FlushSuccess) { + auto LogServer = std::make_unique(testPort); + GraylogInterfaceStandIn cInter("localhost", testPort, 10); + EXPECT_TRUE(cInter.flush(50ms)); +} + +TEST(GraylogInterface, FlushFail) { + GraylogInterfaceStandIn cInter("localhost", testPort, 10); + cInter.sendMessageBase("tst_msg"); + EXPECT_FALSE(cInter.flush(50ms)); +} diff --git a/unit_tests/Semaphore.hpp b/unit_tests/Semaphore.hpp new file mode 100644 index 0000000..bb728ae --- /dev/null +++ b/unit_tests/Semaphore.hpp @@ -0,0 +1,25 @@ +#pragma once + +#include +#include +#include + +class Semaphore { +public: + void notify() { + std::lock_guard lock(Mutex); + ++Count; + Condition.notify_one(); + } + + void wait() { + std::unique_lock lock(Mutex); + while(!Count) // Handle spurious wake-ups. + Condition.wait(lock); + --Count; + } +private: + std::mutex Mutex; + std::condition_variable Condition; + std::atomic Count{0}; // Initialized as locked. +}; \ No newline at end of file From 7f9fbbf97aaed62407551fe43c4e7fba939fa676 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 10:45:42 +0100 Subject: [PATCH 26/43] Clang-format --- unit_tests/ConsoleInterfaceTest.cpp | 9 ++++----- unit_tests/FileInterfaceTest.cpp | 8 ++++---- unit_tests/GraylogInterfaceTest.cpp | 2 +- unit_tests/Semaphore.hpp | 7 ++++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/unit_tests/ConsoleInterfaceTest.cpp b/unit_tests/ConsoleInterfaceTest.cpp index 0ddbb1d..a348833 100644 --- a/unit_tests/ConsoleInterfaceTest.cpp +++ b/unit_tests/ConsoleInterfaceTest.cpp @@ -7,9 +7,9 @@ // #include "graylog_logger/ConsoleInterface.hpp" +#include "Semaphore.hpp" #include #include -#include "Semaphore.hpp" using namespace Log; @@ -59,12 +59,12 @@ TEST(ConsoleInterface, QueueSizeOne) { ConsoleInterfaceStandIn cInter; Semaphore Signal1, Signal2; Semaphore Signal3; - cInter.Executor.SendWork([&](){ + cInter.Executor.SendWork([&]() { Signal1.notify(); Signal2.wait(); Signal3.notify(); }); - cInter.Executor.SendWork([](){}); + cInter.Executor.SendWork([]() {}); Signal1.wait(); EXPECT_EQ(cInter.queueSize(), 1); EXPECT_FALSE(cInter.emptyQueue()); @@ -82,7 +82,7 @@ TEST(ConsoleInterface, FlushSuccess) { TEST(ConsoleInterface, FlushFail) { ConsoleInterfaceStandIn cInter; Semaphore Signal1, Signal2; - cInter.Executor.SendWork([&](){ + cInter.Executor.SendWork([&]() { Signal1.wait(); Signal2.notify(); }); @@ -90,4 +90,3 @@ TEST(ConsoleInterface, FlushFail) { Signal1.notify(); Signal2.wait(); } - diff --git a/unit_tests/FileInterfaceTest.cpp b/unit_tests/FileInterfaceTest.cpp index 37caf36..ce7a67c 100644 --- a/unit_tests/FileInterfaceTest.cpp +++ b/unit_tests/FileInterfaceTest.cpp @@ -7,8 +7,8 @@ // #include "graylog_logger/FileInterface.hpp" -#include "graylog_logger/Log.hpp" #include "Semaphore.hpp" +#include "graylog_logger/Log.hpp" #include #include #include @@ -118,12 +118,12 @@ TEST_F(FileInterfaceTest, QueueSizeOne) { FileInterfaceStandIn cInter(usedFileName); Semaphore Signal1, Signal2; Semaphore Signal3; - cInter.Executor.SendWork([&](){ + cInter.Executor.SendWork([&]() { Signal1.notify(); Signal2.wait(); Signal3.notify(); }); - cInter.Executor.SendWork([](){}); + cInter.Executor.SendWork([]() {}); Signal1.wait(); EXPECT_EQ(cInter.queueSize(), 1); EXPECT_FALSE(cInter.emptyQueue()); @@ -141,7 +141,7 @@ TEST_F(FileInterfaceTest, FlushSuccess) { TEST_F(FileInterfaceTest, FlushFail) { FileInterfaceStandIn cInter(usedFileName); Semaphore Signal1, Signal2; - cInter.Executor.SendWork([&](){ + cInter.Executor.SendWork([&]() { Signal1.wait(); Signal2.notify(); }); diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index c001090..e32e15f 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -34,7 +34,7 @@ class GraylogInterfaceStandIn : public GraylogInterface { : GraylogInterface(host, port, queueLength){}; MOCK_METHOD1(sendMessage, void(std::string)); using GraylogInterface::logMsgToJSON; - void sendMessageBase(std::string Msg) {GraylogInterface::sendMessage(Msg);} + void sendMessageBase(std::string Msg) { GraylogInterface::sendMessage(Msg); } }; class GraylogConnectionStandIn : public GraylogConnection { diff --git a/unit_tests/Semaphore.hpp b/unit_tests/Semaphore.hpp index bb728ae..8803286 100644 --- a/unit_tests/Semaphore.hpp +++ b/unit_tests/Semaphore.hpp @@ -1,8 +1,8 @@ #pragma once -#include -#include #include +#include +#include class Semaphore { public: @@ -14,10 +14,11 @@ class Semaphore { void wait() { std::unique_lock lock(Mutex); - while(!Count) // Handle spurious wake-ups. + while (!Count) // Handle spurious wake-ups. Condition.wait(lock); --Count; } + private: std::mutex Mutex; std::condition_variable Condition; From f35577e91bdeb91be5a44f3b3ac369f9042377cf Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 10:47:03 +0100 Subject: [PATCH 27/43] Re-enabled clang-format. --- Jenkinsfile | 4 ++-- documentation/changes.md | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index dfb6b05..a30596a 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -78,14 +78,14 @@ builders = pipeline_builder.createBuilders { container -> } // if if (container.key == clangformat_os) { -/* pipeline_builder.stage("${container.key}: check formatting") { + pipeline_builder.stage("${container.key}: check formatting") { container.sh """ clang-format -version cd ${pipeline_builder.project} find . \\\\( -name '*.cpp' -or -name '*.cxx' -or -name '*.h' -or -name '*.hpp' \\\\) \\ -exec clangformatdiff.sh {} + """ - }*/ // stage + } // stage pipeline_builder.stage("${container.key}: cppcheck") { def test_output = "cppcheck.txt" diff --git a/documentation/changes.md b/documentation/changes.md index b28727e..6af9b9d 100644 --- a/documentation/changes.md +++ b/documentation/changes.md @@ -11,7 +11,6 @@ * Added and fixed examples. * Fixed the non-working `emptyQueue()` and `queueSize()` implementations. * Fixed/improved code documentation. -* De-activated the use of clang-format in our continuous integration system. It will be re-activated when we have updated clang-format on our build nodes to a newer version. * Removed the message queue size parameter from log message handlers as it was deemed unnecessary. * Fixed and added unit tests. From 3788ff3e926bb6ea2a43820b4ed78ac6984eb882 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 10:56:36 +0100 Subject: [PATCH 28/43] Re-disabled clang-format. --- Jenkinsfile | 4 ++-- documentation/changes.md | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index a30596a..dfb6b05 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -78,14 +78,14 @@ builders = pipeline_builder.createBuilders { container -> } // if if (container.key == clangformat_os) { - pipeline_builder.stage("${container.key}: check formatting") { +/* pipeline_builder.stage("${container.key}: check formatting") { container.sh """ clang-format -version cd ${pipeline_builder.project} find . \\\\( -name '*.cpp' -or -name '*.cxx' -or -name '*.h' -or -name '*.hpp' \\\\) \\ -exec clangformatdiff.sh {} + """ - } // stage + }*/ // stage pipeline_builder.stage("${container.key}: cppcheck") { def test_output = "cppcheck.txt" diff --git a/documentation/changes.md b/documentation/changes.md index 6af9b9d..b28727e 100644 --- a/documentation/changes.md +++ b/documentation/changes.md @@ -11,6 +11,7 @@ * Added and fixed examples. * Fixed the non-working `emptyQueue()` and `queueSize()` implementations. * Fixed/improved code documentation. +* De-activated the use of clang-format in our continuous integration system. It will be re-activated when we have updated clang-format on our build nodes to a newer version. * Removed the message queue size parameter from log message handlers as it was deemed unnecessary. * Fixed and added unit tests. From 49f07fe428eeabe4176500246618c704b66b9e96 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 15:46:58 +0100 Subject: [PATCH 29/43] Update include/graylog_logger/FileInterface.hpp Co-Authored-By: Matt Clarke --- include/graylog_logger/FileInterface.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graylog_logger/FileInterface.hpp b/include/graylog_logger/FileInterface.hpp index 70d4510..d4ab99d 100644 --- a/include/graylog_logger/FileInterface.hpp +++ b/include/graylog_logger/FileInterface.hpp @@ -29,7 +29,7 @@ class FileInterface : public BaseLogHandler { /// time out. Returns false otherwise. bool flush(std::chrono::system_clock::duration TimeOut) override; - /// \brief Are there any more queued messages? + /// \brief Are there any queued messages? /// \note The message queue will show as empty before the last message in /// the queue has been written. /// \return Returns true if message queue is empty. From cad1a216b827599eed272c81df6ecbdcbc3b3933 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 15:47:36 +0100 Subject: [PATCH 30/43] Update include/graylog_logger/GraylogInterface.hpp Co-Authored-By: Matt Clarke --- include/graylog_logger/GraylogInterface.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graylog_logger/GraylogInterface.hpp b/include/graylog_logger/GraylogInterface.hpp index f96441d..245ec33 100644 --- a/include/graylog_logger/GraylogInterface.hpp +++ b/include/graylog_logger/GraylogInterface.hpp @@ -43,7 +43,7 @@ class GraylogInterface : public BaseLogHandler, public GraylogConnection { /// been transmitted even if flush() returns true. bool flush(std::chrono::system_clock::duration TimeOut) override; - /// \brief Are there any more queued messages? + /// \brief Are there any queued messages? /// \note The message queue will show as empty before the last message in /// the queue has been transmitted. /// \return Returns true if message queue is empty. From fc448fb74523f0695510b2677152f9b57382dec1 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 15:51:38 +0100 Subject: [PATCH 31/43] Update include/graylog_logger/LogUtil.hpp --- include/graylog_logger/LogUtil.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/include/graylog_logger/LogUtil.hpp b/include/graylog_logger/LogUtil.hpp index f744556..826d251 100644 --- a/include/graylog_logger/LogUtil.hpp +++ b/include/graylog_logger/LogUtil.hpp @@ -113,7 +113,6 @@ class BaseLogHandler { /// \brief Called by the logging library when a new log message is created. /// - /// Must be implement by any derived classes. /// \param[in] Message The log message. virtual void addMessage(const LogMessage &Message) = 0; From 47e138a823c648bb2fc9011b57f6bfef0d2bf71d Mon Sep 17 00:00:00 2001 From: Matt Clarke Date: Mon, 13 Jan 2020 16:37:53 +0100 Subject: [PATCH 32/43] Trying to add Go Format Yourself --- Jenkinsfile | 53 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 046e1df..21cbe75 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -78,14 +78,51 @@ builders = pipeline_builder.createBuilders { container -> } // if if (container.key == clangformat_os) { - pipeline_builder.stage("${container.key}: check formatting") { - container.sh """ - clang-format -version - cd ${pipeline_builder.project} - find . \\\\( -name '*.cpp' -or -name '*.cxx' -or -name '*.h' -or -name '*.hpp' \\\\) \\ - -exec clangformatdiff.sh {} + - """ - } // stage + pipeline_builder.stage("${container.key}: Formatting") { + if (!env.CHANGE_ID) { + // Ignore non-PRs + return + } + try { + // Do clang-format of C++ files + container.sh """ + clang-format -version + cd ${project} + find . \\\\( -name '*.cpp' -or -name '*.cxx' -or -name '*.h' -or -name '*.hpp' \\\\) \\ + -exec clang-format -i {} + + git config user.email 'dm-jenkins-integration@esss.se' + git config user.name 'cow-bot' + git status -s + git add -u + git commit -m 'GO FORMAT YOURSELF (clang-format)' + """ + } catch (e) { + // Okay to fail as there could be no badly formatted files to commit + } finally { + // Clean up + } + + // Push any changes resulting from formatting + try { + withCredentials([ + usernamePassword( + credentialsId: 'cow-bot-username', + usernameVariable: 'USERNAME', + passwordVariable: 'PASSWORD' + ) + ]) { + container.sh """ + cd ${project} + git push https://${USERNAME}:${PASSWORD}@github.com/ess-dmsc/${pipeline_builder.project}.git HEAD:${CHANGE_BRANCH} + """ + } // withCredentials + } catch (e) { + // Okay to fail; there may be nothing to push + } finally { + // Clean up + } + } // stage + pipeline_builder.stage("${container.key}: cppcheck") { def test_output = "cppcheck.txt" From 44b6a9bb59b7dabdce07fc36971e4ba4b2eed2b2 Mon Sep 17 00:00:00 2001 From: cow-bot Date: Mon, 13 Jan 2020 15:44:01 +0000 Subject: [PATCH 33/43] GO FORMAT YOURSELF (clang-format) --- include/graylog_logger/LoggingBase.hpp | 7 +++---- performance_test/DummyLogHandler.h | 1 - unit_tests/ConsoleInterfaceTest.cpp | 2 +- unit_tests/FileInterfaceTest.cpp | 2 +- unit_tests/GraylogInterfaceTest.cpp | 2 +- unit_tests/LoggingBaseTest.cpp | 2 +- 6 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index eea5df3..6870554 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -58,10 +58,9 @@ class LoggingBase { } virtual void log(const Severity Level, const std::string &Message, const std::pair &ExtraField) { - log(Level, Message, - std::vector>{ - ExtraField, - }); + log(Level, Message, std::vector>{ + ExtraField, + }); } #ifdef WITH_FMT diff --git a/performance_test/DummyLogHandler.h b/performance_test/DummyLogHandler.h index 2d5e4f6..0f98ad1 100644 --- a/performance_test/DummyLogHandler.h +++ b/performance_test/DummyLogHandler.h @@ -7,7 +7,6 @@ /// //===----------------------------------------------------------------------===// - #pragma once #include diff --git a/unit_tests/ConsoleInterfaceTest.cpp b/unit_tests/ConsoleInterfaceTest.cpp index a348833..de07d50 100644 --- a/unit_tests/ConsoleInterfaceTest.cpp +++ b/unit_tests/ConsoleInterfaceTest.cpp @@ -6,8 +6,8 @@ // Copyright © 2016 European Spallation Source. All rights reserved. // -#include "graylog_logger/ConsoleInterface.hpp" #include "Semaphore.hpp" +#include "graylog_logger/ConsoleInterface.hpp" #include #include diff --git a/unit_tests/FileInterfaceTest.cpp b/unit_tests/FileInterfaceTest.cpp index ce7a67c..7b80129 100644 --- a/unit_tests/FileInterfaceTest.cpp +++ b/unit_tests/FileInterfaceTest.cpp @@ -6,8 +6,8 @@ // Copyright © 2016 European Spallation Source. All rights reserved. // -#include "graylog_logger/FileInterface.hpp" #include "Semaphore.hpp" +#include "graylog_logger/FileInterface.hpp" #include "graylog_logger/Log.hpp" #include #include diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index e32e15f..add907a 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -6,9 +6,9 @@ // Copyright © 2016 European Spallation Source. All rights reserved. // -#include "graylog_logger/GraylogInterface.hpp" #include "LogTestServer.hpp" #include "Semaphore.hpp" +#include "graylog_logger/GraylogInterface.hpp" #include #include #include diff --git a/unit_tests/LoggingBaseTest.cpp b/unit_tests/LoggingBaseTest.cpp index b091564..e79aece 100644 --- a/unit_tests/LoggingBaseTest.cpp +++ b/unit_tests/LoggingBaseTest.cpp @@ -6,10 +6,10 @@ // Copyright © 2016 European Spallation Source. All rights reserved. // -#include "graylog_logger/LoggingBase.hpp" #include "BaseLogHandlerStandIn.hpp" #include "graylog_logger/LibConfig.hpp" #include "graylog_logger/LogUtil.hpp" +#include "graylog_logger/LoggingBase.hpp" #include #include #include From b5364d7d511c0582c8e51c3b10ece980e5de01d4 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 17:07:13 +0100 Subject: [PATCH 34/43] Update include/graylog_logger/LoggingBase.hpp Co-Authored-By: Matt Clarke --- include/graylog_logger/LoggingBase.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 6870554..5161e2f 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -75,7 +75,7 @@ class LoggingBase { LogMessage cMsg(BaseMsg); cMsg.SeverityLevel = Level; cMsg.Timestamp = std::chrono::system_clock::now(); - auto format_message = [=, &cMsg](const auto &... args) { + auto format_message = [&Format, &cMsg](const auto &... args) { try { return fmt::format(Format, args...); } catch (fmt::format_error &e) { From 488b398dad1c0f9f15be50d2811794b6abf66b6c Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 17:07:37 +0100 Subject: [PATCH 35/43] Update include/graylog_logger/LoggingBase.hpp Co-Authored-By: Matt Clarke --- include/graylog_logger/LoggingBase.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 5161e2f..bcaecfe 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -109,7 +109,7 @@ class LoggingBase { virtual bool flush(std::chrono::system_clock::duration TimeOut) { auto FlushCompleted = std::make_shared>(); auto FlushCompletedValue = FlushCompleted->get_future(); - Executor.SendWork([ =, FlushCompleted{std::move(FlushCompleted)} ]() { + Executor.SendWork([=, FlushCompleted{std::move(FlushCompleted)} ]() { std::vector> FlushResults; for (auto &CHandler : Handlers) { FlushResults.push_back(std::async( From 6d224e81641a25ac28b0010fae6550d365e534d3 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 17:09:07 +0100 Subject: [PATCH 36/43] Apply suggestions from code review Co-Authored-By: Matt Clarke --- src/LoggingBase.cpp | 5 ++--- unit_tests/ConsoleInterfaceTest.cpp | 4 ++-- unit_tests/FileInterfaceTest.cpp | 4 ++-- unit_tests/GraylogInterfaceTest.cpp | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index f5880c9..1cdf72b 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -97,15 +97,14 @@ LoggingBase::LoggingBase() { Executor.SendWork([=]() { const int StringBufferSize = 100; std::array StringBuffer{}; - int res; - res = + const int res = gethostname(static_cast(StringBuffer.data()), StringBufferSize); if (0 == res) { BaseMsg.Host = std::string(static_cast(StringBuffer.data())); } BaseMsg.ProcessId = getpid(); BaseMsg.ProcessName = get_process_name(); - // We want to wait for this to finnish, make it so + // We want to wait for this to finish, make it so }); } diff --git a/unit_tests/ConsoleInterfaceTest.cpp b/unit_tests/ConsoleInterfaceTest.cpp index de07d50..576e5e4 100644 --- a/unit_tests/ConsoleInterfaceTest.cpp +++ b/unit_tests/ConsoleInterfaceTest.cpp @@ -44,7 +44,7 @@ TEST(ConsoleInterface, ConsoleStringFunctionTest) { ASSERT_EQ(std::string("ALERT: Some test string\n"), output); } -TEST(ConsoleInterface, QueueSizeEmpty) { +TEST(ConsoleInterface, OnInitialisationQueueEmpty) { ConsoleInterface cInter; ASSERT_EQ(cInter.queueSize(), 0); ASSERT_TRUE(cInter.emptyQueue()); @@ -55,7 +55,7 @@ class ConsoleInterfaceStandIn : public ConsoleInterface { using ConsoleInterface::Executor; }; -TEST(ConsoleInterface, QueueSizeOne) { +TEST(ConsoleInterface, QueueSizeOneIsNotEmpty) { ConsoleInterfaceStandIn cInter; Semaphore Signal1, Signal2; Semaphore Signal3; diff --git a/unit_tests/FileInterfaceTest.cpp b/unit_tests/FileInterfaceTest.cpp index 7b80129..79b67f9 100644 --- a/unit_tests/FileInterfaceTest.cpp +++ b/unit_tests/FileInterfaceTest.cpp @@ -108,13 +108,13 @@ TEST_F(FileInterfaceTest, OpenFileMessages) { << "Actual std-output was: " << StdOutputString; } -TEST_F(FileInterfaceTest, QueueSizeEmpty) { +TEST_F(FileInterfaceTest, OnInitialisationQueueEmpty) { FileInterfaceStandIn cInter(usedFileName); ASSERT_EQ(cInter.queueSize(), 0); ASSERT_TRUE(cInter.emptyQueue()); } -TEST_F(FileInterfaceTest, QueueSizeOne) { +TEST_F(FileInterfaceTest, QueueSizeOneIsNotEmpty) { FileInterfaceStandIn cInter(usedFileName); Semaphore Signal1, Signal2; Semaphore Signal3; diff --git a/unit_tests/GraylogInterfaceTest.cpp b/unit_tests/GraylogInterfaceTest.cpp index add907a..fa6e585 100644 --- a/unit_tests/GraylogInterfaceTest.cpp +++ b/unit_tests/GraylogInterfaceTest.cpp @@ -335,13 +335,13 @@ TEST_F(GraylogConnectionCom, DISABLED_MultipleCloseConnectionTest) { } } -TEST(GraylogInterface, QueueSizeEmpty) { +TEST(GraylogInterface, OnInitialisationQueueEmpty) { GraylogInterfaceStandIn cInter("localhost", testPort, 10); ASSERT_EQ(cInter.queueSize(), 0); ASSERT_TRUE(cInter.emptyQueue()); } -TEST(GraylogInterface, QueueSizeOne) { +TEST(GraylogInterface, QueueSizeOneIsNotEmpty) { GraylogInterfaceStandIn cInter("localhost", testPort, 10); cInter.sendMessageBase("tst_msg"); cInter.sendMessageBase("tst_msg"); From f0b6bb44706b1115a17aa2ccf576b8f16bbb0579 Mon Sep 17 00:00:00 2001 From: cow-bot Date: Mon, 13 Jan 2020 16:15:55 +0000 Subject: [PATCH 37/43] GO FORMAT YOURSELF (clang-format) --- include/graylog_logger/LoggingBase.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index bcaecfe..5161e2f 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -109,7 +109,7 @@ class LoggingBase { virtual bool flush(std::chrono::system_clock::duration TimeOut) { auto FlushCompleted = std::make_shared>(); auto FlushCompletedValue = FlushCompleted->get_future(); - Executor.SendWork([=, FlushCompleted{std::move(FlushCompleted)} ]() { + Executor.SendWork([ =, FlushCompleted{std::move(FlushCompleted)} ]() { std::vector> FlushResults; for (auto &CHandler : Handlers) { FlushResults.push_back(std::async( From 46b03ceb8edd75d140df56241971e16d775a90ce Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 23:24:35 +0100 Subject: [PATCH 38/43] Added benchmark and optimised the code. --- include/graylog_logger/LoggingBase.hpp | 12 ++++++------ performance_test/PerformanceTest.cpp | 17 +++++++++++++++++ src/LoggingBase.cpp | 8 ++++++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 5161e2f..83f302a 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -36,11 +36,11 @@ class LoggingBase { virtual void log(const Severity Level, const std::string &Message, const std::vector> &ExtraFields) { + if (int(Level) > int(MinSeverity)) { + return; + } auto ThreadId = std::this_thread::get_id(); Executor.SendWork([=]() { - if (int(Level) > int(MinSeverity)) { - return; - } LogMessage cMsg(BaseMsg); for (auto &fld : ExtraFields) { cMsg.addField(fld.first, fld.second); @@ -66,12 +66,12 @@ class LoggingBase { #ifdef WITH_FMT template void fmt_log(const Severity Level, std::string Format, Args... args) { + if (int(Level) > int(MinSeverity)) { + return; + } auto ThreadId = std::this_thread::get_id(); auto UsedArguments = std::make_tuple(args...); Executor.SendWork([=]() { - if (int(Level) > int(MinSeverity)) { - return; - } LogMessage cMsg(BaseMsg); cMsg.SeverityLevel = Level; cMsg.Timestamp = std::chrono::system_clock::now(); diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index 2dfb8ba..845bc2d 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -11,6 +11,7 @@ #include #include #include +#include static void BM_LogMessageGenerationOnly(benchmark::State &state) { Log::LoggingBase Logger; @@ -58,4 +59,20 @@ BM_LogMessageGenerationWithDeferredFmtFormatting(benchmark::State &state) { } BENCHMARK(BM_LogMessageGenerationWithDeferredFmtFormatting); +static void +BM_RandomSeverityLevel(benchmark::State &state) { + Log::LoggingBase Logger; + Logger.setMinSeverity(Log::Severity::Alert); + auto Handler = std::make_shared(); + Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); + std::random_device Device; + std::mt19937 Generator(Device()); + std::uniform_int_distribution<> Distribution(0, 7); + for (auto _ : state) { + Logger.log(Log::Severity(Distribution(Generator)), "Some format example"); + } + state.SetItemsProcessed(state.iterations()); +} +BENCHMARK(BM_RandomSeverityLevel); + BENCHMARK_MAIN(); diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index 1cdf72b..498722f 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -104,7 +104,6 @@ LoggingBase::LoggingBase() { } BaseMsg.ProcessId = getpid(); BaseMsg.ProcessName = get_process_name(); - // We want to wait for this to finish, make it so }); } @@ -121,7 +120,12 @@ void LoggingBase::removeAllHandlers() { std::vector LoggingBase::getHandlers() { return Handlers; } void LoggingBase::setMinSeverity(Severity Level) { - Executor.SendWork([=]() { MinSeverity = Level; }); + auto WorkDone = std::make_shared>(); + auto WorkDoneFuture = WorkDone->get_future(); + Executor.SendWork([ =, WorkDone{std::move(WorkDone)} ]() { + MinSeverity = Level; + }); + WorkDoneFuture.wait(); } } // namespace Log From eab8c7c0f2fa3b11925d25496c8c912b60b6613d Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 23:27:44 +0100 Subject: [PATCH 39/43] Updated the documentation. --- documentation/changes.md | 1 - 1 file changed, 1 deletion(-) diff --git a/documentation/changes.md b/documentation/changes.md index b28727e..6af9b9d 100644 --- a/documentation/changes.md +++ b/documentation/changes.md @@ -11,7 +11,6 @@ * Added and fixed examples. * Fixed the non-working `emptyQueue()` and `queueSize()` implementations. * Fixed/improved code documentation. -* De-activated the use of clang-format in our continuous integration system. It will be re-activated when we have updated clang-format on our build nodes to a newer version. * Removed the message queue size parameter from log message handlers as it was deemed unnecessary. * Fixed and added unit tests. From ea0c055b57df5d9867bc716201def07b443ff861 Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Mon, 13 Jan 2020 23:29:53 +0100 Subject: [PATCH 40/43] Clang-format. --- include/graylog_logger/LoggingBase.hpp | 7 ++++--- performance_test/PerformanceTest.cpp | 3 +-- src/LoggingBase.cpp | 5 ++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 83f302a..59a0a69 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -58,9 +58,10 @@ class LoggingBase { } virtual void log(const Severity Level, const std::string &Message, const std::pair &ExtraField) { - log(Level, Message, std::vector>{ - ExtraField, - }); + log(Level, Message, + std::vector>{ + ExtraField, + }); } #ifdef WITH_FMT diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index 845bc2d..90b81ff 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -59,8 +59,7 @@ BM_LogMessageGenerationWithDeferredFmtFormatting(benchmark::State &state) { } BENCHMARK(BM_LogMessageGenerationWithDeferredFmtFormatting); -static void -BM_RandomSeverityLevel(benchmark::State &state) { +static void BM_RandomSeverityLevel(benchmark::State &state) { Log::LoggingBase Logger; Logger.setMinSeverity(Log::Severity::Alert); auto Handler = std::make_shared(); diff --git a/src/LoggingBase.cpp b/src/LoggingBase.cpp index 498722f..f3eb786 100644 --- a/src/LoggingBase.cpp +++ b/src/LoggingBase.cpp @@ -122,9 +122,8 @@ std::vector LoggingBase::getHandlers() { return Handlers; } void LoggingBase::setMinSeverity(Severity Level) { auto WorkDone = std::make_shared>(); auto WorkDoneFuture = WorkDone->get_future(); - Executor.SendWork([ =, WorkDone{std::move(WorkDone)} ]() { - MinSeverity = Level; - }); + Executor.SendWork( + [ =, WorkDone{std::move(WorkDone)} ]() { MinSeverity = Level; }); WorkDoneFuture.wait(); } From ac0fb236312c079966987cebd75d93f84f7ff600 Mon Sep 17 00:00:00 2001 From: cow-bot Date: Mon, 13 Jan 2020 22:29:47 +0000 Subject: [PATCH 41/43] GO FORMAT YOURSELF (clang-format) --- include/graylog_logger/LoggingBase.hpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/graylog_logger/LoggingBase.hpp b/include/graylog_logger/LoggingBase.hpp index 59a0a69..83f302a 100644 --- a/include/graylog_logger/LoggingBase.hpp +++ b/include/graylog_logger/LoggingBase.hpp @@ -58,10 +58,9 @@ class LoggingBase { } virtual void log(const Severity Level, const std::string &Message, const std::pair &ExtraField) { - log(Level, Message, - std::vector>{ - ExtraField, - }); + log(Level, Message, std::vector>{ + ExtraField, + }); } #ifdef WITH_FMT From 1e62adb597aff14367d2baed05a49cf980319b5c Mon Sep 17 00:00:00 2001 From: Jonas Nilsson Date: Tue, 14 Jan 2020 09:20:33 +0100 Subject: [PATCH 42/43] More benchmarking code. --- performance_test/PerformanceTest.cpp | 37 +++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index 90b81ff..d7f951c 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -68,10 +68,45 @@ static void BM_RandomSeverityLevel(benchmark::State &state) { std::mt19937 Generator(Device()); std::uniform_int_distribution<> Distribution(0, 7); for (auto _ : state) { - Logger.log(Log::Severity(Distribution(Generator)), "Some format example"); + Logger.log(Log::Severity(Distribution(Generator)), "Some test string."); } state.SetItemsProcessed(state.iterations()); } BENCHMARK(BM_RandomSeverityLevel); +//#include "spdlog/spdlog.h" +//#include "spdlog/sinks/null_sink.h" +//#include "spdlog/async.h" +// +//static void BM_SpdLogWithFmtAndSeverityLvl(benchmark::State &state) { +// Log::LoggingBase Logger; +// auto null_sink = std::make_shared(); +// auto tp = std::make_shared(4096, 1); +// auto logger = std::make_shared("async_logger", std::move(null_sink), std::move(tp), spdlog::async_overflow_policy::overrun_oldest); +// std::random_device Device; +// std::mt19937 Generator(Device()); +// std::uniform_int_distribution<> Distribution(0, 6); +// logger->set_level(spdlog::level::level_enum(3)); +// for (auto _ : state) { +// logger->log(spdlog::level::level_enum(Distribution(Generator)), "Hello - {} - {} - {}", 42, 3.14, "test"); +// } +// state.SetItemsProcessed(state.iterations()); +//} +//BENCHMARK(BM_SpdLogWithFmtAndSeverityLvl); + +static void BM_GraylogWithFmtAndSeverityLvl(benchmark::State &state) { + Log::LoggingBase Logger; + Logger.setMinSeverity(Log::Severity(3)); + auto Handler = std::make_shared(); + Logger.addLogHandler(std::dynamic_pointer_cast(Handler)); + std::random_device Device; + std::mt19937 Generator(Device()); + std::uniform_int_distribution<> Distribution(0, 6); + for (auto _ : state) { + Logger.fmt_log(Log::Severity(Distribution(Generator)), "Hello - {} - {} - {}", 42, 3.14, "test"); + } + state.SetItemsProcessed(state.iterations()); +} +BENCHMARK(BM_GraylogWithFmtAndSeverityLvl); + BENCHMARK_MAIN(); From 8f86bc4f8d435f0bac49b486dc6fbbd185d5d012 Mon Sep 17 00:00:00 2001 From: cow-bot Date: Tue, 14 Jan 2020 08:25:15 +0000 Subject: [PATCH 43/43] GO FORMAT YOURSELF (clang-format) --- performance_test/PerformanceTest.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/performance_test/PerformanceTest.cpp b/performance_test/PerformanceTest.cpp index d7f951c..7c19d48 100644 --- a/performance_test/PerformanceTest.cpp +++ b/performance_test/PerformanceTest.cpp @@ -78,21 +78,24 @@ BENCHMARK(BM_RandomSeverityLevel); //#include "spdlog/sinks/null_sink.h" //#include "spdlog/async.h" // -//static void BM_SpdLogWithFmtAndSeverityLvl(benchmark::State &state) { +// static void BM_SpdLogWithFmtAndSeverityLvl(benchmark::State &state) { // Log::LoggingBase Logger; // auto null_sink = std::make_shared(); // auto tp = std::make_shared(4096, 1); -// auto logger = std::make_shared("async_logger", std::move(null_sink), std::move(tp), spdlog::async_overflow_policy::overrun_oldest); +// auto logger = std::make_shared("async_logger", +// std::move(null_sink), std::move(tp), +// spdlog::async_overflow_policy::overrun_oldest); // std::random_device Device; // std::mt19937 Generator(Device()); // std::uniform_int_distribution<> Distribution(0, 6); // logger->set_level(spdlog::level::level_enum(3)); // for (auto _ : state) { -// logger->log(spdlog::level::level_enum(Distribution(Generator)), "Hello - {} - {} - {}", 42, 3.14, "test"); +// logger->log(spdlog::level::level_enum(Distribution(Generator)), "Hello - +// {} - {} - {}", 42, 3.14, "test"); // } // state.SetItemsProcessed(state.iterations()); //} -//BENCHMARK(BM_SpdLogWithFmtAndSeverityLvl); +// BENCHMARK(BM_SpdLogWithFmtAndSeverityLvl); static void BM_GraylogWithFmtAndSeverityLvl(benchmark::State &state) { Log::LoggingBase Logger; @@ -103,7 +106,8 @@ static void BM_GraylogWithFmtAndSeverityLvl(benchmark::State &state) { std::mt19937 Generator(Device()); std::uniform_int_distribution<> Distribution(0, 6); for (auto _ : state) { - Logger.fmt_log(Log::Severity(Distribution(Generator)), "Hello - {} - {} - {}", 42, 3.14, "test"); + Logger.fmt_log(Log::Severity(Distribution(Generator)), + "Hello - {} - {} - {}", 42, 3.14, "test"); } state.SetItemsProcessed(state.iterations()); }