From 62d6fd116578e6ec55c11ae8bfed8f286bf48359 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Wed, 10 Jun 2026 07:55:06 +0000 Subject: [PATCH 1/6] Revert "chore: remove spdlog dependency (#459)" Re-add spdlog as a build dependency to serve as the foundation for the logging system. Adapted to current structure: spdlog joins the core iceberg lib interface lists (roaring has since moved to iceberg_data). Co-authored-by: Isaac --- README.md | 1 + .../IcebergThirdpartyToolchain.cmake | 57 +++++++++++++++++++ src/iceberg/CMakeLists.txt | 6 +- src/iceberg/meson.build | 3 +- subprojects/spdlog.wrap | 30 ++++++++++ 5 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 subprojects/spdlog.wrap diff --git a/README.md b/README.md index 9dfdbcf7e..7c9a343ec 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,7 @@ If you experience network issues when downloading dependencies, you can customiz - `ICEBERG_NANOARROW_URL`: Nanoarrow tarball URL - `ICEBERG_CROARING_URL`: CRoaring tarball URL - `ICEBERG_NLOHMANN_JSON_URL`: nlohmann-json tarball URL +- `ICEBERG_SPDLOG_URL`: spdlog tarball URL - `ICEBERG_CPR_URL`: cpr tarball URL Example: diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake index 9b5d95a7e..6654e3312 100644 --- a/cmake_modules/IcebergThirdpartyToolchain.cmake +++ b/cmake_modules/IcebergThirdpartyToolchain.cmake @@ -31,6 +31,7 @@ set(ICEBERG_ARROW_INSTALL_INTERFACE_LIBS) # ICEBERG_NANOARROW_URL - Nanoarrow tarball URL # ICEBERG_CROARING_URL - CRoaring tarball URL # ICEBERG_NLOHMANN_JSON_URL - nlohmann-json tarball URL +# ICEBERG_SPDLOG_URL - spdlog tarball URL # ICEBERG_CPR_URL - cpr tarball URL # # Example usage: @@ -437,6 +438,61 @@ function(resolve_nlohmann_json_dependency) PARENT_SCOPE) endfunction() +# ---------------------------------------------------------------------- +# spdlog + +function(resolve_spdlog_dependency) + prepare_fetchcontent() + + find_package(Threads REQUIRED) + + set(SPDLOG_USE_STD_FORMAT + ON + CACHE BOOL "" FORCE) + set(SPDLOG_BUILD_PIC + ON + CACHE BOOL "" FORCE) + + if(DEFINED ENV{ICEBERG_SPDLOG_URL}) + set(SPDLOG_URL "$ENV{ICEBERG_SPDLOG_URL}") + else() + set(SPDLOG_URL "https://github.com/gabime/spdlog/archive/refs/tags/v1.15.3.tar.gz") + endif() + + fetchcontent_declare(spdlog + ${FC_DECLARE_COMMON_OPTIONS} + URL ${SPDLOG_URL} + FIND_PACKAGE_ARGS + NAMES + spdlog + CONFIG) + fetchcontent_makeavailable(spdlog) + + if(spdlog_SOURCE_DIR) + set_target_properties(spdlog PROPERTIES OUTPUT_NAME "iceberg_vendored_spdlog" + POSITION_INDEPENDENT_CODE ON) + target_link_libraries(spdlog INTERFACE Threads::Threads) + install(TARGETS spdlog + EXPORT iceberg_targets + RUNTIME DESTINATION "${ICEBERG_INSTALL_BINDIR}" + ARCHIVE DESTINATION "${ICEBERG_INSTALL_LIBDIR}" + LIBRARY DESTINATION "${ICEBERG_INSTALL_LIBDIR}") + set(SPDLOG_VENDORED TRUE) + else() + set(SPDLOG_VENDORED FALSE) + list(APPEND ICEBERG_SYSTEM_DEPENDENCIES spdlog) + endif() + + list(APPEND ICEBERG_SYSTEM_DEPENDENCIES Threads) + + set(ICEBERG_SYSTEM_DEPENDENCIES + ${ICEBERG_SYSTEM_DEPENDENCIES} + PARENT_SCOPE) + set(SPDLOG_VENDORED + ${SPDLOG_VENDORED} + PARENT_SCOPE) +endfunction() + # ---------------------------------------------------------------------- # zlib @@ -614,6 +670,7 @@ resolve_zlib_dependency() resolve_nanoarrow_dependency() resolve_croaring_dependency() resolve_nlohmann_json_dependency() +resolve_spdlog_dependency() if(ICEBERG_BUILD_BUNDLE) resolve_arrow_dependency() diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index b641bb75e..0fe418d48 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -139,22 +139,24 @@ 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>") add_iceberg_lib(iceberg SOURCES diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index 15fd5d79d..8c4b49e9b 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -180,9 +180,10 @@ croaring_needs_static = ( croaring_dep = dependency('croaring', static: croaring_needs_static) nanoarrow_dep = dependency('nanoarrow') nlohmann_json_dep = dependency('nlohmann_json') +spdlog_dep = dependency('spdlog') zlib_dep = dependency('zlib') -iceberg_deps = [nanoarrow_dep, nlohmann_json_dep, zlib_dep] +iceberg_deps = [nanoarrow_dep, nlohmann_json_dep, spdlog_dep, zlib_dep] iceberg_lib = library( 'iceberg', diff --git a/subprojects/spdlog.wrap b/subprojects/spdlog.wrap new file mode 100644 index 000000000..f17351f9b --- /dev/null +++ b/subprojects/spdlog.wrap @@ -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. + +[wrap-file] +directory = spdlog-1.15.3 +source_url = https://github.com/gabime/spdlog/archive/refs/tags/v1.15.3.tar.gz +source_filename = spdlog-1.15.3.tar.gz +source_hash = 15a04e69c222eb6c01094b5c7ff8a249b36bb22788d72519646fb85feb267e67 +patch_filename = spdlog_1.15.3-4_patch.zip +patch_url = https://wrapdb.mesonbuild.com/v2/spdlog_1.15.3-4/get_patch +patch_hash = ccdc72f3d965980d5edd1a56129a9b7fa5f7c86f31e4ecf2dba6a6068829d4e2 +source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/spdlog_1.15.3-4/spdlog-1.15.3.tar.gz +wrapdb_version = 1.15.3-4 + +[provide] +spdlog = spdlog_dep From 173aa0c13bcbdd986102c677866ba93f85461ec4 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Wed, 10 Jun 2026 17:17:05 +0000 Subject: [PATCH 2/6] Add log levels for the new logging system This is the first of six small PRs that together add a logging system to iceberg-cpp. It introduces LogLevel, the severity scale everything else builds on: trace, debug, info, warn, error, critical, fatal, plus an `off` sentinel for turning logging off entirely. Levels are ordered from most to least verbose, so deciding whether something should be logged is a plain `level >= threshold` comparison. Two helpers come with it: ToString to print a level, and LogLevelFromString to parse one (case-insensitive, returning a Result for unrecognized input). Both follow the same shape as CounterUnit in metrics/counter.h. There is no implementation behind the header yet, so this PR only adds the header, wires the new src/iceberg/logging directory into the build, and adds a test covering the round-trip, ordering, and parsing. Co-authored-by: Isaac --- src/iceberg/CMakeLists.txt | 1 + src/iceberg/logging/CMakeLists.txt | 18 ++++++ src/iceberg/logging/log_level.h | 91 ++++++++++++++++++++++++++++++ src/iceberg/test/CMakeLists.txt | 4 ++ src/iceberg/test/log_level_test.cc | 78 +++++++++++++++++++++++++ 5 files changed, 192 insertions(+) create mode 100644 src/iceberg/logging/CMakeLists.txt create mode 100644 src/iceberg/logging/log_level.h create mode 100644 src/iceberg/test/log_level_test.cc diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 0fe418d48..ba51b57cb 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -239,6 +239,7 @@ add_subdirectory(row) add_subdirectory(update) add_subdirectory(util) add_subdirectory(metrics) +add_subdirectory(logging) if(ICEBERG_BUILD_BUNDLE) set(ICEBERG_BUNDLE_SOURCES diff --git a/src/iceberg/logging/CMakeLists.txt b/src/iceberg/logging/CMakeLists.txt new file mode 100644 index 000000000..75b869908 --- /dev/null +++ b/src/iceberg/logging/CMakeLists.txt @@ -0,0 +1,18 @@ +# 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. + +iceberg_install_all_headers(iceberg/logging) diff --git a/src/iceberg/logging/log_level.h b/src/iceberg/logging/log_level.h new file mode 100644 index 000000000..65f174dd9 --- /dev/null +++ b/src/iceberg/logging/log_level.h @@ -0,0 +1,91 @@ +/* + * 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/log_level.h +/// \brief Severity levels for the logging system. + +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/result.h" +#include "iceberg/util/string_util.h" + +namespace iceberg { + +/// \brief Logging severity level, ordered from most to least verbose. +/// +/// Levels are ordered so that `level >= threshold` is the enabled test. +/// `kOff` is the maximum sentinel: as a threshold it disables all emission +/// (it is never the level of an actual message). +enum class LogLevel { + kTrace, + kDebug, + kInfo, + kWarn, + kError, + kCritical, + kFatal, + kOff, +}; + +/// \brief String representation of a LogLevel. +ICEBERG_EXPORT constexpr std::string_view ToString(LogLevel level) noexcept { + 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::kFatal: + return "fatal"; + case LogLevel::kOff: + return "off"; + } + std::unreachable(); +} + +/// \brief Parse a LogLevel from a string (case-insensitive). +/// +/// \param s The string to parse ("trace", "debug", "info", "warn", "error", +/// "critical", "fatal", or "off"). +/// \return The LogLevel, or an InvalidArgument error if unrecognized. +ICEBERG_EXPORT inline Result LogLevelFromString(std::string_view s) { + auto level = StringUtils::ToLower(s); + if (level == "trace") return LogLevel::kTrace; + if (level == "debug") return LogLevel::kDebug; + if (level == "info") return LogLevel::kInfo; + if (level == "warn") return LogLevel::kWarn; + if (level == "error") return LogLevel::kError; + if (level == "critical") return LogLevel::kCritical; + if (level == "fatal") return LogLevel::kFatal; + if (level == "off") return LogLevel::kOff; + return InvalidArgument("Invalid log level: {}", s); +} + +} // namespace iceberg diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 2d56d7f35..728bf9240 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -101,6 +101,10 @@ add_iceberg_test(table_test table_test.cc table_update_test.cc) +add_iceberg_test(logging_test + SOURCES + log_level_test.cc) + add_iceberg_test(expression_test SOURCES aggregate_test.cc diff --git a/src/iceberg/test/log_level_test.cc b/src/iceberg/test/log_level_test.cc new file mode 100644 index 000000000..e4c25bf67 --- /dev/null +++ b/src/iceberg/test/log_level_test.cc @@ -0,0 +1,78 @@ +/* + * 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/log_level.h" + +#include +#include + +#include + +namespace iceberg { + +namespace { + +constexpr std::array kAllLevels = { + LogLevel::kTrace, LogLevel::kDebug, LogLevel::kInfo, LogLevel::kWarn, + LogLevel::kError, LogLevel::kCritical, LogLevel::kFatal, LogLevel::kOff}; + +} // namespace + +TEST(LogLevelTest, ToStringCoversEveryLevel) { + EXPECT_EQ(ToString(LogLevel::kTrace), "trace"); + EXPECT_EQ(ToString(LogLevel::kDebug), "debug"); + EXPECT_EQ(ToString(LogLevel::kInfo), "info"); + EXPECT_EQ(ToString(LogLevel::kWarn), "warn"); + EXPECT_EQ(ToString(LogLevel::kError), "error"); + EXPECT_EQ(ToString(LogLevel::kCritical), "critical"); + EXPECT_EQ(ToString(LogLevel::kFatal), "fatal"); + EXPECT_EQ(ToString(LogLevel::kOff), "off"); +} + +TEST(LogLevelTest, FromStringRoundTrips) { + for (LogLevel level : kAllLevels) { + auto parsed = LogLevelFromString(ToString(level)); + ASSERT_TRUE(parsed.has_value()) << "failed to parse " << ToString(level); + EXPECT_EQ(parsed.value(), level); + } +} + +TEST(LogLevelTest, FromStringIsCaseInsensitive) { + EXPECT_EQ(LogLevelFromString("WARN").value(), LogLevel::kWarn); + EXPECT_EQ(LogLevelFromString("Warn").value(), LogLevel::kWarn); + EXPECT_EQ(LogLevelFromString("CRITICAL").value(), LogLevel::kCritical); +} + +TEST(LogLevelTest, FromStringRejectsUnknown) { + auto parsed = LogLevelFromString("verbose"); + ASSERT_FALSE(parsed.has_value()); + EXPECT_EQ(parsed.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(LogLevelTest, OrderingIsMonotonicWithOffAsMaximum) { + EXPECT_LT(LogLevel::kTrace, LogLevel::kDebug); + EXPECT_LT(LogLevel::kDebug, LogLevel::kInfo); + EXPECT_LT(LogLevel::kInfo, LogLevel::kWarn); + EXPECT_LT(LogLevel::kWarn, LogLevel::kError); + EXPECT_LT(LogLevel::kError, LogLevel::kCritical); + EXPECT_LT(LogLevel::kCritical, LogLevel::kFatal); + EXPECT_LT(LogLevel::kFatal, LogLevel::kOff); +} + +} // namespace iceberg From f1e8530a0ee78864e87cd76c568ff254cf267447 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Wed, 10 Jun 2026 17:19:26 +0000 Subject: [PATCH 3/6] Add the Logger interface and the process-wide default logger Second of the logging stack. This defines what a logger is and where the rest of the code finds one, with no concrete backend yet. Logger is a small abstract interface: ShouldLog to test a level, Log to emit an already-formatted record, SetLevel/level, Flush, and a shared no-op instance. Records are passed as LogMessage, which owns its formatted text (so a sink may safely hold onto it) and carries the source location plus a reserved slot for structured key/values we may add later. The interface spells out two rules for implementations: never call back into logging from inside Log/Flush, and keep level() a true lower bound for ShouldLog. There is also one default logger per process, designed to be cheap to reach on the logging path: a lock-free atomic level check drops disabled messages outright, and when a message does need logging the current logger comes from a thread-local cache that only refreshes when the default actually changes, so the steady state has no lock or reference-count traffic. SetDefaultLogger and SetDefaultLevel swap it safely under a mutex. The seed default is the no-op logger; CerrLogger and the spdlog backend arrive in later PRs. Also adds the build-generated, .cc-only config header used later to pick the backend, and tests for the interface and the default-logger lifecycle. Co-authored-by: Isaac --- CMakeLists.txt | 1 + src/iceberg/CMakeLists.txt | 13 +++ src/iceberg/logging/config.h.in | 30 +++++ src/iceberg/logging/logger.cc | 127 +++++++++++++++++++++ src/iceberg/logging/logger.h | 143 ++++++++++++++++++++++++ src/iceberg/logging/meson.build | 37 ++++++ src/iceberg/meson.build | 5 + src/iceberg/test/CMakeLists.txt | 3 +- src/iceberg/test/logger_test.cc | 81 ++++++++++++++ src/iceberg/test/logging_test_helpers.h | 77 +++++++++++++ 10 files changed, 516 insertions(+), 1 deletion(-) create mode 100644 src/iceberg/logging/config.h.in create mode 100644 src/iceberg/logging/logger.cc create mode 100644 src/iceberg/logging/logger.h create mode 100644 src/iceberg/logging/meson.build create mode 100644 src/iceberg/test/logger_test.cc create mode 100644 src/iceberg/test/logging_test_helpers.h diff --git a/CMakeLists.txt b/CMakeLists.txt index b03e586b0..9f3f936ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,6 +53,7 @@ option(ICEBERG_SQL_SQLITE "Build the SQLite connector for the SQL catalog" OFF) option(ICEBERG_SQL_POSTGRESQL "Build the PostgreSQL connector for the SQL catalog" OFF) option(ICEBERG_SQL_MYSQL "Build the MySQL connector for the SQL catalog" OFF) option(ICEBERG_S3 "Build with S3 support" OFF) +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/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index ba51b57cb..3af02bab3 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 @@ -44,6 +56,7 @@ set(ICEBERG_SOURCES inheritable_metadata.cc json_serde.cc location_provider.cc + logging/logger.cc manifest/manifest_adapter.cc manifest/manifest_entry.cc manifest/manifest_filter_manager.cc 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/logger.cc b/src/iceberg/logging/logger.cc new file mode 100644 index 000000000..b13d7f5ff --- /dev/null +++ b/src/iceberg/logging/logger.cc @@ -0,0 +1,127 @@ +/* + * 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/logger.h" + +#include +#include +#include +#include +#include + +namespace iceberg { + +namespace { + +/// \brief Logger that drops every record. +class NoopLogger final : public Logger { + public: + bool ShouldLog(LogLevel /*level*/) const override { return false; } + void Log(LogMessage&& /*message*/) noexcept override {} + void SetLevel(LogLevel /*level*/) override {} + LogLevel level() const override { return LogLevel::kOff; } + bool IsNoop() const override { return true; } +}; + +/// \brief Construct the process default logger for this build configuration. +/// +/// Block 2 defaults to the no-op logger; Block 3 switches this to CerrLogger and +/// Block 5 wraps the selection in `#ifdef ICEBERG_HAS_SPDLOG` to prefer SpdLogger. +std::shared_ptr MakeDefaultLogger() { return Logger::Noop(); } + +/// \brief The process-global default-logger slot. +struct DefaultSlot { + std::mutex mtx; + std::shared_ptr logger; + // Seeded to 1 so a fresh thread (tls_gen == 0) always refreshes on first use. + std::atomic gen{1}; + + DefaultSlot() : logger(MakeDefaultLogger()) {} +}; + +/// \brief Immortal (leaked, hence reachable -> LSan-clean) accessor for the slot. +DefaultSlot& Slot() { + static DefaultSlot* slot = new DefaultSlot(); + return *slot; +} + +/// \brief Cached effective minimum level (lock-free fast-path gate). +/// +/// Constant-initialized permissive (kTrace); seeded to the default logger's +/// level() on first slot use and on every Set*. As a lower bound it may only +/// admit extra calls through to the authoritative Logger::ShouldLog. +std::atomic g_effective_level{LogLevel::kTrace}; + +} // namespace + +std::shared_ptr Logger::Noop() { + // Intentionally leaked: reachable via the function-local static (LSan-clean) + // and never destroyed, so logging during static teardown stays safe. + static std::shared_ptr* instance = + new std::shared_ptr(std::make_shared()); + return *instance; +} + +std::shared_ptr GetDefaultLogger() { + DefaultSlot& slot = Slot(); + std::lock_guard lock(slot.mtx); + return slot.logger; +} + +void SetDefaultLogger(std::shared_ptr logger) { + if (!logger) { + logger = Logger::Noop(); + } + DefaultSlot& slot = Slot(); + std::lock_guard lock(slot.mtx); + g_effective_level.store(logger->level(), std::memory_order_relaxed); + slot.logger = std::move(logger); + // Publish the swap; the mutex provides the happens-before, gen is a detector. + slot.gen.fetch_add(1, std::memory_order_relaxed); +} + +void SetDefaultLevel(LogLevel level) { + DefaultSlot& slot = Slot(); + std::lock_guard lock(slot.mtx); + slot.logger->SetLevel(level); + g_effective_level.store(level, std::memory_order_relaxed); +} + +namespace detail { + +LogLevel EffectiveLevel() noexcept { + return g_effective_level.load(std::memory_order_relaxed); +} + +const std::shared_ptr& CurrentLogger() noexcept { + static thread_local std::shared_ptr tls; + static thread_local uint64_t tls_gen = 0; + DefaultSlot& slot = Slot(); + uint64_t current = slot.gen.load(std::memory_order_relaxed); + if (current != tls_gen) { + std::lock_guard lock(slot.mtx); + tls = slot.logger; + tls_gen = current; + } + return tls; +} + +} // namespace detail + +} // namespace iceberg diff --git a/src/iceberg/logging/logger.h b/src/iceberg/logging/logger.h new file mode 100644 index 000000000..f7e4e3962 --- /dev/null +++ b/src/iceberg/logging/logger.h @@ -0,0 +1,143 @@ +/* + * 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/logger.h +/// \brief Pluggable logging interface and the process-global default logger. +/// +/// This header is backend-agnostic: it never includes the build-generated +/// backend configuration header and never references the spdlog feature macro, +/// so consumers see one stable API regardless of how the backend was configured. + +#include +#include +#include +#include +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/logging/log_level.h" +#include "iceberg/result.h" + +namespace iceberg { + +/// \brief A structured key/value attribute attached to a log record. +/// +/// Both key and value are owned so a sink may retain the record safely. +/// Unused in v1; reserved so structured logging can be added without an ABI +/// break to LogMessage. +struct LogAttribute { + std::string key; + std::string value; +}; + +/// \brief A single log record handed to a Logger. +/// +/// The formatted message is owned (moved in by the logging macros), so a sink +/// may safely retain the record beyond the Log() call. The member set must not +/// depend on the build's logging backend (the spdlog backend never appears here). +struct LogMessage { + LogLevel level; + std::string message; + std::source_location location; + std::vector attributes; +}; + +/// \brief Pluggable logging sink. +/// +/// Implementations must be thread-safe and must not throw. They must also obey: +/// - No reentrancy: Log()/Flush() must not call the logging macros or +/// GetDefaultLogger() (UB -- deadlock with mutex-based sinks). +/// - Lower-bound level: ShouldLog(l) must imply l >= level(), i.e. level() +/// summarizes ShouldLog(), because the global fast-path gate reflects only +/// level(). A logger needing finer logic must report its most permissive +/// threshold from level(). +class ICEBERG_EXPORT Logger { + public: + virtual ~Logger() = default; + + /// \brief Property-based setup, called by Loggers::Load() before first use. + /// + /// The default is a no-op. Recognized properties include "level" (parsed via + /// LogLevelFromString) and, for formatting sinks, "pattern". + virtual Status Initialize( + [[maybe_unused]] const std::unordered_map& properties) { + return {}; + } + + /// \brief Cheap check whether a record at \p level would be emitted. + virtual bool ShouldLog(LogLevel level) const = 0; + + /// \brief Emit one (already-formatted) record, taking ownership. Must not throw. + virtual void Log(LogMessage&& message) noexcept = 0; + + /// \brief Set the minimum level this logger emits. + virtual void SetLevel(LogLevel level) = 0; + + /// \brief Return the minimum level this logger emits. + virtual LogLevel level() const = 0; + + /// \brief Flush any buffered output. Must not throw; best-effort on the fatal path. + virtual void Flush() noexcept {} + + /// \brief Return true if this logger is a no-op. + virtual bool IsNoop() const { return false; } + + /// \brief Return a shared, immortal no-op logger singleton. + static std::shared_ptr Noop(); +}; + +/// \brief Return the process-global default logger (never null). +/// +/// Off the hot path -- acquires the slot lock and returns an owning copy. The +/// logging macros use the cheaper internal hot-path accessor instead. +ICEBERG_EXPORT std::shared_ptr GetDefaultLogger(); + +/// \brief Install a new process-global default logger. +/// +/// A null argument installs the no-op logger. Thread-safe; intended for +/// occasional (configuration-time) use rather than the hot path. +ICEBERG_EXPORT void SetDefaultLogger(std::shared_ptr logger); + +/// \brief Set the minimum level of the current default logger. +/// +/// Updates both the logger and the lock-free fast-path gate atomically. +ICEBERG_EXPORT void SetDefaultLevel(LogLevel level); + +namespace detail { + +/// \brief Lock-free read of the cached effective minimum level. +/// +/// This is a non-authoritative lower-bound early-out: it may admit extra calls +/// through to the authoritative Logger::ShouldLog, but never wrongly rejects. +ICEBERG_EXPORT LogLevel EffectiveLevel() noexcept; + +/// \brief Hot-path accessor for the default logger. +/// +/// Returns a reference to a thread-local cached shared_ptr that is refreshed +/// only when the default logger has changed (no lock / no refcount churn in +/// steady state). The reference is valid for the duration of the calling +/// statement. +ICEBERG_EXPORT const std::shared_ptr& CurrentLogger() noexcept; + +} // namespace detail + +} // namespace iceberg diff --git a/src/iceberg/logging/meson.build b/src/iceberg/logging/meson.build new file mode 100644 index 000000000..23b53a444 --- /dev/null +++ b/src/iceberg/logging/meson.build @@ -0,0 +1,37 @@ +# 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. + +# 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( + [ + 'log_level.h', + 'logger.h', + ], + subdir: 'iceberg/logging', +) diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index 8c4b49e9b..7bd89a642 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -38,6 +38,10 @@ 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('..') iceberg_sources = files( 'arrow_c_data_guard_internal.cc', @@ -66,6 +70,7 @@ iceberg_sources = files( 'inheritable_metadata.cc', 'json_serde.cc', 'location_provider.cc', + 'logging/logger.cc', 'manifest/manifest_adapter.cc', 'manifest/manifest_entry.cc', 'manifest/manifest_filter_manager.cc', diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 728bf9240..4fbb2cd2f 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -103,7 +103,8 @@ add_iceberg_test(table_test add_iceberg_test(logging_test SOURCES - log_level_test.cc) + log_level_test.cc + logger_test.cc) add_iceberg_test(expression_test SOURCES diff --git a/src/iceberg/test/logger_test.cc b/src/iceberg/test/logger_test.cc new file mode 100644 index 000000000..9696869f1 --- /dev/null +++ b/src/iceberg/test/logger_test.cc @@ -0,0 +1,81 @@ +/* + * 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/logger.h" + +#include + +#include + +#include "iceberg/logging/log_level.h" +#include "iceberg/test/logging_test_helpers.h" + +namespace iceberg { + +TEST(LoggerTest, NoopIsSharedImmortalAndSilent) { + auto noop = Logger::Noop(); + ASSERT_NE(noop, nullptr); + EXPECT_TRUE(noop->IsNoop()); + EXPECT_FALSE(noop->ShouldLog(LogLevel::kFatal)); + EXPECT_EQ(noop->level(), LogLevel::kOff); + // Same singleton instance every call. + EXPECT_EQ(noop.get(), Logger::Noop().get()); +} + +TEST(LoggerTest, DefaultLoggerIsNeverNull) { EXPECT_NE(GetDefaultLogger(), nullptr); } + +TEST(LoggerTest, SetAndGetDefaultLogger) { + auto capturing = std::make_shared(); + ScopedDefaultLogger guard(capturing); + EXPECT_EQ(GetDefaultLogger().get(), capturing.get()); + EXPECT_EQ(detail::CurrentLogger().get(), capturing.get()); +} + +TEST(LoggerTest, SetNullFallsBackToNoop) { + ScopedDefaultLogger guard(std::make_shared()); + SetDefaultLogger(nullptr); + EXPECT_TRUE(GetDefaultLogger()->IsNoop()); +} + +TEST(LoggerTest, CurrentLoggerTracksSwaps) { + auto first = std::make_shared(); + auto second = std::make_shared(); + ScopedDefaultLogger guard(first); + EXPECT_EQ(detail::CurrentLogger().get(), first.get()); + SetDefaultLogger(second); + // Generation bump must invalidate the thread-local cache. + EXPECT_EQ(detail::CurrentLogger().get(), second.get()); +} + +TEST(LoggerTest, SetDefaultLevelSyncsGateAndLogger) { + auto capturing = std::make_shared(); + ScopedDefaultLogger guard(capturing); + SetDefaultLevel(LogLevel::kError); + EXPECT_EQ(detail::EffectiveLevel(), LogLevel::kError); + EXPECT_EQ(capturing->level(), LogLevel::kError); +} + +TEST(LoggerTest, SetDefaultLoggerSeedsGateFromLoggerLevel) { + auto capturing = std::make_shared(); + capturing->SetLevel(LogLevel::kWarn); + ScopedDefaultLogger guard(capturing); + EXPECT_EQ(detail::EffectiveLevel(), LogLevel::kWarn); +} + +} // namespace iceberg diff --git a/src/iceberg/test/logging_test_helpers.h b/src/iceberg/test/logging_test_helpers.h new file mode 100644 index 000000000..00a57cf82 --- /dev/null +++ b/src/iceberg/test/logging_test_helpers.h @@ -0,0 +1,77 @@ +/* + * 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 "iceberg/logging/logger.h" + +namespace iceberg { + +/// \brief Test sink that records every emitted LogMessage under a mutex. +class CapturingLogger : public Logger { + public: + bool ShouldLog(LogLevel level) const override { return level >= level_; } + + void Log(LogMessage&& message) noexcept override { + std::lock_guard lock(mutex_); + records_.push_back(std::move(message)); + } + + void SetLevel(LogLevel level) override { level_ = level; } + LogLevel level() const override { return level_; } + + std::vector records() const { + std::lock_guard lock(mutex_); + return records_; + } + + std::size_t count() const { + std::lock_guard lock(mutex_); + return records_.size(); + } + + private: + mutable std::mutex mutex_; + LogLevel level_ = LogLevel::kTrace; + std::vector records_; +}; + +/// \brief RAII guard that restores the process default logger on scope exit, so +/// tests that swap the global default don't leak state into other tests. +class ScopedDefaultLogger { + public: + explicit ScopedDefaultLogger(std::shared_ptr logger) + : previous_(GetDefaultLogger()) { + SetDefaultLogger(std::move(logger)); + } + ~ScopedDefaultLogger() { SetDefaultLogger(previous_); } + + ScopedDefaultLogger(const ScopedDefaultLogger&) = delete; + ScopedDefaultLogger& operator=(const ScopedDefaultLogger&) = delete; + + private: + std::shared_ptr previous_; +}; + +} // namespace iceberg From 6e2b6c1bfe649d944874027c11a0e952a42ea144 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Wed, 10 Jun 2026 17:20:18 +0000 Subject: [PATCH 4/6] Add CerrLogger, a std::cerr logging backend Third of the logging stack. This is the first concrete backend, and the default sink when spdlog is not compiled in. CerrLogger writes one line per record to std::cerr, for example: 2026-06-10T18:06:17.518Z error [295021] table.cc:42] opened snapshot 7 a UTC ISO-8601 timestamp with millisecond precision, the level, the OS thread id, the source file:line, then the message. The level threshold is a lock-free atomic, and a mutex guards each whole-line write so messages from different threads never interleave. Log and Flush wrap their work in try/catch so a logging call never throws, honoring the interface's noexcept contract. Being pure standard library, it is always compiled and becomes the seed default logger, replacing the no-op placeholder from the previous PR. Adds tests for the line format, level filtering, and concurrent writes. Co-authored-by: Isaac --- src/iceberg/CMakeLists.txt | 1 + src/iceberg/logging/cerr_logger.cc | 105 +++++++++++++++++++++++++ src/iceberg/logging/cerr_logger.h | 59 ++++++++++++++ src/iceberg/logging/logger.cc | 8 +- src/iceberg/logging/meson.build | 1 + src/iceberg/meson.build | 1 + src/iceberg/test/CMakeLists.txt | 1 + src/iceberg/test/cerr_logger_test.cc | 111 +++++++++++++++++++++++++++ 8 files changed, 284 insertions(+), 3 deletions(-) create mode 100644 src/iceberg/logging/cerr_logger.cc create mode 100644 src/iceberg/logging/cerr_logger.h create mode 100644 src/iceberg/test/cerr_logger_test.cc diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 3af02bab3..4f100d64e 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -56,6 +56,7 @@ set(ICEBERG_SOURCES inheritable_metadata.cc json_serde.cc location_provider.cc + logging/cerr_logger.cc logging/logger.cc manifest/manifest_adapter.cc manifest/manifest_entry.cc diff --git a/src/iceberg/logging/cerr_logger.cc b/src/iceberg/logging/cerr_logger.cc new file mode 100644 index 000000000..33d86b9e6 --- /dev/null +++ b/src/iceberg/logging/cerr_logger.cc @@ -0,0 +1,105 @@ +/* + * 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/cerr_logger.h" + +#include +#include +#include +#include +#include +#include +#include + +#if defined(_WIN32) +# include +#elif defined(__APPLE__) +# include +#else +# include + +# include +#endif + +namespace iceberg { + +namespace { + +/// \brief OS-native thread id, cached per thread to avoid a syscall per log. +/// +/// Matches the cross-process-correlatable id used by spdlog/glog (not the opaque +/// std::thread::id), and avoids the std::formatter (P2693) +/// minimum-toolchain dependency. +uint64_t OsThreadId() noexcept { + static thread_local uint64_t tid = []() -> uint64_t { +#if defined(_WIN32) + return static_cast(::GetCurrentThreadId()); +#elif defined(__APPLE__) + uint64_t id = 0; + pthread_threadid_np(nullptr, &id); + return id; +#else + return static_cast(::syscall(SYS_gettid)); +#endif + }(); + return tid; +} + +/// \brief Trailing path component of a source file path. +std::string_view Basename(std::string_view path) noexcept { + auto pos = path.find_last_of("/\\"); + return pos == std::string_view::npos ? path : path.substr(pos + 1); +} + +/// \brief Format a record into a single newline-terminated line. +std::string FormatLine(const LogMessage& message) { + auto now = + std::chrono::floor(std::chrono::system_clock::now()); + return std::format("{:%Y-%m-%dT%H:%M:%S}Z {} [{}] {}:{}] {}\n", now, + ToString(message.level), OsThreadId(), + Basename(message.location.file_name()), message.location.line(), + message.message); +} + +} // namespace + +void CerrLogger::Log(LogMessage&& message) noexcept { + try { + std::string line = FormatLine(message); + std::lock_guard lock(mutex_); + std::cerr << line; + } catch (...) { + // Logging must never throw. Best-effort fallback, swallow any failure. + try { + std::lock_guard lock(mutex_); + std::cerr << "\n"; + } catch (...) { + } + } +} + +void CerrLogger::Flush() noexcept { + try { + std::lock_guard lock(mutex_); + std::cerr.flush(); + } catch (...) { + } +} + +} // namespace iceberg diff --git a/src/iceberg/logging/cerr_logger.h b/src/iceberg/logging/cerr_logger.h new file mode 100644 index 000000000..15af69625 --- /dev/null +++ b/src/iceberg/logging/cerr_logger.h @@ -0,0 +1,59 @@ +/* + * 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/cerr_logger.h +/// \brief Always-available std::cerr logging backend. + +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/logging/log_level.h" +#include "iceberg/logging/logger.h" + +namespace iceberg { + +/// \brief Logger that writes one line per record to std::cerr. +/// +/// Line layout: `YYYY-MM-DDThh:mm:ss.mmmZ LEVEL [tid] file:line] message`. +/// The minimum level is held in a lock-free atomic; a mutex serializes the +/// whole-line write so concurrent records never interleave. Pure standard +/// library -- always compiled, regardless of ICEBERG_SPDLOG. +class ICEBERG_EXPORT CerrLogger : public Logger { + public: + explicit CerrLogger(LogLevel level = LogLevel::kInfo) : level_(level) {} + + bool ShouldLog(LogLevel level) const override { + return level >= level_.load(std::memory_order_relaxed); + } + void Log(LogMessage&& message) noexcept override; + void SetLevel(LogLevel level) override { + level_.store(level, std::memory_order_relaxed); + } + LogLevel level() const override { return level_.load(std::memory_order_relaxed); } + void Flush() noexcept override; + + private: + std::atomic level_; + std::mutex mutex_; +}; + +} // namespace iceberg diff --git a/src/iceberg/logging/logger.cc b/src/iceberg/logging/logger.cc index b13d7f5ff..e0f46a89b 100644 --- a/src/iceberg/logging/logger.cc +++ b/src/iceberg/logging/logger.cc @@ -25,6 +25,8 @@ #include #include +#include "iceberg/logging/cerr_logger.h" + namespace iceberg { namespace { @@ -41,9 +43,9 @@ class NoopLogger final : public Logger { /// \brief Construct the process default logger for this build configuration. /// -/// Block 2 defaults to the no-op logger; Block 3 switches this to CerrLogger and -/// Block 5 wraps the selection in `#ifdef ICEBERG_HAS_SPDLOG` to prefer SpdLogger. -std::shared_ptr MakeDefaultLogger() { return Logger::Noop(); } +/// Defaults to the always-available std::cerr logger. Block 5 wraps this in +/// `#ifdef ICEBERG_HAS_SPDLOG` to prefer SpdLogger when spdlog is compiled in. +std::shared_ptr MakeDefaultLogger() { return std::make_shared(); } /// \brief The process-global default-logger slot. struct DefaultSlot { diff --git a/src/iceberg/logging/meson.build b/src/iceberg/logging/meson.build index 23b53a444..b08aee7e8 100644 --- a/src/iceberg/logging/meson.build +++ b/src/iceberg/logging/meson.build @@ -30,6 +30,7 @@ configure_file( # SpdLogger header are intentionally NOT installed. install_headers( [ + 'cerr_logger.h', 'log_level.h', 'logger.h', ], diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index 7bd89a642..59b71a38d 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -70,6 +70,7 @@ iceberg_sources = files( 'inheritable_metadata.cc', 'json_serde.cc', 'location_provider.cc', + 'logging/cerr_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 4fbb2cd2f..13a64ea01 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -103,6 +103,7 @@ add_iceberg_test(table_test add_iceberg_test(logging_test SOURCES + cerr_logger_test.cc log_level_test.cc logger_test.cc) diff --git a/src/iceberg/test/cerr_logger_test.cc b/src/iceberg/test/cerr_logger_test.cc new file mode 100644 index 000000000..126698307 --- /dev/null +++ b/src/iceberg/test/cerr_logger_test.cc @@ -0,0 +1,111 @@ +/* + * 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/cerr_logger.h" + +#include +#include +#include +#include +#include +#include + +#include + +#include "iceberg/logging/log_level.h" +#include "iceberg/logging/logger.h" + +namespace iceberg { + +namespace { + +/// \brief RAII redirect of std::cerr to a stringstream for the test scope. +class CerrCapture { + public: + CerrCapture() : old_(std::cerr.rdbuf(buffer_.rdbuf())) {} + ~CerrCapture() { std::cerr.rdbuf(old_); } + std::string str() const { return buffer_.str(); } + + private: + std::ostringstream buffer_; + std::streambuf* old_; +}; + +LogMessage MakeMessage(LogLevel level, std::string text) { + return LogMessage{.level = level, + .message = std::move(text), + .location = std::source_location::current(), + .attributes = {}}; +} + +} // namespace + +TEST(CerrLoggerTest, DefaultLevelIsInfo) { + CerrLogger logger; + EXPECT_EQ(logger.level(), LogLevel::kInfo); + EXPECT_FALSE(logger.ShouldLog(LogLevel::kDebug)); + EXPECT_TRUE(logger.ShouldLog(LogLevel::kInfo)); + EXPECT_TRUE(logger.ShouldLog(LogLevel::kError)); +} + +TEST(CerrLoggerTest, SetLevelFilters) { + CerrLogger logger(LogLevel::kError); + EXPECT_FALSE(logger.ShouldLog(LogLevel::kWarn)); + logger.SetLevel(LogLevel::kTrace); + EXPECT_TRUE(logger.ShouldLog(LogLevel::kTrace)); +} + +TEST(CerrLoggerTest, LineContainsLevelAndMessage) { + CerrLogger logger; + CerrCapture capture; + logger.Log(MakeMessage(LogLevel::kError, "boom 42")); + std::string out = capture.str(); + EXPECT_NE(out.find("error"), std::string::npos); + EXPECT_NE(out.find("boom 42"), std::string::npos); + EXPECT_NE(out.find("cerr_logger_test.cc"), std::string::npos); + EXPECT_EQ(out.back(), '\n'); +} + +TEST(CerrLoggerTest, ConcurrentLogsDoNotInterleave) { + CerrLogger logger(LogLevel::kTrace); + CerrCapture capture; + constexpr int kThreads = 8; + constexpr int kPerThread = 50; + + std::vector threads; + for (int t = 0; t < kThreads; ++t) { + threads.emplace_back([&logger] { + for (int i = 0; i < kPerThread; ++i) { + logger.Log(MakeMessage(LogLevel::kInfo, "line")); + } + }); + } + for (auto& thread : threads) thread.join(); + + // Every record is exactly one well-formed line; no interleaving means the + // line count equals the record count. + std::string out = capture.str(); + int newlines = 0; + for (char c : out) { + if (c == '\n') ++newlines; + } + EXPECT_EQ(newlines, kThreads * kPerThread); +} + +} // namespace iceberg From a90813201f6f70aa2e78158a97386c35cfce0fef Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Wed, 10 Jun 2026 17:20:55 +0000 Subject: [PATCH 5/6] Add the logging macros Fourth of the logging stack. This is the API application code actually uses; the earlier PRs were the machinery behind it. The fixed-severity macros (ICEBERG_LOG_TRACE / _DEBUG / _INFO / _WARN / _ERROR / _CRITICAL) take a std::format string and arguments, e.g. ICEBERG_LOG_INFO("opened {} in {} ms", name, elapsed); Each is cheap when it won't print: a compile-time floor (ICEBERG_LOG_ACTIVE_LEVEL) drops calls below the configured level out of the binary entirely, and at runtime a lock-free level check skips the work before the arguments are evaluated. The format string is validated at compile time and the source location is captured automatically. ICEBERG_LOG_FATAL logs, flushes, and aborts, so code after it is unreachable (matching glog/abseil). ICEBERG_LOG(level, ...) and ICEBERG_LOG_TO(logger, level, ...) cover a runtime-chosen level and an explicit target; ICEBERG_LOG_RUNTIME_FMT handles a non-literal format string. Formatting failures cannot escape a log call -- they fall back to a fixed marker rather than throwing. Short LOG_* aliases are opt-in via ICEBERG_LOG_SHORT_MACROS to avoid clashing with other libraries. Tests cover formatting, level gating, disabled calls not evaluating their arguments, dangling-else safety, the never-throws fallback, compile-time stripping, and that FATAL aborts. Co-authored-by: Isaac --- src/iceberg/logging/logger.cc | 17 ++ src/iceberg/logging/logger.h | 176 ++++++++++++++++++- src/iceberg/test/CMakeLists.txt | 4 +- src/iceberg/test/macros_active_level_test.cc | 57 ++++++ src/iceberg/test/macros_test.cc | 130 ++++++++++++++ 5 files changed, 382 insertions(+), 2 deletions(-) create mode 100644 src/iceberg/test/macros_active_level_test.cc create mode 100644 src/iceberg/test/macros_test.cc diff --git a/src/iceberg/logging/logger.cc b/src/iceberg/logging/logger.cc index e0f46a89b..4090b6840 100644 --- a/src/iceberg/logging/logger.cc +++ b/src/iceberg/logging/logger.cc @@ -124,6 +124,23 @@ const std::shared_ptr& CurrentLogger() noexcept { return tls; } +void Emit(Logger& logger, LogLevel level, const std::source_location& location, + std::string&& message) { + logger.Log(LogMessage{.level = level, + .message = std::move(message), + .location = location, + .attributes = {}}); +} + +void EmitFormatError(Logger& logger, LogLevel level, + const std::source_location& location) noexcept { + // Fixed short literal (SSO -> no heap allocation), no std::format, no retry. + logger.Log(LogMessage{.level = level, + .message = std::string(""), + .location = location, + .attributes = {}}); +} + } // namespace detail } // namespace iceberg diff --git a/src/iceberg/logging/logger.h b/src/iceberg/logging/logger.h index f7e4e3962..2a60708f2 100644 --- a/src/iceberg/logging/logger.h +++ b/src/iceberg/logging/logger.h @@ -20,17 +20,21 @@ #pragma once /// \file iceberg/logging/logger.h -/// \brief Pluggable logging interface and the process-global default logger. +/// \brief Pluggable logging interface, the process-global default logger, and +/// the logging macros. /// /// This header is backend-agnostic: it never includes the build-generated /// backend configuration header and never references the spdlog feature macro, /// so consumers see one stable API regardless of how the backend was configured. +#include +#include #include #include #include #include #include +#include #include #include "iceberg/iceberg_export.h" @@ -138,6 +142,176 @@ ICEBERG_EXPORT LogLevel EffectiveLevel() noexcept; /// statement. ICEBERG_EXPORT const std::shared_ptr& CurrentLogger() noexcept; +/// \brief Build a LogMessage from the already-formatted text and dispatch it. +/// +/// Declared ICEBERG_EXPORT because the logging macros expand into this call in +/// consumer translation units. +ICEBERG_EXPORT void Emit(Logger& logger, LogLevel level, + const std::source_location& location, std::string&& message); + +/// \brief Emit a fixed fallback record when formatting threw. +/// +/// noexcept, allocation-light (small/SSO literal), performs no std::format, and +/// does not recurse -- so the macro's "logging never throws" guarantee holds +/// even when a format argument throws. +ICEBERG_EXPORT void EmitFormatError(Logger& logger, LogLevel level, + const std::source_location& location) noexcept; + +/// \brief Runtime (non-literal) format-string helper. +/// +/// std::format requires a compile-time format string; this routes a runtime +/// string through std::vformat. Args are bound as named lvalues and the +/// arg-store is held in a named variable so it outlives the vformat call +/// (C++23 make_format_args rejects rvalues -- P2905 / LWG3631). +template +std::string VFormat(std::string_view fmt, Args&&... args) { + auto store = std::make_format_args(args...); + return std::vformat(fmt, store); +} + } // namespace detail } // namespace iceberg + +/// \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 + lock-free gate + +// authoritative ShouldLog, formatting only on the taken path, never throwing. +#define ICEBERG_INTERNAL_LOG(level_, FMT_, ...) \ + do { \ + if constexpr ((level_) >= ICEBERG_LOG_ACTIVE_LEVEL) { \ + if ((level_) >= ::iceberg::detail::EffectiveLevel()) { \ + const auto& _ib_logger = ::iceberg::detail::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(level_)) { \ + try { \ + ::iceberg::detail::Emit(*_ib_logger, (level_), \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::detail::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. +#define ICEBERG_LOG_FATAL(FMT_, ...) \ + do { \ + if (::iceberg::LogLevel::kFatal >= ::iceberg::detail::EffectiveLevel()) { \ + const auto& _ib_logger = ::iceberg::detail::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(::iceberg::LogLevel::kFatal)) { \ + try { \ + ::iceberg::detail::Emit(*_ib_logger, ::iceberg::LogLevel::kFatal, \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::detail::EmitFormatError(*_ib_logger, ::iceberg::LogLevel::kFatal, \ + ::std::source_location::current()); \ + } \ + } \ + } \ + if (auto _ib_fatal = ::iceberg::GetDefaultLogger()) _ib_fatal->Flush(); \ + ::std::abort(); \ + } while (0) + +// Generic, runtime-level form against the default logger. No compile-time floor +// (the level is not a constant). Aborts when level == kFatal. +#define ICEBERG_LOG(level_, FMT_, ...) \ + do { \ + const ::iceberg::LogLevel _ib_lvl = (level_); \ + if (_ib_lvl >= ::iceberg::detail::EffectiveLevel()) { \ + const auto& _ib_logger = ::iceberg::detail::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(_ib_lvl)) { \ + try { \ + ::iceberg::detail::Emit(*_ib_logger, _ib_lvl, \ + ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::detail::EmitFormatError(*_ib_logger, _ib_lvl, \ + ::std::source_location::current()); \ + } \ + } \ + } \ + if (_ib_lvl == ::iceberg::LogLevel::kFatal) { \ + if (auto _ib_fatal = ::iceberg::GetDefaultLogger()) _ib_fatal->Flush(); \ + ::std::abort(); \ + } \ + } while (0) + +// Generic form targeting an EXPLICIT logger. Honors only that logger's +// ShouldLog -- never the default logger's global gate. 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::detail::Emit(_ib_logger, _ib_lvl, ::std::source_location::current(), \ + ::std::format(FMT_ __VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::detail::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. +#define ICEBERG_LOG_RUNTIME_FMT(level_, FMT_, ...) \ + do { \ + const ::iceberg::LogLevel _ib_lvl = (level_); \ + if (_ib_lvl >= ::iceberg::detail::EffectiveLevel()) { \ + const auto& _ib_logger = ::iceberg::detail::CurrentLogger(); \ + if (_ib_logger && _ib_logger->ShouldLog(_ib_lvl)) { \ + try { \ + ::iceberg::detail::Emit( \ + *_ib_logger, _ib_lvl, ::std::source_location::current(), \ + ::iceberg::detail::VFormat((FMT_)__VA_OPT__(, ) __VA_ARGS__)); \ + } catch (...) { \ + ::iceberg::detail::EmitFormatError(*_ib_logger, _ib_lvl, \ + ::std::source_location::current()); \ + } \ + } \ + } \ + if (_ib_lvl == ::iceberg::LogLevel::kFatal) { \ + if (auto _ib_fatal = ::iceberg::GetDefaultLogger()) _ib_fatal->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/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 13a64ea01..93466ead7 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -105,7 +105,9 @@ 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) 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..95ede353a --- /dev/null +++ b/src/iceberg/test/macros_active_level_test.cc @@ -0,0 +1,57 @@ +/* + * 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; + 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..74047248a --- /dev/null +++ b/src/iceberg/test/macros_test.cc @@ -0,0 +1,130 @@ +/* + * 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/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, RuntimeGateFiltersBelowEffectiveLevel) { + 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; + if (false) + ICEBERG_LOG_INFO("if-branch"); + else + took_else = true; + 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"); + }, + ""); +} + +} // namespace iceberg From c6831cbd4bc505df82228f7324b8723da3dee540 Mon Sep 17 00:00:00 2001 From: Kam Cheung Ting Date: Wed, 10 Jun 2026 17:22:08 +0000 Subject: [PATCH 6/6] Add the spdlog backend, selectable with ICEBERG_SPDLOG Fifth of the logging stack. Adds SpdLogger, a backend that forwards records to spdlog, and makes spdlog an optional dependency. SpdLogger maps our levels onto spdlog's (critical for both critical and fatal -- the process abort stays in the macro layer) and forwards the already-formatted message with its source location. It is synchronous only for now: spdlog's source_loc keeps raw pointers that are unsafe to hand to an async logger, so async is deliberately left out. The class lives under logging/internal and is not installed -- applications get it through the default logger or the "spdlog" registry entry, never by constructing it directly. spdlog is now behind the ICEBERG_SPDLOG option (on by default). When on, it is the default backend and its dependency/link are pulled in; when off, the library has no spdlog dependency at all and CerrLogger is the default. This supersedes the unconditional spdlog link restored by the earlier #459 revert. The choice is carried by the generated config.h (ICEBERG_HAS_SPDLOG) that only .cc files see, so the public headers stay backend-agnostic. Tests run on the spdlog path and compile to nothing when spdlog is off. Co-authored-by: Isaac --- .../IcebergThirdpartyToolchain.cmake | 4 +- src/iceberg/CMakeLists.txt | 20 ++-- src/iceberg/logging/internal/spdlog_logger.cc | 92 ++++++++++++++++++ src/iceberg/logging/internal/spdlog_logger.h | 75 +++++++++++++++ src/iceberg/logging/logger.cc | 18 +++- src/iceberg/meson.build | 1 + src/iceberg/test/CMakeLists.txt | 3 +- src/iceberg/test/spdlog_logger_test.cc | 94 +++++++++++++++++++ 8 files changed, 296 insertions(+), 11 deletions(-) create mode 100644 src/iceberg/logging/internal/spdlog_logger.cc create mode 100644 src/iceberg/logging/internal/spdlog_logger.h create mode 100644 src/iceberg/test/spdlog_logger_test.cc diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake index 6654e3312..939cbb15d 100644 --- a/cmake_modules/IcebergThirdpartyToolchain.cmake +++ b/cmake_modules/IcebergThirdpartyToolchain.cmake @@ -670,7 +670,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_BUILD_BUNDLE) resolve_arrow_dependency() diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 4f100d64e..7cae49279 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -153,24 +153,32 @@ 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>") + "$,iceberg::nlohmann_json,$,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>") 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>") + "$,iceberg::nlohmann_json,$,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>") + +# 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/internal/spdlog_logger.cc b/src/iceberg/logging/internal/spdlog_logger.cc new file mode 100644 index 000000000..9f8145d81 --- /dev/null +++ b/src/iceberg/logging/internal/spdlog_logger.cc @@ -0,0 +1,92 @@ +/* + * 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 + +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) {} + +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..625ae9e95 --- /dev/null +++ b/src/iceberg/logging/internal/spdlog_logger.h @@ -0,0 +1,75 @@ +/* + * 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). +class 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. + explicit SpdLogger(std::shared_ptr logger, + LogLevel level = LogLevel::kInfo); + + bool ShouldLog(LogLevel level) const override { + return level >= level_.load(std::memory_order_relaxed); + } + void Log(LogMessage&& message) noexcept override; + void SetLevel(LogLevel level) override { + level_.store(level, std::memory_order_relaxed); + } + LogLevel level() const 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 4090b6840..fb6e41efb 100644 --- a/src/iceberg/logging/logger.cc +++ b/src/iceberg/logging/logger.cc @@ -25,7 +25,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 { @@ -43,9 +49,15 @@ class NoopLogger final : public Logger { /// \brief Construct the process default logger for this build configuration. /// -/// Defaults to the always-available std::cerr logger. Block 5 wraps this in -/// `#ifdef ICEBERG_HAS_SPDLOG` to prefer SpdLogger when spdlog is compiled in. -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/meson.build b/src/iceberg/meson.build index 59b71a38d..8aad003f0 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -71,6 +71,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 93466ead7..b40042c3f 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -107,7 +107,8 @@ add_iceberg_test(logging_test log_level_test.cc logger_test.cc macros_active_level_test.cc - macros_test.cc) + macros_test.cc + spdlog_logger_test.cc) add_iceberg_test(expression_test SOURCES diff --git a/src/iceberg/test/spdlog_logger_test.cc b/src/iceberg/test/spdlog_logger_test.cc new file mode 100644 index 000000000..7e24c7157 --- /dev/null +++ b/src/iceberg/test/spdlog_logger_test.cc @@ -0,0 +1,94 @@ +/* + * 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 "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); +} + +} // namespace iceberg + +#endif // ICEBERG_HAS_SPDLOG