From 41d493afe80128b8f248b6d326c59054a476a830 Mon Sep 17 00:00:00 2001 From: Gustav Andersson Date: Thu, 21 Mar 2024 14:03:51 +0100 Subject: [PATCH 1/5] Passing as spdlog::format_string_t and not const char* --- include/elklog/elk_logger.h | 44 +++++++++++++++++++++++------------ include/elklog/rtlogger.h | 38 ++++++++++++++++++++++++------ include/elklog/rtlogmessage.h | 22 +++++++++++++++--- 3 files changed, 79 insertions(+), 25 deletions(-) diff --git a/include/elklog/elk_logger.h b/include/elklog/elk_logger.h index 31d64bb..f4a81c1 100644 --- a/include/elklog/elk_logger.h +++ b/include/elklog/elk_logger.h @@ -250,77 +250,91 @@ class ElkLogger } template - void debug(const char* format_str, Args&&... args) + void debug(spdlog::format_string_t format_str, Args&&... args) { if (_closed == true) return; if (twine::is_current_thread_realtime()) { - _rt_logger->log_debug(format_str, args...); + _rt_logger->log_debug(format_str, std::forward(args)...); } else { - _logger_instance->debug(format_str, args...); + _logger_instance->debug(format_str, std::forward(args)...); } } template - void info(const char* format_str, Args&&... args) + void info(spdlog::format_string_t format_str, Args&&... args) { if (_closed == true) return; if (twine::is_current_thread_realtime()) { - _rt_logger->log_info(format_str, args...); + _rt_logger->log_info(format_str, std::forward(args)...); } else { - _logger_instance->info(format_str, args...); + _logger_instance->info(format_str, std::forward(args)...); + } + } + + void info(spdlog::string_view_t msg) + { + if (_closed == true) return; + + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_info(msg); + } + else + { + _logger_instance->info(msg); } } template - void warning(const char* format_str, Args&&... args) + void warning(spdlog::format_string_t format_str, Args&&... args) { if (_closed == true) return; if (twine::is_current_thread_realtime()) { - _rt_logger->log_warning(format_str, args...); + _rt_logger->log_warning(format_str, std::forward(args)...); } else { - _logger_instance->warn(format_str, args...); + _logger_instance->warn(format_str, std::forward(args)...); } } template - void error(const char* format_str, Args&&... args) + void error(spdlog::format_string_t format_str, Args&&... args) { if (_closed == true) return; if (twine::is_current_thread_realtime()) { - _rt_logger->log_error(format_str, args...); + _rt_logger->log_error(format_str, std::forward(args)...); } else { - _logger_instance->error(format_str, args...); + _logger_instance->error(format_str, std::forward(args)...); } } template - void critical(const char* format_str, Args&&... args) + void critical(spdlog::format_string_t format_str, Args&&... args) { if (_closed == true) return; if (twine::is_current_thread_realtime()) { - _rt_logger->log_error(format_str, args...); + _rt_logger->log_error(format_str, std::forward(args)...); } else { - _logger_instance->critical(format_str, args...); + _logger_instance->critical(format_str, std::forward(args)...); } } diff --git a/include/elklog/rtlogger.h b/include/elklog/rtlogger.h index 16ffb41..bffd561 100644 --- a/include/elklog/rtlogger.h +++ b/include/elklog/rtlogger.h @@ -26,6 +26,7 @@ #include #include +#include #include "rtlogmessage.h" @@ -37,6 +38,8 @@ #include #include +#include "spdlog/spdlog.h" + #include "fifo/circularfifo_memory_relaxed_aquire_release.h" #include "twine/twine.h" @@ -92,14 +95,29 @@ class RtLogger } template - void log(const char* format_str, Args&&... args) + void log(spdlog::format_string_t format_str, Args&&... args) + { + if (_min_log_level < level) + { + return; + } + RtLogMessage message; + message.set_message(level, twine::current_rt_time(), format_str, std::forward(args)...); + + _lock.lock(); + _queue.push(message); + _lock.unlock(); + } + + template + void log(spdlog::string_view_t msg) { if (_min_log_level < level) { return; } RtLogMessage message; - message.set_message(level, twine::current_rt_time(), format_str, args...); + message.set_message(level, twine::current_rt_time(), msg); _lock.lock(); _queue.push(message); @@ -107,25 +125,31 @@ class RtLogger } template - void log_debug(const char* format_str, Args&&... args) + void log_debug(spdlog::string_view_t format_str, Args&&... args) { log(format_str, args...); } template - void log_info(const char* format_str, Args&&... args) + void log_info(spdlog::string_view_t format_str, Args&&... args) { log(format_str, args...); } + void log_info(spdlog::string_view_t msg) + { + log(msg); + } + template - void log_warning(const char* format_str, Args&&... args) + void log_warning(spdlog::format_string_t format_str, Args&&... args) { - log(format_str, args...); + static_assert(std::is_constant_evaluated()); + log(format_str, std::forward(args)...); } template - void log_error(const char* format_str, Args&&... args) + void log_error(spdlog::string_view_t format_str, Args&&... args) { log(format_str, args...); } diff --git a/include/elklog/rtlogmessage.h b/include/elklog/rtlogmessage.h index 7990c88..c01b44d 100644 --- a/include/elklog/rtlogmessage.h +++ b/include/elklog/rtlogmessage.h @@ -37,6 +37,7 @@ ELK_DISABLE_DEPRECATED #include ELK_POP_WARNING +#include "spdlog/spdlog.h" #include "rtloglevel.h" namespace elklog { @@ -101,17 +102,32 @@ class RtLogMessage */ template void set_message(RtLogLevel level, std::chrono::nanoseconds timestamp, - const char* format_str, Args&&... args) + spdlog::format_string_t format_str, Args&&... args) { _level = level; _timestamp = timestamp; - auto end = fmt::format_to_n(_buffer.data(), buffer_len - 1, format_str, args...); + auto end = fmt::format_to_n(_buffer.data(), buffer_len - 1, format_str, std::forward(args)...); + //auto end = fmt::format_to(_buffer.data(), format_str, args...); - // Add null-termination character + //Add null-termination character *end.out = '\0'; _length = static_cast(std::distance(_buffer.data(), end.out)); } + void set_message(RtLogLevel level, std::chrono::nanoseconds timestamp, + spdlog::string_view_t msg) + { + _level = level; + _timestamp = timestamp; + auto end = std::copy_n(msg.begin(), std::min(msg.size(), buffer_len - 1), _buffer.data()); + + //Add null-termination character + *end = '\0'; + _length = msg.size() + 1; + } + + + private: RtLogLevel _level; std::chrono::nanoseconds _timestamp; From e35111e12870b1886f6a02af3f6b9734fa260d3b Mon Sep 17 00:00:00 2001 From: Gustav Andersson Date: Thu, 21 Mar 2024 14:12:26 +0100 Subject: [PATCH 2/5] Adding also for warning logs --- include/elklog/rtlogger.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/elklog/rtlogger.h b/include/elklog/rtlogger.h index bffd561..33e4347 100644 --- a/include/elklog/rtlogger.h +++ b/include/elklog/rtlogger.h @@ -131,7 +131,7 @@ class RtLogger } template - void log_info(spdlog::string_view_t format_str, Args&&... args) + void log_info(spdlog::format_string_t format_str, Args&&... args) { log(format_str, args...); } @@ -144,7 +144,6 @@ class RtLogger template void log_warning(spdlog::format_string_t format_str, Args&&... args) { - static_assert(std::is_constant_evaluated()); log(format_str, std::forward(args)...); } From ea62b6a25d5376df1a1f66adfe06b83ccec88ffe Mon Sep 17 00:00:00 2001 From: Gustav Andersson Date: Thu, 21 Mar 2024 14:30:43 +0100 Subject: [PATCH 3/5] Adding single message log function for all log levels --- include/elklog/elk_logger.h | 56 +++++++++++++++++++++++++++++++++++++ include/elklog/rtlogger.h | 18 ++++++++++-- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/include/elklog/elk_logger.h b/include/elklog/elk_logger.h index f4a81c1..6a120ed 100644 --- a/include/elklog/elk_logger.h +++ b/include/elklog/elk_logger.h @@ -264,6 +264,20 @@ class ElkLogger } } + void debug(spdlog::string_view_t msg) + { + if (_closed == true) return; + + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_debug(msg); + } + else + { + _logger_instance->debug(msg); + } + } + template void info(spdlog::format_string_t format_str, Args&&... args) { @@ -308,6 +322,20 @@ class ElkLogger } } + void warning(spdlog::string_view_t msg) + { + if (_closed == true) return; + + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_warning(msg); + } + else + { + _logger_instance->warn(msg); + } + } + template void error(spdlog::format_string_t format_str, Args&&... args) { @@ -323,6 +351,20 @@ class ElkLogger } } + void error(spdlog::string_view_t msg) + { + if (_closed == true) return; + + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_error(msg); + } + else + { + _logger_instance->error(msg); + } + } + template void critical(spdlog::format_string_t format_str, Args&&... args) { @@ -338,6 +380,20 @@ class ElkLogger } } + void critical(spdlog::string_view_t msg) + { + if (_closed == true) return; + + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_error(msg); + } + else + { + _logger_instance->critical(msg); + } + } + void close_log() { if (_type != Type::JSON) diff --git a/include/elklog/rtlogger.h b/include/elklog/rtlogger.h index 33e4347..c5ec9ab 100644 --- a/include/elklog/rtlogger.h +++ b/include/elklog/rtlogger.h @@ -125,11 +125,16 @@ class RtLogger } template - void log_debug(spdlog::string_view_t format_str, Args&&... args) + void log_debug(spdlog::format_string_t format_str, Args&&... args) { log(format_str, args...); } + void log_debug(spdlog::string_view_t msg) + { + log(msg); + } + template void log_info(spdlog::format_string_t format_str, Args&&... args) { @@ -147,12 +152,21 @@ class RtLogger log(format_str, std::forward(args)...); } + void log_warning(spdlog::string_view_t msg) + { + log(msg); + } + template - void log_error(spdlog::string_view_t format_str, Args&&... args) + void log_error(spdlog::format_string_t format_str, Args&&... args) { log(format_str, args...); } + void log_error(spdlog::string_view_t msg) + { + log(msg); + } private: void _consumer_worker() From 0179cb4c07719b6edacd199307e4459ba4bddc66 Mon Sep 17 00:00:00 2001 From: Gustav Andersson Date: Thu, 21 Mar 2024 14:46:30 +0100 Subject: [PATCH 4/5] Cleanup and comments --- include/elklog/elk_logger.h | 170 ++++++++++++++++++---------------- include/elklog/rtlogmessage.h | 9 +- 2 files changed, 95 insertions(+), 84 deletions(-) diff --git a/include/elklog/elk_logger.h b/include/elklog/elk_logger.h index 6a120ed..8adca05 100644 --- a/include/elklog/elk_logger.h +++ b/include/elklog/elk_logger.h @@ -252,145 +252,155 @@ class ElkLogger template void debug(spdlog::format_string_t format_str, Args&&... args) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) - { - _rt_logger->log_debug(format_str, std::forward(args)...); - } - else + if (!_closed) { - _logger_instance->debug(format_str, std::forward(args)...); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_debug(format_str, std::forward(args)...); + } + else + { + _logger_instance->debug(format_str, std::forward(args)...); + } } } void debug(spdlog::string_view_t msg) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) + if (!_closed) { - _rt_logger->log_debug(msg); - } - else - { - _logger_instance->debug(msg); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_debug(msg); + } + else + { + _logger_instance->debug(msg); + } } } template void info(spdlog::format_string_t format_str, Args&&... args) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) - { - _rt_logger->log_info(format_str, std::forward(args)...); - } - else + if (!_closed) { - _logger_instance->info(format_str, std::forward(args)...); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_info(format_str, std::forward(args)...); + } + else + { + _logger_instance->info(format_str, std::forward(args)...); + } } } void info(spdlog::string_view_t msg) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) + if (!_closed) { - _rt_logger->log_info(msg); - } - else - { - _logger_instance->info(msg); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_info(msg); + } + else + { + _logger_instance->info(msg); + } } } template void warning(spdlog::format_string_t format_str, Args&&... args) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) + if (!_closed) { - _rt_logger->log_warning(format_str, std::forward(args)...); - } - else - { - _logger_instance->warn(format_str, std::forward(args)...); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_warning(format_str, std::forward(args)...); + } + else + { + _logger_instance->warn(format_str, std::forward(args)...); + } } } void warning(spdlog::string_view_t msg) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) - { - _rt_logger->log_warning(msg); - } - else + if (!_closed) { - _logger_instance->warn(msg); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_warning(msg); + } + else + { + _logger_instance->warn(msg); + } } } template void error(spdlog::format_string_t format_str, Args&&... args) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) + if (!_closed) { - _rt_logger->log_error(format_str, std::forward(args)...); - } - else - { - _logger_instance->error(format_str, std::forward(args)...); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_error(format_str, std::forward(args)...); + } + else + { + _logger_instance->error(format_str, std::forward(args)...); + } } } void error(spdlog::string_view_t msg) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) - { - _rt_logger->log_error(msg); - } - else + if (!_closed) { - _logger_instance->error(msg); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_error(msg); + } + else + { + _logger_instance->error(msg); + } } } template void critical(spdlog::format_string_t format_str, Args&&... args) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) + if (!_closed) { - _rt_logger->log_error(format_str, std::forward(args)...); - } - else - { - _logger_instance->critical(format_str, std::forward(args)...); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_error(format_str, std::forward(args)...); + } + else + { + _logger_instance->critical(format_str, std::forward(args)...); + } } } void critical(spdlog::string_view_t msg) { - if (_closed == true) return; - - if (twine::is_current_thread_realtime()) + if (!_closed) { - _rt_logger->log_error(msg); - } - else - { - _logger_instance->critical(msg); + if (twine::is_current_thread_realtime()) + { + _rt_logger->log_error(msg); + } + else + { + _logger_instance->critical(msg); + } } } diff --git a/include/elklog/rtlogmessage.h b/include/elklog/rtlogmessage.h index c01b44d..04e7895 100644 --- a/include/elklog/rtlogmessage.h +++ b/include/elklog/rtlogmessage.h @@ -107,9 +107,8 @@ class RtLogMessage _level = level; _timestamp = timestamp; auto end = fmt::format_to_n(_buffer.data(), buffer_len - 1, format_str, std::forward(args)...); - //auto end = fmt::format_to(_buffer.data(), format_str, args...); - //Add null-termination character + // Add null-termination character *end.out = '\0'; _length = static_cast(std::distance(_buffer.data(), end.out)); } @@ -119,11 +118,13 @@ class RtLogMessage { _level = level; _timestamp = timestamp; + // This specialization is for single string messages without need for formatting + // In this case we simply copy the string to the buffer without invoking fmt::format() auto end = std::copy_n(msg.begin(), std::min(msg.size(), buffer_len - 1), _buffer.data()); - //Add null-termination character + // Add null-termination character *end = '\0'; - _length = msg.size() + 1; + _length = static_cast(msg.size()) + 1; } From 0196aeb55175ad43618eb91b73477f862e39bc71 Mon Sep 17 00:00:00 2001 From: Gustav Andersson Date: Fri, 22 Mar 2024 11:46:03 +0100 Subject: [PATCH 5/5] Adding unittest for unformatted message --- include/elklog/rtlogger.h | 6 +++--- test/unittests/rtlogmessage_test.cpp | 14 +++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/include/elklog/rtlogger.h b/include/elklog/rtlogger.h index c5ec9ab..8ea2d96 100644 --- a/include/elklog/rtlogger.h +++ b/include/elklog/rtlogger.h @@ -127,7 +127,7 @@ class RtLogger template void log_debug(spdlog::format_string_t format_str, Args&&... args) { - log(format_str, args...); + log(format_str, std::forward(args)...); } void log_debug(spdlog::string_view_t msg) @@ -138,7 +138,7 @@ class RtLogger template void log_info(spdlog::format_string_t format_str, Args&&... args) { - log(format_str, args...); + log(format_str, std::forward(args)...); } void log_info(spdlog::string_view_t msg) @@ -160,7 +160,7 @@ class RtLogger template void log_error(spdlog::format_string_t format_str, Args&&... args) { - log(format_str, args...); + log(format_str, std::forward(args)...); } void log_error(spdlog::string_view_t msg) diff --git a/test/unittests/rtlogmessage_test.cpp b/test/unittests/rtlogmessage_test.cpp index 5b8d162..ab57e20 100644 --- a/test/unittests/rtlogmessage_test.cpp +++ b/test/unittests/rtlogmessage_test.cpp @@ -14,7 +14,7 @@ TEST(RtLogMessageTest, TestCreation) ASSERT_STREQ("", module_under_test.message()); } -TEST(RtLogMessageTest, TestCopyingAndAssignment) +TEST(RtLogMessageTest, TestCopyingAndAssignmentFormatted) { RtLogMessage<512> module_under_test; @@ -33,6 +33,18 @@ TEST(RtLogMessageTest, TestCopyingAndAssignment) EXPECT_STREQ(module_under_test.message(), msg_2.message()); } +TEST(RtLogMessageTest, TestCopyingAndAssignment) +{ + RtLogMessage<512> module_under_test; + + module_under_test.set_message(RtLogLevel::INFO, std::chrono::nanoseconds(456), "Test single message"); + + EXPECT_STREQ("Test single message", module_under_test.message()); + EXPECT_EQ(std::chrono::nanoseconds(456), module_under_test.timestamp()); + EXPECT_EQ(RtLogLevel::INFO, module_under_test.level()); + EXPECT_EQ(20, module_under_test.length()); +} + TEST(RtLogMessageTest, TestMaxSize) { RtLogMessage<24> module_under_test;