diff --git a/CMakeLists.txt b/CMakeLists.txt index cb0b2ae2b..0e0b1ea04 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,6 +55,7 @@ option(ICEBERG_SQL_MYSQL "Build the MySQL connector for the SQL catalog" OFF) option(ICEBERG_S3 "Build with S3 support" OFF) option(ICEBERG_SIGV4 "Build with SigV4 support" OFF) option(ICEBERG_BUNDLE_AWSSDK "Bundle AWS SDK for S3/SigV4 support" ON) +option(ICEBERG_SPDLOG "Use spdlog as the default logging backend" ON) option(ICEBERG_ENABLE_ASAN "Enable Address Sanitizer" OFF) option(ICEBERG_ENABLE_UBSAN "Enable Undefined Behavior Sanitizer" OFF) diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake index 8e10fd8ec..fa99db664 100644 --- a/cmake_modules/IcebergThirdpartyToolchain.cmake +++ b/cmake_modules/IcebergThirdpartyToolchain.cmake @@ -720,7 +720,9 @@ resolve_zlib_dependency() resolve_nanoarrow_dependency() resolve_croaring_dependency() resolve_nlohmann_json_dependency() -resolve_spdlog_dependency() +if(ICEBERG_SPDLOG) + resolve_spdlog_dependency() +endif() if(ICEBERG_S3 OR ICEBERG_SIGV4) if(ICEBERG_SIGV4 AND NOT ICEBERG_BUILD_REST) diff --git a/meson.build b/meson.build index 46446cd8d..46d7ab6ae 100644 --- a/meson.build +++ b/meson.build @@ -31,7 +31,9 @@ project( ) cpp = meson.get_compiler('cpp') -args = cpp.get_supported_arguments(['/bigobj']) +# /Zc:preprocessor: MSVC's conforming preprocessor, required for the __VA_OPT__ +# used by the logging macros. get_supported_arguments drops it on non-MSVC. +args = cpp.get_supported_arguments(['/bigobj', '/Zc:preprocessor']) add_project_arguments(args, language: 'cpp') subdir('src') diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 94523fb54..5d618d8a1 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -17,6 +17,18 @@ set(ICEBERG_INCLUDES "$" "$") + +# Generate the logging backend config header. ALWAYS generated (not gated by +# ICEBERG_SPDLOG) so logging/logger.cc can include it in both ON and OFF builds; +# only the definedness of ICEBERG_HAS_SPDLOG varies. Generated into the build +# tree (already on ICEBERG_INCLUDES), included as "iceberg/logging/config.h", and +# NOT installed (it must never appear in a public/installed header). +if(ICEBERG_SPDLOG) + set(ICEBERG_HAS_SPDLOG ON) +endif() +configure_file("${CMAKE_CURRENT_SOURCE_DIR}/logging/config.h.in" + "${CMAKE_CURRENT_BINARY_DIR}/logging/config.h") + set(ICEBERG_SOURCES arrow_c_data_util.cc arrow_c_data_guard_internal.cc @@ -151,24 +163,34 @@ list(APPEND ICEBERG_STATIC_BUILD_INTERFACE_LIBS "$,nanoarrow::nanoarrow_static,$,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>" nlohmann_json::nlohmann_json - spdlog::spdlog ZLIB::ZLIB) list(APPEND ICEBERG_SHARED_BUILD_INTERFACE_LIBS "$,nanoarrow::nanoarrow_static,$,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>" nlohmann_json::nlohmann_json - spdlog::spdlog ZLIB::ZLIB) list(APPEND ICEBERG_STATIC_INSTALL_INTERFACE_LIBS "$,iceberg::nanoarrow_static,$,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>" "$,iceberg::nlohmann_json,$,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>" - "$,iceberg::spdlog,spdlog::spdlog>") +) list(APPEND ICEBERG_SHARED_INSTALL_INTERFACE_LIBS "$,iceberg::nanoarrow_static,$,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>" "$,iceberg::nlohmann_json,$,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>" - "$,iceberg::spdlog,spdlog::spdlog>") +) + +# spdlog backend: linked and compiled only when ICEBERG_SPDLOG is ON. When OFF, +# the core library has no spdlog dependency and CerrLogger is the default sink. +if(ICEBERG_SPDLOG) + list(APPEND ICEBERG_SOURCES logging/internal/spdlog_logger.cc) + list(APPEND ICEBERG_STATIC_BUILD_INTERFACE_LIBS spdlog::spdlog) + list(APPEND ICEBERG_SHARED_BUILD_INTERFACE_LIBS spdlog::spdlog) + list(APPEND ICEBERG_STATIC_INSTALL_INTERFACE_LIBS + "$,iceberg::spdlog,spdlog::spdlog>") + list(APPEND ICEBERG_SHARED_INSTALL_INTERFACE_LIBS + "$,iceberg::spdlog,spdlog::spdlog>") +endif() add_iceberg_lib(iceberg SOURCES diff --git a/src/iceberg/logging/config.h.in b/src/iceberg/logging/config.h.in new file mode 100644 index 000000000..1b1e0d02c --- /dev/null +++ b/src/iceberg/logging/config.h.in @@ -0,0 +1,30 @@ +/* + * 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 + +// Internal, build-generated configuration for the logging backend. +// This header is NOT installed and must only be included from .cc files +// (logger.cc, internal/spdlog_logger.cc) -- never from a public header. +// +// ICEBERG_HAS_SPDLOG is defined when the project is built with -DICEBERG_SPDLOG=ON +// and left undefined otherwise. Always test it with #ifdef / #ifndef, never #if +// (it carries no value). + +#cmakedefine ICEBERG_HAS_SPDLOG diff --git a/src/iceberg/logging/internal/spdlog_logger.cc b/src/iceberg/logging/internal/spdlog_logger.cc new file mode 100644 index 000000000..4ede3f402 --- /dev/null +++ b/src/iceberg/logging/internal/spdlog_logger.cc @@ -0,0 +1,103 @@ +/* + * 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/logging/internal/spdlog_logger.h" + +#ifdef ICEBERG_HAS_SPDLOG + +# include +# include +# include +# include + +# include +# include + +namespace iceberg::internal { + +namespace { + +spdlog::level::level_enum ToSpdLevel(LogLevel level) noexcept { + 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: + case LogLevel::kFatal: + // spdlog has no "fatal"; the process abort is owned by the macro layer. + return spdlog::level::critical; + case LogLevel::kOff: + return spdlog::level::off; + } + return spdlog::level::off; +} + +} // namespace + +SpdLogger::SpdLogger(LogLevel level) + : SpdLogger(std::make_shared( + "iceberg", std::make_shared()), + level) {} + +Status SpdLogger::Initialize( + const std::unordered_map& properties) { + if (auto it = properties.find(std::string(kPatternProperty)); it != properties.end()) { + logger_->set_pattern(it->second); + } + // Apply "level" via the base implementation. + return Logger::Initialize(properties); +} + +SpdLogger::SpdLogger(std::shared_ptr logger, LogLevel level) + : logger_(std::move(logger)), level_(level) { + if (logger_) { + logger_->set_level(spdlog::level::trace); // filtering is done by ShouldLog + } +} + +void SpdLogger::Log(LogMessage&& message) noexcept { + try { + spdlog::source_loc loc{message.location.file_name(), + static_cast(message.location.line()), + message.location.function_name()}; + // Pass the pre-formatted text as an argument ("{}") so any braces in the + // message are not re-interpreted as a format string. + logger_->log(loc, ToSpdLevel(message.level), "{}", message.message); + } catch (...) { + // Logging must never throw. + } +} + +void SpdLogger::Flush() noexcept { + try { + logger_->flush(); + } catch (...) { + } +} + +} // namespace iceberg::internal + +#endif // ICEBERG_HAS_SPDLOG diff --git a/src/iceberg/logging/internal/spdlog_logger.h b/src/iceberg/logging/internal/spdlog_logger.h new file mode 100644 index 000000000..649ed3073 --- /dev/null +++ b/src/iceberg/logging/internal/spdlog_logger.h @@ -0,0 +1,89 @@ +/* + * 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 + +/// \file iceberg/logging/internal/spdlog_logger.h +/// \brief spdlog-backed logging sink. +/// +/// INTERNAL, NOT INSTALLED. It is only included from .cc files (logger.cc and +/// spdlog_logger.cc) after config.h, and only when the project is built with +/// ICEBERG_SPDLOG=ON. SpdLogger is not a consumer-constructible public type -- +/// applications obtain it via the default logger or the "logger-impl"="spdlog" +/// registry factory. + +#include "iceberg/logging/config.h" + +#ifdef ICEBERG_HAS_SPDLOG + +# include +# include + +# include + +# include "iceberg/logging/log_level.h" +# include "iceberg/logging/logger.h" + +namespace iceberg::internal { + +/// \brief Logger backed by spdlog (synchronous only in v1). +/// +/// Synchronous because spdlog::source_loc holds non-owning const char* that are +/// unsafe to forward into an async logger (spdlog #3227). +/// ICEBERG_EXPORT so the symbol is linkable from in-tree tests (and any +/// internal consumer) under -fvisibility=hidden / MSVC DLL builds. The header +/// is still not installed -- this is a binary-visibility detail, not public API. +class ICEBERG_EXPORT SpdLogger : public Logger { + public: + /// \brief Construct over a default stderr-backed spdlog logger. + explicit SpdLogger(LogLevel level = LogLevel::kInfo); + + /// \brief Construct over a caller-provided spdlog logger. + /// + /// The logger MUST be synchronous. Log() forwards spdlog::source_loc, which + /// borrows the std::source_location's const char* pointers; an async spdlog + /// logger would queue them past their lifetime (spdlog #3227 -> UB). This is a + /// caller contract -- spdlog exposes no reliable sync/async query to assert on. + explicit SpdLogger(std::shared_ptr logger, + LogLevel level = LogLevel::kInfo); + + /// \brief Apply the "pattern" property (spdlog set_pattern), then "level". + Status Initialize( + const std::unordered_map& properties) override; + + bool ShouldLog(LogLevel level) const noexcept override { + return level >= level_.load(std::memory_order_relaxed); + } + void Log(LogMessage&& message) noexcept override; + void SetLevel(LogLevel level) noexcept override { + level_.store(level, std::memory_order_relaxed); + } + LogLevel level() const noexcept override { + return level_.load(std::memory_order_relaxed); + } + void Flush() noexcept override; + + private: + std::shared_ptr logger_; + std::atomic level_; +}; + +} // namespace iceberg::internal + +#endif // ICEBERG_HAS_SPDLOG diff --git a/src/iceberg/logging/logger.cc b/src/iceberg/logging/logger.cc index a8db67ca9..8d6efbe78 100644 --- a/src/iceberg/logging/logger.cc +++ b/src/iceberg/logging/logger.cc @@ -26,7 +26,13 @@ #include #include +// Build-generated, .cc-only (never from a public header). Defines +// ICEBERG_HAS_SPDLOG when built with -DICEBERG_SPDLOG=ON; tested with #ifdef. #include "iceberg/logging/cerr_logger.h" +#include "iceberg/logging/config.h" +#ifdef ICEBERG_HAS_SPDLOG +# include "iceberg/logging/internal/spdlog_logger.h" +#endif namespace iceberg { @@ -44,9 +50,15 @@ class NoopLogger final : public Logger { /// \brief Construct the process default logger for this build configuration. /// -/// Uses the always-available std::cerr sink. The spdlog backend (preferred when -/// compiled in) is wired into this factory in a later block. -std::shared_ptr MakeDefaultLogger() { return std::make_shared(); } +/// Prefers the spdlog backend when compiled in; otherwise the always-available +/// std::cerr logger. +std::shared_ptr MakeDefaultLogger() { +#ifdef ICEBERG_HAS_SPDLOG + return std::make_shared(); +#else + return std::make_shared(); +#endif +} /// \brief The process-global default-logger slot. struct DefaultSlot { diff --git a/src/iceberg/logging/logger.h b/src/iceberg/logging/logger.h index 66f81501a..6af862f00 100644 --- a/src/iceberg/logging/logger.h +++ b/src/iceberg/logging/logger.h @@ -371,3 +371,179 @@ void Log(Logger& logger, LogLevel level, } } // namespace iceberg + +// --------------------------------------------------------------------------- +// Logging macros. +// +// Every macro takes a std::format string followed by its arguments. The +// rendered line depends on the active backend (see cerr_logger.h for the +// std::cerr layout, or the spdlog pattern); the examples below show the call +// site and, for the default CerrLogger, the line it produces. +// +// ICEBERG_LOG_TRACE("entering scan for {}", table); +// 2026-06-16T10:59:41.186Z trace [12345] table_scan.cc:88] entering scan for db.t +// ICEBERG_LOG_DEBUG("cache miss key={}", key); +// 2026-06-16T10:59:41.186Z debug [12345] cache.cc:42] cache miss key=manifest-7 +// ICEBERG_LOG_INFO("loaded {} manifests in {} ms", n, ms); +// 2026-06-16T10:59:41.186Z info [12345] table_scan.cc:91] loaded 5 manifests in 12 ms +// ICEBERG_LOG_WARN("retry {} after {}", attempt, err); +// 2026-06-16T10:59:41.186Z warn [12345] io.cc:51] retry 2 after timeout +// ICEBERG_LOG_ERROR("commit failed: {}", status); +// 2026-06-16T10:59:41.186Z error [12345] txn.cc:77] commit failed: conflict +// ICEBERG_LOG_CRITICAL("metadata unreadable at {}", path); +// 2026-06-16T10:59:41.186Z critical [12345] meta.cc:30] metadata unreadable at +// s3://b/m.json +// ICEBERG_LOG_FATAL("unrecoverable: {}", reason); // emits, flushes, then +// std::abort() +// 2026-06-16T10:59:41.186Z fatal [12345] boot.cc:19] unrecoverable: bad config +// +// Less common forms: +// ICEBERG_LOG(level, "level chosen at runtime: {}", x); // runtime severity +// ICEBERG_LOG_TO(logger, level, "to an explicit logger {}", y); +// ICEBERG_LOG_RUNTIME_FMT(level, fmt_string, args...); // non-literal format +// +// With ICEBERG_LOG_SHORT_MACROS defined, bare aliases (LOG_INFO, ...) are also +// available. A format string is mandatory; zero extra args is fine +// (ICEBERG_LOG_INFO("done")). +// --------------------------------------------------------------------------- + +/// \brief Compile-time severity floor: statements below this level are removed +/// entirely from the build (their format call sites and source_location literals +/// are never emitted). Defaults to keeping everything. ICEBERG_LOG_FATAL is never +/// gated by this floor -- its abort is always compiled in. +#ifndef ICEBERG_LOG_ACTIVE_LEVEL +# define ICEBERG_LOG_ACTIVE_LEVEL ::iceberg::LogLevel::kTrace +#endif + +// Internal: fixed-severity emit with compile-time floor then the authoritative +// Logger::ShouldLog (the single source of truth for runtime filtering), with +// formatting only on the taken path, never throwing. +#define ICEBERG_INTERNAL_LOG(level_, FMT_, ...) \ + do { \ + if constexpr ((level_) >= ICEBERG_LOG_ACTIVE_LEVEL) { \ + const auto& _ib_logger = ::iceberg::internal::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(level_)) { \ + try { \ + ::iceberg::internal::Emit(*_ib_logger, (level_), \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::internal::EmitFormatError(*_ib_logger, (level_), \ + ::std::source_location::current()); \ + } \ + } \ + } \ + } while (0) + +#define ICEBERG_LOG_TRACE(...) \ + ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kTrace, __VA_ARGS__) +#define ICEBERG_LOG_DEBUG(...) \ + ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kDebug, __VA_ARGS__) +#define ICEBERG_LOG_INFO(...) \ + ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kInfo, __VA_ARGS__) +#define ICEBERG_LOG_WARN(...) \ + ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kWarn, __VA_ARGS__) +#define ICEBERG_LOG_ERROR(...) \ + ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kError, __VA_ARGS__) +#define ICEBERG_LOG_CRITICAL(...) \ + ICEBERG_INTERNAL_LOG(::iceberg::LogLevel::kCritical, __VA_ARGS__) + +// FATAL: emit if enabled (never compile-stripped), then ALWAYS flush + abort. +// Acquires the default logger ONCE and uses the same instance for emit and flush +// so a concurrent SetDefaultLogger cannot flush a different logger than it emitted to. +#define ICEBERG_LOG_FATAL(FMT_, ...) \ + do { \ + auto _ib_logger = ::iceberg::GetDefaultLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(::iceberg::LogLevel::kFatal)) { \ + try { \ + ::iceberg::internal::Emit(*_ib_logger, ::iceberg::LogLevel::kFatal, \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::internal::EmitFormatError(*_ib_logger, ::iceberg::LogLevel::kFatal, \ + ::std::source_location::current()); \ + } \ + } \ + if (_ib_logger) _ib_logger->Flush(); \ + ::std::abort(); \ + } while (0) + +// Generic, runtime-level form against the default logger. No compile-time floor +// (the level is not a constant). Acquires the logger once; aborts when level == kFatal +// (flushing that same logger first). +#define ICEBERG_LOG(level_, FMT_, ...) \ + do { \ + const ::iceberg::LogLevel _ib_lvl = (level_); \ + const auto& _ib_logger = ::iceberg::internal::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(_ib_lvl)) { \ + try { \ + ::iceberg::internal::Emit(*_ib_logger, _ib_lvl, \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::internal::EmitFormatError(*_ib_logger, _ib_lvl, \ + ::std::source_location::current()); \ + } \ + } \ + if (_ib_lvl == ::iceberg::LogLevel::kFatal) { \ + if (_ib_logger) _ib_logger->Flush(); \ + ::std::abort(); \ + } \ + } while (0) + +// Generic form targeting an EXPLICIT logger (must be an lvalue Logger&). Honors +// only that logger's ShouldLog. Aborts when level == kFatal. +#define ICEBERG_LOG_TO(logger_, level_, FMT_, ...) \ + do { \ + ::iceberg::Logger& _ib_logger = (logger_); \ + const ::iceberg::LogLevel _ib_lvl = (level_); \ + if (_ib_logger.ShouldLog(_ib_lvl)) { \ + try { \ + ::iceberg::internal::Emit(_ib_logger, _ib_lvl, \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::internal::EmitFormatError(_ib_logger, _ib_lvl, \ + ::std::source_location::current()); \ + } \ + } \ + if (_ib_lvl == ::iceberg::LogLevel::kFatal) { \ + _ib_logger.Flush(); \ + ::std::abort(); \ + } \ + } while (0) + +// Runtime (non-literal) format string against the default logger. Acquires the +// logger once; aborts when level == kFatal (flushing that same logger first). +#define ICEBERG_LOG_RUNTIME_FMT(level_, FMT_, ...) \ + do { \ + const ::iceberg::LogLevel _ib_lvl = (level_); \ + const auto& _ib_logger = ::iceberg::internal::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(_ib_lvl)) { \ + try { \ + ::iceberg::internal::Emit( \ + *_ib_logger, _ib_lvl, ::std::source_location::current(), \ + ::iceberg::internal::VFormat((FMT_)__VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::internal::EmitFormatError(*_ib_logger, _ib_lvl, \ + ::std::source_location::current()); \ + } \ + } \ + if (_ib_lvl == ::iceberg::LogLevel::kFatal) { \ + if (_ib_logger) _ib_logger->Flush(); \ + ::std::abort(); \ + } \ + } while (0) + +// Bare, Java-style aliases. Opt-IN only (define ICEBERG_LOG_SHORT_MACROS before +// including this header) to avoid colliding with glog/abseil/windows.h in +// consumer translation units. No bare LOG(level) is provided. +#ifdef ICEBERG_LOG_SHORT_MACROS +# define LOG_TRACE(...) ICEBERG_LOG_TRACE(__VA_ARGS__) +# define LOG_DEBUG(...) ICEBERG_LOG_DEBUG(__VA_ARGS__) +# define LOG_INFO(...) ICEBERG_LOG_INFO(__VA_ARGS__) +# define LOG_WARN(...) ICEBERG_LOG_WARN(__VA_ARGS__) +# define LOG_ERROR(...) ICEBERG_LOG_ERROR(__VA_ARGS__) +# define LOG_CRITICAL(...) ICEBERG_LOG_CRITICAL(__VA_ARGS__) +# define LOG_FATAL(...) ICEBERG_LOG_FATAL(__VA_ARGS__) +#endif // ICEBERG_LOG_SHORT_MACROS diff --git a/src/iceberg/logging/meson.build b/src/iceberg/logging/meson.build index e4ca111f4..75621fc91 100644 --- a/src/iceberg/logging/meson.build +++ b/src/iceberg/logging/meson.build @@ -15,6 +15,16 @@ # specific language governing permissions and limitations # under the License. +# Generate the .cc-only logging backend config header. The meson build always +# links spdlog, so ICEBERG_HAS_SPDLOG is always defined here. Generated into +# build/src/iceberg/logging/config.h (resolved via include_directories('..'), +# which exposes both the source and build trees); not installed. +logging_config_data = configuration_data() +logging_config_data.set('ICEBERG_HAS_SPDLOG', 1) +configure_file(output: 'config.h', configuration: logging_config_data) + +# Public logging headers. The build-generated config.h and the internal +# SpdLogger header are intentionally NOT installed. install_headers( ['cerr_logger.h', 'log_level.h', 'logger.h'], subdir: 'iceberg/logging', diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index 71a4498f1..a87284512 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -39,6 +39,8 @@ configure_file( install_dir: get_option('includedir') / 'iceberg', ) +# Generate iceberg/logging/config.h (must precede the library() that compiles +# the logging sources which include it). subdir('logging') iceberg_include_dir = include_directories('..') @@ -75,6 +77,7 @@ iceberg_sources = files( 'json_serde.cc', 'location_provider.cc', 'logging/cerr_logger.cc', + 'logging/internal/spdlog_logger.cc', 'logging/logger.cc', 'manifest/manifest_adapter.cc', 'manifest/manifest_entry.cc', diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index fcbc22126..0716a2c9e 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -64,7 +64,9 @@ function(add_iceberg_test test_name) endif() if(MSVC_TOOLCHAIN) - target_compile_options(${test_name} PRIVATE /bigobj) + # /Zc:preprocessor: conforming preprocessor for the __VA_OPT__ in the logging + # macros (MSVC's traditional preprocessor rejects it). + target_compile_options(${test_name} PRIVATE /bigobj /Zc:preprocessor) endif() add_test(NAME ${test_name} COMMAND ${test_name}) @@ -106,7 +108,10 @@ add_iceberg_test(logging_test SOURCES cerr_logger_test.cc log_level_test.cc - logger_test.cc) + logger_test.cc + macros_active_level_test.cc + macros_test.cc + spdlog_logger_test.cc) add_iceberg_test(expression_test SOURCES diff --git a/src/iceberg/test/macros_active_level_test.cc b/src/iceberg/test/macros_active_level_test.cc new file mode 100644 index 000000000..c0e138a29 --- /dev/null +++ b/src/iceberg/test/macros_active_level_test.cc @@ -0,0 +1,60 @@ +/* + * 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. + */ + +// Compile-time floor set to kOff for this translation unit: every fixed-severity +// macro below kFatal must be stripped to nothing, while ICEBERG_LOG_FATAL must +// still abort (its abort is never gated by the compile-time floor). +#define ICEBERG_LOG_ACTIVE_LEVEL ::iceberg::LogLevel::kOff + +#include + +#include + +#include "iceberg/logging/log_level.h" +#include "iceberg/logging/logger.h" +#include "iceberg/test/logging_test_helpers.h" + +namespace iceberg { + +TEST(MacrosActiveLevelTest, BelowFloorStatementsAreCompiledOut) { + auto logger = std::make_shared(); + logger->SetLevel(LogLevel::kTrace); + ScopedDefaultLogger guard(logger); + + int calls = 0; + // counted() is only "called" from the compile-time-stripped macros below, so the + // analyzer sees its init as a dead store -- which is exactly what this verifies. + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) + auto counted = [&calls]() { + ++calls; + return 1; + }; + // Stripped at compile time -> arguments never evaluated, nothing emitted, + // even though the runtime logger would accept these levels. + ICEBERG_LOG_INFO("{}", counted()); + ICEBERG_LOG_CRITICAL("{}", counted()); + EXPECT_EQ(calls, 0); + EXPECT_EQ(logger->count(), 0u); +} + +TEST(MacrosActiveLevelDeathTest, FatalStillAbortsWhenEverythingElseStripped) { + EXPECT_DEATH({ ICEBERG_LOG_FATAL("still fatal"); }, ""); +} + +} // namespace iceberg diff --git a/src/iceberg/test/macros_test.cc b/src/iceberg/test/macros_test.cc new file mode 100644 index 000000000..28ac8f526 --- /dev/null +++ b/src/iceberg/test/macros_test.cc @@ -0,0 +1,150 @@ +/* + * 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 + +#include + +#include "iceberg/logging/cerr_logger.h" +#include "iceberg/logging/log_level.h" +#include "iceberg/logging/logger.h" +#include "iceberg/test/logging_test_helpers.h" + +namespace iceberg { + +namespace { + +std::shared_ptr InstallCapturing(LogLevel level = LogLevel::kTrace) { + auto logger = std::make_shared(); + logger->SetLevel(level); + return logger; +} + +} // namespace + +TEST(MacrosTest, InfoFormatsAndCapturesLocation) { + auto logger = InstallCapturing(); + ScopedDefaultLogger guard(logger); + ICEBERG_LOG_INFO("x={}", 42); + auto records = logger->records(); + ASSERT_EQ(records.size(), 1u); + EXPECT_EQ(records[0].level, LogLevel::kInfo); + EXPECT_EQ(records[0].message, "x=42"); + EXPECT_NE(records[0].location.line(), 0u); +} + +TEST(MacrosTest, RuntimeLevelFiltersBelowThreshold) { + auto logger = InstallCapturing(); + ScopedDefaultLogger guard(logger); + SetDefaultLevel(LogLevel::kError); + ICEBERG_LOG_INFO("dropped"); + ICEBERG_LOG_ERROR("kept"); + auto records = logger->records(); + ASSERT_EQ(records.size(), 1u); + EXPECT_EQ(records[0].message, "kept"); +} + +TEST(MacrosTest, DisabledLevelDoesNotEvaluateArguments) { + auto logger = InstallCapturing(); + ScopedDefaultLogger guard(logger); + SetDefaultLevel(LogLevel::kError); + int calls = 0; + auto counted = [&calls]() { + ++calls; + return 1; + }; + ICEBERG_LOG_INFO("{}", counted()); + EXPECT_EQ(calls, 0); +} + +TEST(MacrosTest, DanglingElseBindsCorrectly) { + auto logger = InstallCapturing(); + ScopedDefaultLogger guard(logger); + bool took_else = false; + // Intentionally brace-free: this verifies the macro keeps dangling-else binding + // correct. Adding braces would defeat the test, so suppress the tidy check. + // NOLINTBEGIN(google-readability-braces-around-statements) + if (false) + ICEBERG_LOG_INFO("if-branch"); + else + took_else = true; + // NOLINTEND(google-readability-braces-around-statements) + EXPECT_TRUE(took_else); + EXPECT_EQ(logger->count(), 0u); +} + +TEST(MacrosTest, GenericRuntimeLevelMacroCompilesAndLogs) { + auto logger = InstallCapturing(); + ScopedDefaultLogger guard(logger); + LogLevel level = LogLevel::kWarn; + ICEBERG_LOG(level, "n={}", 7); + auto records = logger->records(); + ASSERT_EQ(records.size(), 1u); + EXPECT_EQ(records[0].message, "n=7"); + EXPECT_EQ(records[0].level, LogLevel::kWarn); +} + +TEST(MacrosTest, LogToHonorsOnlyExplicitLoggerNotDefaultGate) { + auto sink = InstallCapturing(); + ScopedDefaultLogger guard(InstallCapturing()); + SetDefaultLevel(LogLevel::kOff); // default gate would block everything + ICEBERG_LOG_TO(*sink, LogLevel::kInfo, "explicit {}", 1); + EXPECT_EQ(sink->count(), 1u); +} + +TEST(MacrosTest, NeverThrowsOnBadRuntimeFormat) { + auto logger = InstallCapturing(); + ScopedDefaultLogger guard(logger); + // Invalid runtime format string -> std::vformat throws -> swallowed -> fallback. + EXPECT_NO_THROW(ICEBERG_LOG_RUNTIME_FMT(LogLevel::kInfo, "{")); + auto records = logger->records(); + ASSERT_EQ(records.size(), 1u); + EXPECT_EQ(records[0].message, ""); +} + +TEST(MacrosDeathTest, FatalEmitsThenAborts) { + // Default logger writes to std::cerr; the message must appear before abort. + EXPECT_DEATH({ ICEBERG_LOG_FATAL("fatalmsg {}", 7); }, "fatalmsg 7"); +} + +TEST(MacrosDeathTest, FatalAbortsEvenWhenRuntimeDisabled) { + EXPECT_DEATH( + { + SetDefaultLevel(LogLevel::kOff); + ICEBERG_LOG_FATAL("suppressed"); + }, + ""); +} + +TEST(MacrosDeathTest, GenericRuntimeFatalEmitsThenAborts) { + // ICEBERG_LOG with a runtime kFatal level must also emit then abort. + EXPECT_DEATH({ ICEBERG_LOG(LogLevel::kFatal, "gfatal {}", 1); }, "gfatal 1"); +} + +TEST(MacrosDeathTest, LogToFatalEmitsThenAborts) { + // ICEBERG_LOG_TO with kFatal must emit to the explicit logger then abort. + EXPECT_DEATH( + { + CerrLogger sink(LogLevel::kTrace); + ICEBERG_LOG_TO(sink, LogLevel::kFatal, "tofatal {}", 2); + }, + "tofatal 2"); +} + +} // namespace iceberg diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index ba0f6230d..f7d3d1b2f 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -66,6 +66,9 @@ iceberg_tests = { 'cerr_logger_test.cc', 'log_level_test.cc', 'logger_test.cc', + 'macros_active_level_test.cc', + 'macros_test.cc', + 'spdlog_logger_test.cc', ), }, 'expression_test': { diff --git a/src/iceberg/test/spdlog_logger_test.cc b/src/iceberg/test/spdlog_logger_test.cc new file mode 100644 index 000000000..30bbe6ff7 --- /dev/null +++ b/src/iceberg/test/spdlog_logger_test.cc @@ -0,0 +1,106 @@ +/* + * 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. + */ + +// Internal/build-generated header is acceptable in a test TU (not installed). +#include "iceberg/logging/config.h" + +#ifdef ICEBERG_HAS_SPDLOG + +# include +# include +# include +# include +# include + +# include +# include +# include + +# include "iceberg/logging/internal/spdlog_logger.h" +# include "iceberg/logging/log_level.h" +# include "iceberg/logging/logger.h" + +namespace iceberg { + +namespace { + +LogMessage MakeMessage(LogLevel level, std::string text) { + return LogMessage{.level = level, + .message = std::move(text), + .location = std::source_location::current(), + .attributes = {}}; +} + +internal::SpdLogger MakeCapturing(std::ostringstream& out, + LogLevel level = LogLevel::kTrace) { + auto sink = std::make_shared(out); + auto spd = std::make_shared("test", sink); + return internal::SpdLogger(spd, level); +} + +} // namespace + +TEST(SpdLoggerTest, DefaultLevelIsInfo) { + internal::SpdLogger logger; + EXPECT_EQ(logger.level(), LogLevel::kInfo); + EXPECT_FALSE(logger.ShouldLog(LogLevel::kDebug)); + EXPECT_TRUE(logger.ShouldLog(LogLevel::kError)); +} + +TEST(SpdLoggerTest, ForwardsMessageToSink) { + std::ostringstream out; + auto logger = MakeCapturing(out); + logger.Log(MakeMessage(LogLevel::kError, "boom 42")); + logger.Flush(); + EXPECT_NE(out.str().find("boom 42"), std::string::npos); +} + +TEST(SpdLoggerTest, MessageBracesAreNotInterpreted) { + std::ostringstream out; + auto logger = MakeCapturing(out); + // A pre-formatted message containing braces must pass through verbatim. + logger.Log(MakeMessage(LogLevel::kInfo, "literal {not a placeholder}")); + logger.Flush(); + EXPECT_NE(out.str().find("literal {not a placeholder}"), std::string::npos); +} + +TEST(SpdLoggerTest, CriticalAndFatalBothEmit) { + std::ostringstream out; + auto logger = MakeCapturing(out); + logger.Log(MakeMessage(LogLevel::kCritical, "crit")); + logger.Log(MakeMessage(LogLevel::kFatal, "fatal-tag")); + logger.Flush(); + EXPECT_NE(out.str().find("crit"), std::string::npos); + EXPECT_NE(out.str().find("fatal-tag"), std::string::npos); +} + +TEST(SpdLoggerTest, PatternPropertyChangesLayout) { + std::ostringstream out; + auto logger = MakeCapturing(out); + auto status = + logger.Initialize({{std::string(kPatternProperty), std::string("PFX %v")}}); + ASSERT_TRUE(status.has_value()); + logger.Log(MakeMessage(LogLevel::kError, "hello")); + logger.Flush(); + EXPECT_NE(out.str().find("PFX hello"), std::string::npos); +} + +} // namespace iceberg + +#endif // ICEBERG_HAS_SPDLOG