From 14a0d796c0601cd731395a0417cd2ae706620341 Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Fri, 25 Jul 2025 15:09:22 +0800 Subject: [PATCH 1/5] feat: add a plugable logger interface. --- src/iceberg/CMakeLists.txt | 3 +- src/iceberg/util/logger.cc | 135 ++++++++++++++++++ src/iceberg/util/logger.h | 285 +++++++++++++++++++++++++++++++++++++ test/CMakeLists.txt | 3 +- test/logger_test.cc | 133 +++++++++++++++++ 5 files changed, 557 insertions(+), 2 deletions(-) create mode 100644 src/iceberg/util/logger.cc create mode 100644 src/iceberg/util/logger.h create mode 100644 test/logger_test.cc diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 0900068a6..97ddb3c63 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -49,7 +49,8 @@ set(ICEBERG_SOURCES arrow_c_data_guard_internal.cc util/murmurhash3_internal.cc util/timepoint.cc - util/gzip_internal.cc) + util/gzip_internal.cc + util/logger.cc) set(ICEBERG_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/util/logger.cc b/src/iceberg/util/logger.cc new file mode 100644 index 000000000..a70f6928d --- /dev/null +++ b/src/iceberg/util/logger.cc @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/util/logger.h" + +#include +#include + +namespace iceberg { + +namespace { + +/// \brief Convert iceberg LogLevel to spdlog level +spdlog::level::level_enum ToSpdlogLevel(LogLevel level) { + switch (level) { + case LogLevel::kTrace: + return spdlog::level::trace; + case LogLevel::kDebug: + return spdlog::level::debug; + case LogLevel::kInfo: + return spdlog::level::info; + case LogLevel::kWarn: + return spdlog::level::warn; + case LogLevel::kError: + return spdlog::level::err; + case LogLevel::kCritical: + return spdlog::level::critical; + case LogLevel::kOff: + return spdlog::level::off; + default: + return spdlog::level::info; + } +} + +/** + * \brief Convert spdlog level to iceberg LogLevel + */ +LogLevel FromSpdlogLevel(spdlog::level::level_enum level) { + switch (level) { + case spdlog::level::trace: + return LogLevel::kTrace; + case spdlog::level::debug: + return LogLevel::kDebug; + case spdlog::level::info: + return LogLevel::kInfo; + case spdlog::level::warn: + return LogLevel::kWarn; + case spdlog::level::err: + return LogLevel::kError; + case spdlog::level::critical: + return LogLevel::kCritical; + case spdlog::level::off: + return LogLevel::kOff; + default: + return LogLevel::kInfo; + } +} + +} // namespace + +// SpdlogLogger implementation +SpdlogLogger::SpdlogLogger(std::string_view logger_name) { + auto spdlog_logger = spdlog::get(std::string(logger_name)); + if (!spdlog_logger) { + spdlog_logger = spdlog::stdout_color_mt(std::string(logger_name)); + } + logger_ = std::static_pointer_cast(spdlog_logger); + current_level_ = FromSpdlogLevel(spdlog_logger->level()); +} + +SpdlogLogger::SpdlogLogger(std::shared_ptr spdlog_logger) + : logger_(std::move(spdlog_logger)) { + auto typed_logger = std::static_pointer_cast(logger_); + current_level_ = FromSpdlogLevel(typed_logger->level()); +} + +bool SpdlogLogger::ShouldLogImpl(LogLevel level) const noexcept { + return level >= current_level_; +} + +void SpdlogLogger::LogRawImpl(LogLevel level, const std::string& message) const { + auto typed_logger = std::static_pointer_cast(logger_); + auto spdlog_level = ToSpdlogLevel(level); + typed_logger->log(spdlog_level, message); +} + +void SpdlogLogger::LogWithLocationRawImpl(LogLevel level, const std::string& message, + const std::source_location& location) const { + auto typed_logger = std::static_pointer_cast(logger_); + auto spdlog_level = ToSpdlogLevel(level); + + // Add source location information + std::string full_message = + std::format("[{}:{}:{}] {}", location.file_name(), location.line(), + location.function_name(), message); + + typed_logger->log(spdlog_level, full_message); +} + +void SpdlogLogger::SetLevelImpl(LogLevel level) { + current_level_ = level; + auto typed_logger = std::static_pointer_cast(logger_); + typed_logger->set_level(ToSpdlogLevel(level)); +} + +LogLevel SpdlogLogger::GetLevelImpl() const noexcept { return current_level_; } + +// LoggerRegistry implementation +LoggerRegistry& LoggerRegistry::Instance() { + static LoggerRegistry instance; + return instance; +} + +void LoggerRegistry::InitializeDefault(std::string_view logger_name) { + auto spdlog_logger = std::make_shared(logger_name); + SetDefaultLogger(spdlog_logger); +} + +} // namespace iceberg diff --git a/src/iceberg/util/logger.h b/src/iceberg/util/logger.h new file mode 100644 index 000000000..62c1a6836 --- /dev/null +++ b/src/iceberg/util/logger.h @@ -0,0 +1,285 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +namespace iceberg { + +/// \brief Log levels for the iceberg logger system +enum class LogLevel : int { + kTrace = 0, + kDebug = 1, + kInfo = 2, + kWarn = 3, + kError = 4, + kCritical = 5, + kOff = 6 +}; + +/// \brief Convert log level to string representation +constexpr std::string_view LogLevelToString(LogLevel level) { + switch (level) { + case LogLevel::kTrace: + return "TRACE"; + case LogLevel::kDebug: + return "DEBUG"; + case LogLevel::kInfo: + return "INFO"; + case LogLevel::kWarn: + return "WARN"; + case LogLevel::kError: + return "ERROR"; + case LogLevel::kCritical: + return "CRITICAL"; + case LogLevel::kOff: + return "OFF"; + default: + return "UNKNOWN"; + } +} + +/// \brief Logger interface that uses CRTP to avoid virtual function overhead +/// \tparam Derived The concrete logger implementation +template +class LoggerInterface { + public: + /// \brief Check if a log level is enabled + bool ShouldLog(LogLevel level) const noexcept { + return static_cast(this)->ShouldLogImpl(level); + } + + /// \brief Log a message with the specified level + template + void Log(LogLevel level, std::string_view format_str, Args&&... args) const { + if (ShouldLog(level)) { + static_cast(this)->LogImpl(level, format_str, + std::forward(args)...); + } + } + + /// \brief Log a message with source location information + template + void LogWithLocation(LogLevel level, std::string_view format_str, + const std::source_location& location, Args&&... args) const { + if (ShouldLog(level)) { + static_cast(this)->LogWithLocationImpl(level, format_str, location, + std::forward(args)...); + } + } + + /// \brief Set the minimum log level + void SetLevel(LogLevel level) { static_cast(this)->SetLevelImpl(level); } + + /// \brief Get the current minimum log level + LogLevel GetLevel() const noexcept { + return static_cast(this)->GetLevelImpl(); + } + + // Convenience methods for different log levels + template + void Trace(std::string_view format_str, Args&&... args) const { + Log(LogLevel::kTrace, format_str, std::forward(args)...); + } + + template + void Debug(std::string_view format_str, Args&&... args) const { + Log(LogLevel::kDebug, format_str, std::forward(args)...); + } + + template + void Info(std::string_view format_str, Args&&... args) const { + Log(LogLevel::kInfo, format_str, std::forward(args)...); + } + + template + void Warn(std::string_view format_str, Args&&... args) const { + Log(LogLevel::kWarn, format_str, std::forward(args)...); + } + + template + void Error(std::string_view format_str, Args&&... args) const { + Log(LogLevel::kError, format_str, std::forward(args)...); + } + + template + void Critical(std::string_view format_str, Args&&... args) const { + Log(LogLevel::kCritical, format_str, std::forward(args)...); + } + + protected: + LoggerInterface() = default; + ~LoggerInterface() = default; +}; + +/// \brief Default spdlog-based logger implementation +class SpdlogLogger : public LoggerInterface { + public: + /// \brief Create a new spdlog logger with the given name + explicit SpdlogLogger(std::string_view logger_name = "iceberg"); + + /// \brief Create a spdlog logger from an existing spdlog::logger + explicit SpdlogLogger(std::shared_ptr spdlog_logger); + + // Implementation methods required by LoggerInterface + bool ShouldLogImpl(LogLevel level) const noexcept; + + template + void LogImpl(LogLevel level, std::string_view format_str, Args&&... args) const { + if constexpr (sizeof...(args) > 0) { + std::string formatted_message = + std::vformat(format_str, std::make_format_args(args...)); + LogRawImpl(level, formatted_message); + } else { + LogRawImpl(level, std::string(format_str)); + } + } + + template + void LogWithLocationImpl(LogLevel level, std::string_view format_str, + const std::source_location& location, Args&&... args) const { + std::string message; + if constexpr (sizeof...(args) > 0) { + message = std::vformat(format_str, std::make_format_args(args...)); + } else { + message = std::string(format_str); + } + LogWithLocationRawImpl(level, message, location); + } + + void SetLevelImpl(LogLevel level); + LogLevel GetLevelImpl() const noexcept; + + /// \brief Get the underlying spdlog logger (for advanced usage) + std::shared_ptr GetUnderlyingLogger() const { return logger_; } + + private: + void LogRawImpl(LogLevel level, const std::string& message) const; + void LogWithLocationRawImpl(LogLevel level, const std::string& message, + const std::source_location& location) const; + + std::shared_ptr logger_; // Type-erased spdlog::logger + LogLevel current_level_ = LogLevel::kInfo; +}; + +/// \brief No-op logger implementation for performance-critical scenarios +class NullLogger : public LoggerInterface { + public: + bool ShouldLogImpl(LogLevel) const noexcept { return false; } + + template + void LogImpl(LogLevel, std::string_view, Args&&...) const noexcept {} + + template + void LogWithLocationImpl(LogLevel, std::string_view, const std::source_location&, + Args&&...) const noexcept {} + + void SetLevelImpl(LogLevel) noexcept {} + LogLevel GetLevelImpl() const noexcept { return LogLevel::kOff; } +}; + +/// \brief Global logger registry for managing logger instances +class LoggerRegistry { + public: + /// \brief Get the singleton instance of the logger registry + static LoggerRegistry& Instance(); + + /// \brief Set the default logger implementation + /// + /// \tparam LoggerImpl The logger implementation type + /// \param logger The logger instance to set as default + template + void SetDefaultLogger(std::shared_ptr logger) { + static_assert(std::is_base_of_v, LoggerImpl>, + "LoggerImpl must inherit from LoggerInterface"); + default_logger_ = std::static_pointer_cast(logger); + log_func_ = [logger](LogLevel level, std::string_view format_str, + const std::string& formatted_message) { + if (logger->ShouldLog(level)) { + logger->LogImpl(level, "{}", formatted_message); + } + }; + } + + /// \brief Get the default logger + /// + /// \tparam LoggerImpl The expected logger implementation type + /// \return Shared pointer to the logger, or nullptr if type doesn't match + template + std::shared_ptr GetDefaultLogger() const { + return std::static_pointer_cast(default_logger_); + } + + /// \brief Log using the default logger + template + void Log(LogLevel level, std::string_view format_str, Args&&... args) const { + if (log_func_) { + std::string formatted_message; + if constexpr (sizeof...(args) > 0) { + formatted_message = std::vformat(format_str, std::make_format_args(args...)); + } else { + formatted_message = std::string(format_str); + } + log_func_(level, format_str, formatted_message); + } + } + + /// \brief Initialize with default spdlog logger + void InitializeDefault(std::string_view logger_name = "iceberg"); + + private: + LoggerRegistry() = default; + + std::shared_ptr default_logger_; + std::function log_func_; +}; + +/// \brief Convenience macros for logging with automatic source location +#define LOG_WITH_LOCATION(level, format_str, ...) \ + do { \ + ::iceberg::LoggerRegistry::Instance().Log(level, \ + format_str __VA_OPT__(, ) __VA_ARGS__); \ + } while (0) + +#define LOG_TRACE(format_str, ...) \ + LOG_WITH_LOCATION(::iceberg::LogLevel::kTrace, format_str __VA_OPT__(, ) __VA_ARGS__) + +#define LOG_DEBUG(format_str, ...) \ + LOG_WITH_LOCATION(::iceberg::LogLevel::kDebug, format_str __VA_OPT__(, ) __VA_ARGS__) + +#define LOG_INFO(format_str, ...) \ + LOG_WITH_LOCATION(::iceberg::LogLevel::kInfo, format_str __VA_OPT__(, ) __VA_ARGS__) + +#define LOG_WARN(format_str, ...) \ + LOG_WITH_LOCATION(::iceberg::LogLevel::kWarn, format_str __VA_OPT__(, ) __VA_ARGS__) + +#define LOG_ERROR(format_str, ...) \ + LOG_WITH_LOCATION(::iceberg::LogLevel::kError, format_str __VA_OPT__(, ) __VA_ARGS__) + +#define LOG_CRITICAL(format_str, ...) \ + LOG_WITH_LOCATION(::iceberg::LogLevel::kCritical, format_str __VA_OPT__(, ) __VA_ARGS__) + +} // namespace iceberg diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0597d766d..45c89e1f4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -66,7 +66,8 @@ target_link_libraries(json_serde_test PRIVATE iceberg_static GTest::gtest_main add_test(NAME json_serde_test COMMAND json_serde_test) add_executable(util_test) -target_sources(util_test PRIVATE formatter_test.cc config_test.cc visit_type_test.cc) +target_sources(util_test PRIVATE formatter_test.cc config_test.cc visit_type_test.cc + logger_test.cc) target_link_libraries(util_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) add_test(NAME util_test COMMAND util_test) diff --git a/test/logger_test.cc b/test/logger_test.cc new file mode 100644 index 000000000..cc6594047 --- /dev/null +++ b/test/logger_test.cc @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/util/logger.h" + +#include + +#include + +namespace iceberg { + +/// \brief Example custom logger implementation using std::cout for testing +/// +/// This shows how downstream projects can implement their own logger +/// by inheriting from LoggerInterface and implementing the required methods. +class StdoutLogger : public LoggerInterface { + public: + explicit StdoutLogger(LogLevel min_level = LogLevel::kInfo) : min_level_(min_level) {} + + // Required implementation methods + bool ShouldLogImpl(LogLevel level) const noexcept { return level >= min_level_; } + + template + void LogImpl(LogLevel level, std::string_view format_str, Args&&... args) const { + if constexpr (sizeof...(args) > 0) { + std::string formatted_message = + std::vformat(format_str, std::make_format_args(args...)); + LogRawImpl(level, formatted_message); + } else { + LogRawImpl(level, std::string(format_str)); + } + } + + template + void LogWithLocationImpl(LogLevel level, std::string_view format_str, + const std::source_location& location, Args&&... args) const { + std::string message; + if constexpr (sizeof...(args) > 0) { + message = std::vformat(format_str, std::make_format_args(args...)); + } else { + message = std::string(format_str); + } + + std::string full_message = + std::format("[{}:{}] {}", location.file_name(), location.line(), message); + LogRawImpl(level, full_message); + } + + void SetLevelImpl(LogLevel level) noexcept { min_level_ = level; } + + LogLevel GetLevelImpl() const noexcept { return min_level_; } + + private: + void LogRawImpl(LogLevel level, const std::string& message) const { + std::cout << "[" << LogLevelToString(level) << "] " << message << std::endl; + } + + LogLevel min_level_; +}; + +class LoggerTest : public ::testing::Test { + protected: + void SetUp() override { + // Each test starts with a fresh logger registry + } + + void TearDown() override { + // Reset to default state + LoggerRegistry::Instance().InitializeDefault("test_logger"); + } +}; + +TEST_F(LoggerTest, DefaultSpdlogLogger) { + // Initialize with default spdlog logger + LoggerRegistry::Instance().InitializeDefault("test_logger"); + + // Test basic logging functionality + LOG_INFO("This is an info message"); + LOG_DEBUG("This is a debug message with value: {}", 42); + LOG_WARN("This is a warning message"); + LOG_ERROR("This is an error message"); + + // The test passes if no exceptions are thrown + SUCCEED(); +} + +TEST_F(LoggerTest, CustomStdoutLogger) { + // Create and register a custom logger + auto custom_logger = std::make_shared(LogLevel::kDebug); + LoggerRegistry::Instance().SetDefaultLogger(custom_logger); + + // Test logging with custom logger + LOG_DEBUG("Debug message from custom logger"); + LOG_INFO("Info message with parameter: {}", "test"); + LOG_WARN("Warning from custom logger"); + + SUCCEED(); +} + +TEST_F(LoggerTest, LoggerLevels) { + auto logger = std::make_shared("test_level_logger"); + + // Test level filtering + logger->SetLevel(LogLevel::kWarn); + EXPECT_EQ(logger->GetLevel(), LogLevel::kWarn); + + // These should be filtered out + EXPECT_FALSE(logger->ShouldLog(LogLevel::kTrace)); + EXPECT_FALSE(logger->ShouldLog(LogLevel::kDebug)); + EXPECT_FALSE(logger->ShouldLog(LogLevel::kInfo)); + + // These should pass through + EXPECT_TRUE(logger->ShouldLog(LogLevel::kWarn)); + EXPECT_TRUE(logger->ShouldLog(LogLevel::kError)); + EXPECT_TRUE(logger->ShouldLog(LogLevel::kCritical)); +} +} // namespace iceberg From 2126cd6c0d29b0047b479175cc4cdac433a40b36 Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Fri, 25 Jul 2025 15:46:13 +0800 Subject: [PATCH 2/5] try to fix msvc build. --- test/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 45c89e1f4..b1f4e85db 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -69,6 +69,9 @@ add_executable(util_test) target_sources(util_test PRIVATE formatter_test.cc config_test.cc visit_type_test.cc logger_test.cc) target_link_libraries(util_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock) +if(MSVC) + target_compile_options(util_test PRIVATE /Zc:preprocessor) +endif() add_test(NAME util_test COMMAND util_test) if(ICEBERG_BUILD_BUNDLE) From cc320fa80892881fa953b000acf2b1a86072827b Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Mon, 28 Jul 2025 16:08:59 +0800 Subject: [PATCH 3/5] Introduce Logger concept and eliminate use of std::function for logger injection. --- src/iceberg/util/logger.h | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/iceberg/util/logger.h b/src/iceberg/util/logger.h index 62c1a6836..1d0dcc4b2 100644 --- a/src/iceberg/util/logger.h +++ b/src/iceberg/util/logger.h @@ -20,7 +20,6 @@ #pragma once #include -#include #include #include #include @@ -135,6 +134,10 @@ class LoggerInterface { ~LoggerInterface() = default; }; +/// \brief Concept to constrain types that implement the Logger interface +template +concept Logger = std::is_base_of_v, T>; + /// \brief Default spdlog-based logger implementation class SpdlogLogger : public LoggerInterface { public: @@ -211,15 +214,13 @@ class LoggerRegistry { /// /// \tparam LoggerImpl The logger implementation type /// \param logger The logger instance to set as default - template + template void SetDefaultLogger(std::shared_ptr logger) { - static_assert(std::is_base_of_v, LoggerImpl>, - "LoggerImpl must inherit from LoggerInterface"); default_logger_ = std::static_pointer_cast(logger); - log_func_ = [logger](LogLevel level, std::string_view format_str, - const std::string& formatted_message) { - if (logger->ShouldLog(level)) { - logger->LogImpl(level, "{}", formatted_message); + log_func_ = [](const void* logger_ptr, LogLevel level, const std::string& message) { + auto* typed_logger = static_cast(logger_ptr); + if (typed_logger->ShouldLog(level)) { + typed_logger->LogImpl(level, "{}", message); } }; } @@ -228,7 +229,7 @@ class LoggerRegistry { /// /// \tparam LoggerImpl The expected logger implementation type /// \return Shared pointer to the logger, or nullptr if type doesn't match - template + template std::shared_ptr GetDefaultLogger() const { return std::static_pointer_cast(default_logger_); } @@ -236,14 +237,14 @@ class LoggerRegistry { /// \brief Log using the default logger template void Log(LogLevel level, std::string_view format_str, Args&&... args) const { - if (log_func_) { + if (default_logger_ && log_func_) { std::string formatted_message; if constexpr (sizeof...(args) > 0) { formatted_message = std::vformat(format_str, std::make_format_args(args...)); } else { formatted_message = std::string(format_str); } - log_func_(level, format_str, formatted_message); + log_func_(default_logger_.get(), level, formatted_message); } } @@ -254,7 +255,7 @@ class LoggerRegistry { LoggerRegistry() = default; std::shared_ptr default_logger_; - std::function log_func_; + void (*log_func_)(const void*, LogLevel, const std::string&) = nullptr; }; /// \brief Convenience macros for logging with automatic source location From 9f1e7e87a1e3751a2a6138b2b1ec13dd613f3065 Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Tue, 5 Aug 2025 13:08:17 +0800 Subject: [PATCH 4/5] simplify logging and fix review comments. --- src/iceberg/util/logger.cc | 12 +- src/iceberg/util/logger.h | 223 ++++++++++--------------------- src/iceberg/util/spdlog_logger.h | 69 ++++++++++ test/logger_test.cc | 41 +++--- 4 files changed, 162 insertions(+), 183 deletions(-) create mode 100644 src/iceberg/util/spdlog_logger.h diff --git a/src/iceberg/util/logger.cc b/src/iceberg/util/logger.cc index a70f6928d..fa9a0ca8f 100644 --- a/src/iceberg/util/logger.cc +++ b/src/iceberg/util/logger.cc @@ -22,6 +22,8 @@ #include #include +#include "iceberg/util/spdlog_logger.h" + namespace iceberg { namespace { @@ -94,14 +96,8 @@ bool SpdlogLogger::ShouldLogImpl(LogLevel level) const noexcept { return level >= current_level_; } -void SpdlogLogger::LogRawImpl(LogLevel level, const std::string& message) const { - auto typed_logger = std::static_pointer_cast(logger_); - auto spdlog_level = ToSpdlogLevel(level); - typed_logger->log(spdlog_level, message); -} - -void SpdlogLogger::LogWithLocationRawImpl(LogLevel level, const std::string& message, - const std::source_location& location) const { +void SpdlogLogger::LogRawImpl(LogLevel level, const std::source_location& location, + const std::string& message) const { auto typed_logger = std::static_pointer_cast(logger_); auto spdlog_level = ToSpdlogLevel(level); diff --git a/src/iceberg/util/logger.h b/src/iceberg/util/logger.h index 1d0dcc4b2..1c9c6b98f 100644 --- a/src/iceberg/util/logger.h +++ b/src/iceberg/util/logger.h @@ -20,12 +20,15 @@ #pragma once #include +#include #include #include #include #include #include +#include + namespace iceberg { /// \brief Log levels for the iceberg logger system @@ -40,7 +43,7 @@ enum class LogLevel : int { }; /// \brief Convert log level to string representation -constexpr std::string_view LogLevelToString(LogLevel level) { +ICEBERG_EXPORT constexpr std::string_view LogLevelToString(LogLevel level) { switch (level) { case LogLevel::kTrace: return "TRACE"; @@ -64,148 +67,51 @@ constexpr std::string_view LogLevelToString(LogLevel level) { /// \brief Logger interface that uses CRTP to avoid virtual function overhead /// \tparam Derived The concrete logger implementation template -class LoggerInterface { +class ICEBERG_EXPORT LoggerInterface { public: /// \brief Check if a log level is enabled bool ShouldLog(LogLevel level) const noexcept { - return static_cast(this)->ShouldLogImpl(level); + return derived()->ShouldLogImpl(level); } /// \brief Log a message with the specified level template - void Log(LogLevel level, std::string_view format_str, Args&&... args) const { - if (ShouldLog(level)) { - static_cast(this)->LogImpl(level, format_str, - std::forward(args)...); - } - } - - /// \brief Log a message with source location information - template - void LogWithLocation(LogLevel level, std::string_view format_str, - const std::source_location& location, Args&&... args) const { - if (ShouldLog(level)) { - static_cast(this)->LogWithLocationImpl(level, format_str, location, - std::forward(args)...); - } + void Log(LogLevel level, const std::source_location& location, + std::string_view format_str, Args&&... args) const { + derived()->LogImpl(level, location, format_str, std::forward(args)...); } /// \brief Set the minimum log level - void SetLevel(LogLevel level) { static_cast(this)->SetLevelImpl(level); } + void SetLevel(LogLevel level) { derived()->SetLevelImpl(level); } /// \brief Get the current minimum log level - LogLevel GetLevel() const noexcept { - return static_cast(this)->GetLevelImpl(); - } - - // Convenience methods for different log levels - template - void Trace(std::string_view format_str, Args&&... args) const { - Log(LogLevel::kTrace, format_str, std::forward(args)...); - } - - template - void Debug(std::string_view format_str, Args&&... args) const { - Log(LogLevel::kDebug, format_str, std::forward(args)...); - } - - template - void Info(std::string_view format_str, Args&&... args) const { - Log(LogLevel::kInfo, format_str, std::forward(args)...); - } - - template - void Warn(std::string_view format_str, Args&&... args) const { - Log(LogLevel::kWarn, format_str, std::forward(args)...); - } - - template - void Error(std::string_view format_str, Args&&... args) const { - Log(LogLevel::kError, format_str, std::forward(args)...); - } - - template - void Critical(std::string_view format_str, Args&&... args) const { - Log(LogLevel::kCritical, format_str, std::forward(args)...); - } + LogLevel GetLevel() const noexcept { return derived()->GetLevelImpl(); } protected: LoggerInterface() = default; ~LoggerInterface() = default; -}; - -/// \brief Concept to constrain types that implement the Logger interface -template -concept Logger = std::is_base_of_v, T>; - -/// \brief Default spdlog-based logger implementation -class SpdlogLogger : public LoggerInterface { - public: - /// \brief Create a new spdlog logger with the given name - explicit SpdlogLogger(std::string_view logger_name = "iceberg"); - - /// \brief Create a spdlog logger from an existing spdlog::logger - explicit SpdlogLogger(std::shared_ptr spdlog_logger); - - // Implementation methods required by LoggerInterface - bool ShouldLogImpl(LogLevel level) const noexcept; - - template - void LogImpl(LogLevel level, std::string_view format_str, Args&&... args) const { - if constexpr (sizeof...(args) > 0) { - std::string formatted_message = - std::vformat(format_str, std::make_format_args(args...)); - LogRawImpl(level, formatted_message); - } else { - LogRawImpl(level, std::string(format_str)); - } - } - - template - void LogWithLocationImpl(LogLevel level, std::string_view format_str, - const std::source_location& location, Args&&... args) const { - std::string message; - if constexpr (sizeof...(args) > 0) { - message = std::vformat(format_str, std::make_format_args(args...)); - } else { - message = std::string(format_str); - } - LogWithLocationRawImpl(level, message, location); - } - - void SetLevelImpl(LogLevel level); - LogLevel GetLevelImpl() const noexcept; - - /// \brief Get the underlying spdlog logger (for advanced usage) - std::shared_ptr GetUnderlyingLogger() const { return logger_; } private: - void LogRawImpl(LogLevel level, const std::string& message) const; - void LogWithLocationRawImpl(LogLevel level, const std::string& message, - const std::source_location& location) const; + /// \brief Get const pointer to the derived class instance + const Derived* derived() const noexcept { return static_cast(this); } - std::shared_ptr logger_; // Type-erased spdlog::logger - LogLevel current_level_ = LogLevel::kInfo; + /// \brief Get non-const pointer to the derived class instance + Derived* derived() noexcept { return static_cast(this); } }; -/// \brief No-op logger implementation for performance-critical scenarios -class NullLogger : public LoggerInterface { - public: - bool ShouldLogImpl(LogLevel) const noexcept { return false; } - - template - void LogImpl(LogLevel, std::string_view, Args&&...) const noexcept {} - - template - void LogWithLocationImpl(LogLevel, std::string_view, const std::source_location&, - Args&&...) const noexcept {} - - void SetLevelImpl(LogLevel) noexcept {} - LogLevel GetLevelImpl() const noexcept { return LogLevel::kOff; } -}; +/// \brief Concept to constrain types that implement the Logger interface +template +concept Logger = requires(const T& t, T& nt, LogLevel level, + const std::source_location& location, + std::string_view format_str) { + { t.ShouldLogImpl(level) } -> std::convertible_to; + { t.LogImpl(level, location, format_str) } -> std::same_as; + { nt.SetLevelImpl(level) } -> std::same_as; + { t.GetLevelImpl() } -> std::convertible_to; +} && std::is_base_of_v, T>; /// \brief Global logger registry for managing logger instances -class LoggerRegistry { +class ICEBERG_EXPORT LoggerRegistry { public: /// \brief Get the singleton instance of the logger registry static LoggerRegistry& Instance(); @@ -217,11 +123,16 @@ class LoggerRegistry { template void SetDefaultLogger(std::shared_ptr logger) { default_logger_ = std::static_pointer_cast(logger); - log_func_ = [](const void* logger_ptr, LogLevel level, const std::string& message) { + log_func_ = [](const void* logger_ptr, LogLevel level, + const std::source_location& location, std::string_view format_str, + std::format_args args) { auto* typed_logger = static_cast(logger_ptr); - if (typed_logger->ShouldLog(level)) { - typed_logger->LogImpl(level, "{}", message); - } + std::string formatted_message = std::vformat(format_str, args); + typed_logger->Log(level, location, "{}", formatted_message); + }; + should_log_func_ = [](const void* logger_ptr, LogLevel level) -> bool { + auto* typed_logger = static_cast(logger_ptr); + return typed_logger->ShouldLog(level); }; } @@ -236,15 +147,22 @@ class LoggerRegistry { /// \brief Log using the default logger template - void Log(LogLevel level, std::string_view format_str, Args&&... args) const { - if (default_logger_ && log_func_) { - std::string formatted_message; - if constexpr (sizeof...(args) > 0) { - formatted_message = std::vformat(format_str, std::make_format_args(args...)); - } else { - formatted_message = std::string(format_str); + void Log(LogLevel level, const std::source_location& location, + std::string_view format_str, Args&&... args) const { + if (default_logger_ && should_log_func_ && log_func_) { + if (should_log_func_(default_logger_.get(), level)) { + try { + if constexpr (sizeof...(args) > 0) { + auto args_store = std::make_format_args(args...); + log_func_(default_logger_.get(), level, location, format_str, args_store); + } else { + log_func_(default_logger_.get(), level, location, format_str, + std::format_args{}); + } + } catch (const std::exception& e) { + std::cerr << "Logging error: " << e.what() << std::endl; + } } - log_func_(default_logger_.get(), level, formatted_message); } } @@ -255,32 +173,39 @@ class LoggerRegistry { LoggerRegistry() = default; std::shared_ptr default_logger_; - void (*log_func_)(const void*, LogLevel, const std::string&) = nullptr; + void (*log_func_)(const void*, LogLevel, const std::source_location&, std::string_view, + std::format_args) = nullptr; + bool (*should_log_func_)(const void*, LogLevel) = nullptr; }; /// \brief Convenience macros for logging with automatic source location -#define LOG_WITH_LOCATION(level, format_str, ...) \ +#define ICEBERG_LOG_WITH_LOCATION(level, format_str, ...) \ do { \ - ::iceberg::LoggerRegistry::Instance().Log(level, \ + ::iceberg::LoggerRegistry::Instance().Log(level, std::source_location::current(), \ format_str __VA_OPT__(, ) __VA_ARGS__); \ - } while (0) - -#define LOG_TRACE(format_str, ...) \ - LOG_WITH_LOCATION(::iceberg::LogLevel::kTrace, format_str __VA_OPT__(, ) __VA_ARGS__) + } while (false) -#define LOG_DEBUG(format_str, ...) \ - LOG_WITH_LOCATION(::iceberg::LogLevel::kDebug, format_str __VA_OPT__(, ) __VA_ARGS__) +#define ICEBERG_LOG_TRACE(format_str, ...) \ + ICEBERG_LOG_WITH_LOCATION(::iceberg::LogLevel::kTrace, \ + format_str __VA_OPT__(, ) __VA_ARGS__) -#define LOG_INFO(format_str, ...) \ - LOG_WITH_LOCATION(::iceberg::LogLevel::kInfo, format_str __VA_OPT__(, ) __VA_ARGS__) +#define ICEBERG_LOG_DEBUG(format_str, ...) \ + ICEBERG_LOG_WITH_LOCATION(::iceberg::LogLevel::kDebug, \ + format_str __VA_OPT__(, ) __VA_ARGS__) -#define LOG_WARN(format_str, ...) \ - LOG_WITH_LOCATION(::iceberg::LogLevel::kWarn, format_str __VA_OPT__(, ) __VA_ARGS__) +#define ICEBERG_LOG_INFO(format_str, ...) \ + ICEBERG_LOG_WITH_LOCATION(::iceberg::LogLevel::kInfo, \ + format_str __VA_OPT__(, ) __VA_ARGS__) -#define LOG_ERROR(format_str, ...) \ - LOG_WITH_LOCATION(::iceberg::LogLevel::kError, format_str __VA_OPT__(, ) __VA_ARGS__) +#define ICEBERG_LOG_WARN(format_str, ...) \ + ICEBERG_LOG_WITH_LOCATION(::iceberg::LogLevel::kWarn, \ + format_str __VA_OPT__(, ) __VA_ARGS__) -#define LOG_CRITICAL(format_str, ...) \ - LOG_WITH_LOCATION(::iceberg::LogLevel::kCritical, format_str __VA_OPT__(, ) __VA_ARGS__) +#define ICEBERG_LOG_ERROR(format_str, ...) \ + ICEBERG_LOG_WITH_LOCATION(::iceberg::LogLevel::kError, \ + format_str __VA_OPT__(, ) __VA_ARGS__) +#define ICEBERG_LOG_CRITICAL(format_str, ...) \ + ICEBERG_LOG_WITH_LOCATION(::iceberg::LogLevel::kCritical, \ + format_str __VA_OPT__(, ) __VA_ARGS__) } // namespace iceberg diff --git a/src/iceberg/util/spdlog_logger.h b/src/iceberg/util/spdlog_logger.h new file mode 100644 index 000000000..dadffeed3 --- /dev/null +++ b/src/iceberg/util/spdlog_logger.h @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include + +#include "iceberg/util/logger.h" + +namespace iceberg { + +/// \brief Default spdlog-based logger implementation +class SpdlogLogger : public LoggerInterface { + public: + /// \brief Create a new spdlog logger with the given name + explicit SpdlogLogger(std::string_view logger_name = "iceberg"); + + /// \brief Create a spdlog logger from an existing spdlog::logger + explicit SpdlogLogger(std::shared_ptr spdlog_logger); + + // Implementation methods required by LoggerInterface + bool ShouldLogImpl(LogLevel level) const noexcept; + + template + void LogImpl(LogLevel level, const std::source_location& location, + std::string_view format_str, Args&&... args) const { + if constexpr (sizeof...(args) > 0) { + std::string formatted_message = + std::vformat(format_str, std::make_format_args(args...)); + LogRawImpl(level, location, formatted_message); + } else { + LogRawImpl(level, location, std::string(format_str)); + } + } + + void SetLevelImpl(LogLevel level); + LogLevel GetLevelImpl() const noexcept; + + /// \brief Get the underlying spdlog logger (for advanced usage) + std::shared_ptr GetUnderlyingLogger() const { return logger_; } + + private: + void LogRawImpl(LogLevel level, const std::source_location& location, + const std::string& message) const; + std::shared_ptr logger_; // Type-erased spdlog::logger + LogLevel current_level_ = LogLevel::kInfo; +}; + +} // namespace iceberg diff --git a/test/logger_test.cc b/test/logger_test.cc index cc6594047..7001c31ac 100644 --- a/test/logger_test.cc +++ b/test/logger_test.cc @@ -23,6 +23,8 @@ #include +#include "iceberg/util/spdlog_logger.h" + namespace iceberg { /// \brief Example custom logger implementation using std::cout for testing @@ -37,29 +39,15 @@ class StdoutLogger : public LoggerInterface { bool ShouldLogImpl(LogLevel level) const noexcept { return level >= min_level_; } template - void LogImpl(LogLevel level, std::string_view format_str, Args&&... args) const { + void LogImpl(LogLevel level, const std::source_location& location, + std::string_view format_str, Args&&... args) const { if constexpr (sizeof...(args) > 0) { std::string formatted_message = std::vformat(format_str, std::make_format_args(args...)); - LogRawImpl(level, formatted_message); - } else { - LogRawImpl(level, std::string(format_str)); - } - } - - template - void LogWithLocationImpl(LogLevel level, std::string_view format_str, - const std::source_location& location, Args&&... args) const { - std::string message; - if constexpr (sizeof...(args) > 0) { - message = std::vformat(format_str, std::make_format_args(args...)); + LogRawImpl(level, location, formatted_message); } else { - message = std::string(format_str); + LogRawImpl(level, location, std::string(format_str)); } - - std::string full_message = - std::format("[{}:{}] {}", location.file_name(), location.line(), message); - LogRawImpl(level, full_message); } void SetLevelImpl(LogLevel level) noexcept { min_level_ = level; } @@ -67,7 +55,8 @@ class StdoutLogger : public LoggerInterface { LogLevel GetLevelImpl() const noexcept { return min_level_; } private: - void LogRawImpl(LogLevel level, const std::string& message) const { + void LogRawImpl(LogLevel level, const std::source_location& location, + const std::string& message) const { std::cout << "[" << LogLevelToString(level) << "] " << message << std::endl; } @@ -91,10 +80,10 @@ TEST_F(LoggerTest, DefaultSpdlogLogger) { LoggerRegistry::Instance().InitializeDefault("test_logger"); // Test basic logging functionality - LOG_INFO("This is an info message"); - LOG_DEBUG("This is a debug message with value: {}", 42); - LOG_WARN("This is a warning message"); - LOG_ERROR("This is an error message"); + ICEBERG_LOG_INFO("This is an info message"); + ICEBERG_LOG_DEBUG("This is a debug message with value: {}", 42); + ICEBERG_LOG_WARN("This is a warning message"); + ICEBERG_LOG_ERROR("This is an error message"); // The test passes if no exceptions are thrown SUCCEED(); @@ -106,9 +95,9 @@ TEST_F(LoggerTest, CustomStdoutLogger) { LoggerRegistry::Instance().SetDefaultLogger(custom_logger); // Test logging with custom logger - LOG_DEBUG("Debug message from custom logger"); - LOG_INFO("Info message with parameter: {}", "test"); - LOG_WARN("Warning from custom logger"); + ICEBERG_LOG_DEBUG("Debug message from custom logger"); + ICEBERG_LOG_INFO("Info message with parameter: {}", "test"); + ICEBERG_LOG_WARN("Warning from custom logger"); SUCCEED(); } From 93c797afeed6b47e4d6f74937c5e90db9a2dc1a3 Mon Sep 17 00:00:00 2001 From: Ying Cai Date: Mon, 11 Aug 2025 17:42:57 +0800 Subject: [PATCH 5/5] try to fix lint issue. --- src/iceberg/util/logger.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/iceberg/util/logger.h b/src/iceberg/util/logger.h index 1c9c6b98f..1c7aab7a7 100644 --- a/src/iceberg/util/logger.h +++ b/src/iceberg/util/logger.h @@ -128,7 +128,7 @@ class ICEBERG_EXPORT LoggerRegistry { std::format_args args) { auto* typed_logger = static_cast(logger_ptr); std::string formatted_message = std::vformat(format_str, args); - typed_logger->Log(level, location, "{}", formatted_message); + typed_logger->Log(level, location, formatted_message); }; should_log_func_ = [](const void* logger_ptr, LogLevel level) -> bool { auto* typed_logger = static_cast(logger_ptr); @@ -157,7 +157,7 @@ class ICEBERG_EXPORT LoggerRegistry { log_func_(default_logger_.get(), level, location, format_str, args_store); } else { log_func_(default_logger_.get(), level, location, format_str, - std::format_args{}); + std::make_format_args()); } } catch (const std::exception& e) { std::cerr << "Logging error: " << e.what() << std::endl;