From 35d79e12528379ba97ecae33876e3863229ac3e1 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 22 May 2020 13:58:29 +0300 Subject: [PATCH] fmt-style logging, part 1 --- .gitmodules | 3 ++ base/common/CMakeLists.txt | 1 + base/common/logger_useful.h | 47 +++++++++++++++++++++++------ contrib/CMakeLists.txt | 2 ++ contrib/fmtlib | 1 + contrib/fmtlib-cmake/CMakeLists.txt | 20 ++++++++++++ src/IO/S3Common.cpp | 4 +-- src/Storages/Kafka/StorageKafka.cpp | 4 +-- 8 files changed, 68 insertions(+), 14 deletions(-) create mode 160000 contrib/fmtlib create mode 100644 contrib/fmtlib-cmake/CMakeLists.txt diff --git a/.gitmodules b/.gitmodules index f7a16b84d37..7f5d1307a6e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -157,3 +157,6 @@ [submodule "contrib/openldap"] path = contrib/openldap url = https://github.com/openldap/openldap.git +[submodule "contrib/fmtlib"] + path = contrib/fmtlib + url = https://github.com/fmtlib/fmt.git diff --git a/base/common/CMakeLists.txt b/base/common/CMakeLists.txt index c3eddb4d26c..9b827cdb468 100644 --- a/base/common/CMakeLists.txt +++ b/base/common/CMakeLists.txt @@ -79,6 +79,7 @@ target_link_libraries (common Poco::Util Poco::Foundation replxx + fmt PRIVATE cctz diff --git a/base/common/logger_useful.h b/base/common/logger_useful.h index ea1a25cc8fa..4cedb3dfc30 100644 --- a/base/common/logger_useful.h +++ b/base/common/logger_useful.h @@ -3,15 +3,14 @@ /// Macros for convenient usage of Poco logger. #include +#include #include #include #include #include -#ifndef QUERY_PREVIEW_LENGTH -#define QUERY_PREVIEW_LENGTH 160 -#endif +/// TODO Remove this. using Poco::Logger; using Poco::Message; using DB::LogsLevel; @@ -19,7 +18,7 @@ using DB::CurrentThread; /// Logs a message to a specified logger with that level. -#define LOG_SIMPLE(logger, message, priority, PRIORITY) do \ +#define LOG_IMPL(logger, priority, PRIORITY, message) do \ { \ const bool is_clients_log = (CurrentThread::getGroup() != nullptr) && \ (CurrentThread::getGroup()->client_logs_level >= (priority)); \ @@ -41,10 +40,38 @@ using DB::CurrentThread; } while (false) -#define LOG_TRACE(logger, message) LOG_SIMPLE(logger, message, LogsLevel::trace, Message::PRIO_TRACE) -#define LOG_DEBUG(logger, message) LOG_SIMPLE(logger, message, LogsLevel::debug, Message::PRIO_DEBUG) -#define LOG_INFO(logger, message) LOG_SIMPLE(logger, message, LogsLevel::information, Message::PRIO_INFORMATION) -#define LOG_WARNING(logger, message) LOG_SIMPLE(logger, message, LogsLevel::warning, Message::PRIO_WARNING) -#define LOG_ERROR(logger, message) LOG_SIMPLE(logger, message, LogsLevel::error, Message::PRIO_ERROR) -#define LOG_FATAL(logger, message) LOG_SIMPLE(logger, message, LogsLevel::error, Message::PRIO_FATAL) +#define LOG_TRACE(logger, message) LOG_IMPL(logger, LogsLevel::trace, Message::PRIO_TRACE, message) +#define LOG_DEBUG(logger, message) LOG_IMPL(logger, LogsLevel::debug, Message::PRIO_DEBUG, message) +#define LOG_INFO(logger, message) LOG_IMPL(logger, LogsLevel::information, Message::PRIO_INFORMATION, message) +#define LOG_WARNING(logger, message) LOG_IMPL(logger, LogsLevel::warning, Message::PRIO_WARNING, message) +#define LOG_ERROR(logger, message) LOG_IMPL(logger, LogsLevel::error, Message::PRIO_ERROR, message) +#define LOG_FATAL(logger, message) LOG_IMPL(logger, LogsLevel::error, Message::PRIO_FATAL, message) + +#define LOG_IMPL_FORMATTED(logger, priority, PRIORITY, message, ...) do \ +{ \ + const bool is_clients_log = (CurrentThread::getGroup() != nullptr) && \ + (CurrentThread::getGroup()->client_logs_level >= (priority)); \ + if ((logger)->is((PRIORITY)) || is_clients_log) \ + { \ + std::string formatted_message = fmt::format(message, __VA_ARGS__); \ + if (auto channel = (logger)->getChannel()) \ + { \ + std::string file_function; \ + file_function += __FILE__; \ + file_function += "; "; \ + file_function += __PRETTY_FUNCTION__; \ + Message poco_message((logger)->name(), formatted_message, \ + (PRIORITY), file_function.c_str(), __LINE__); \ + channel->log(poco_message); \ + } \ + } \ +} while (false) + + +#define LOG_TRACE_FORMATTED(logger, message, ...) LOG_IMPL_FORMATTED(logger, LogsLevel::trace, Message::PRIO_TRACE, message, __VA_ARGS__) +#define LOG_DEBUG_FORMATTED(logger, message, ...) LOG_IMPL_FORMATTED(logger, LogsLevel::debug, Message::PRIO_DEBUG, message, __VA_ARGS__) +#define LOG_INFO_FORMATTED(logger, message, ...) LOG_IMPL_FORMATTED(logger, LogsLevel::information, Message::PRIO_INFORMATION, message, __VA_ARGS__) +#define LOG_WARNING_FORMATTED(logger, message, ...) LOG_IMPL_FORMATTED(logger, LogsLevel::warning, Message::PRIO_WARNING, message, __VA_ARGS__) +#define LOG_ERROR_FORMATTED(logger, message, ...) LOG_IMPL_FORMATTED(logger, LogsLevel::error, Message::PRIO_ERROR, message, __VA_ARGS__) +#define LOG_FATAL_FORMATTED(logger, message, ...) LOG_IMPL_FORMATTED(logger, LogsLevel::error, Message::PRIO_FATAL, message, __VA_ARGS__) diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index 1031285eac7..99f7be2cbb7 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -317,3 +317,5 @@ endif() if (USE_FASTOPS) add_subdirectory (fastops-cmake) endif() + +add_subdirectory (fmtlib-cmake) diff --git a/contrib/fmtlib b/contrib/fmtlib new file mode 160000 index 00000000000..297c3b2ed55 --- /dev/null +++ b/contrib/fmtlib @@ -0,0 +1 @@ +Subproject commit 297c3b2ed551a4989826fc8c4780bf533e964bd9 diff --git a/contrib/fmtlib-cmake/CMakeLists.txt b/contrib/fmtlib-cmake/CMakeLists.txt new file mode 100644 index 00000000000..f3bf73d7dbc --- /dev/null +++ b/contrib/fmtlib-cmake/CMakeLists.txt @@ -0,0 +1,20 @@ +set (SRCS + ../fmtlib/src/format.cc + ../fmtlib/src/os.cc + + ../fmtlib/include/fmt/chrono.h + ../fmtlib/include/fmt/color.h + ../fmtlib/include/fmt/compile.h + ../fmtlib/include/fmt/core.h + ../fmtlib/include/fmt/format.h + ../fmtlib/include/fmt/format-inl.h + ../fmtlib/include/fmt/locale.h + ../fmtlib/include/fmt/os.h + ../fmtlib/include/fmt/ostream.h + ../fmtlib/include/fmt/posix.h + ../fmtlib/include/fmt/printf.h + ../fmtlib/include/fmt/ranges.h +) + +add_library(fmt ${SRCS}) +target_include_directories(fmt SYSTEM PUBLIC ../fmtlib/include) diff --git a/src/IO/S3Common.cpp b/src/IO/S3Common.cpp index a6b518afcdc..0cf8cf22326 100644 --- a/src/IO/S3Common.cpp +++ b/src/IO/S3Common.cpp @@ -40,13 +40,13 @@ public: void Log(Aws::Utils::Logging::LogLevel log_level, const char * tag, const char * format_str, ...) final // NOLINT { const auto & [level, prio] = convertLogLevel(log_level); - LOG_SIMPLE(log, std::string(tag) + ": " + format_str, level, prio); + LOG_IMPL(log, level, prio, std::string(tag) + ": " + format_str); } void LogStream(Aws::Utils::Logging::LogLevel log_level, const char * tag, const Aws::OStringStream & message_stream) final { const auto & [level, prio] = convertLogLevel(log_level); - LOG_SIMPLE(log, std::string(tag) + ": " + message_stream.str(), level, prio); + LOG_IMPL(log, level, prio, std::string(tag) + ": " + message_stream.str()); } void Flush() final {} diff --git a/src/Storages/Kafka/StorageKafka.cpp b/src/Storages/Kafka/StorageKafka.cpp index 793a9a29676..6d5d569ce5d 100644 --- a/src/Storages/Kafka/StorageKafka.cpp +++ b/src/Storages/Kafka/StorageKafka.cpp @@ -325,7 +325,7 @@ void StorageKafka::updateConfiguration(cppkafka::Configuration & conf) conf.set_log_callback([this](cppkafka::KafkaHandleBase &, int level, const std::string & /* facility */, const std::string & message) { auto [poco_level, client_logs_level] = parseSyslogLevel(level); - LOG_SIMPLE(log, message, client_logs_level, poco_level); + LOG_IMPL(log, client_logs_level, poco_level, message); }); // Configure interceptor to change thread name @@ -334,7 +334,7 @@ void StorageKafka::updateConfiguration(cppkafka::Configuration & conf) // XXX: rdkafka uses pthread_set_name_np(), but glibc-compatibliity overrides it to noop. { // This should be safe, since we wait the rdkafka object anyway. - void * self = reinterpret_cast(this); + void * self = static_cast(this); int status;