feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6) by kamcheungting-db · Pull Request #726 · apache/iceberg-cpp

kamcheungting-db · 2026-06-11T01:24:13Z

Part 5 of the logging stack (builds on #725). Adds the spdlog backend and the build option that selects it — the default in production builds.

What's here

SpdLogger wraps spdlog::logger (synchronous), maps levels (fatal/critical → spdlog critical; the abort stays in the macro layer), and forwards source location. Default sink is a colored stderr sink.
New ICEBERG_SPDLOG CMake option (ON by default). When OFF, the build has no spdlog dependency at all and CerrLogger is the default.
pattern property is honored here via spdlog set_pattern (the cerr backend keeps its fixed layout); level works on both backends.
SpdLogger lives in logging/internal/ and is not installed — apps get it via the default logger or the "spdlog" registry type.

Tests — spdlog_logger_test (compiled only when spdlog is ON): level mapping incl. fatal→critical, source-location forwarding, and the pattern property. clang/libc++ with spdlog 1.15.3.

This pull request and its description were written by Isaac.

Copilot

Pull request overview

This PR adds a selectable spdlog-backed logging sink to Iceberg C++ (guarded by ICEBERG_SPDLOG in CMake), introduces the public logging macro headers (log_macros.h / short_log_macros.h), and extends the test/build wiring to cover macro behavior and the spdlog backend.

Changes:

Add internal::SpdLogger (spdlog backend) and route the process default logger to it when ICEBERG_HAS_SPDLOG is enabled.
Introduce installed logging macro headers (ICEBERG_LOG_*, plus opt-in bare LOG_* aliases) and MSVC preprocessor flags needed for __VA_OPT__.
Add unit tests for logging macros and the spdlog backend; update CMake/Meson build definitions accordingly.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/iceberg/test/spdlog_logger_test.cc	Adds unit tests for the spdlog-backed logger behavior.
src/iceberg/test/meson.build	Includes the new logging-related tests in the Meson test target.
src/iceberg/test/macros_test.cc	Adds tests for formatting, level gating, runtime-format safety, and fatal-abort behavior in macros.
src/iceberg/test/macros_active_level_test.cc	Adds tests for compile-time log stripping via `ICEBERG_LOG_ACTIVE_LEVEL`.
src/iceberg/test/CMakeLists.txt	Wires the new tests into CMake and adds MSVC `/Zc:preprocessor` for test compilation.
src/iceberg/meson.build	Compiles the spdlog backend source in the Meson build.
src/iceberg/logging/short_log_macros.h	Adds opt-in bare `LOG_*` aliases for the Iceberg-prefixed macros.
src/iceberg/logging/meson.build	Generates `logging/config.h` and installs the new public logging headers in Meson.
src/iceberg/logging/logger.h	Extends the logging API docs and adds the `FatalHandler` hook API.
src/iceberg/logging/logger.cc	Selects spdlog vs `CerrLogger` as the default logger based on `ICEBERG_HAS_SPDLOG` and implements fatal-handler storage.
src/iceberg/logging/log_macros.h	Introduces the public logging macros and internal helpers used by those macros.
src/iceberg/logging/internal/spdlog_logger.h	Declares the internal spdlog-backed `SpdLogger` sink (not installed).
src/iceberg/logging/internal/spdlog_logger.cc	Implements the spdlog-backed sink (pattern support, level mapping, forwarding).
src/iceberg/logging/config.h.in	Adds the build-generated backend selection header template (`ICEBERG_HAS_SPDLOG`).
src/iceberg/CMakeLists.txt	Generates `iceberg/logging/config.h`, gates spdlog linkage/compilation behind `ICEBERG_SPDLOG`, and exports `/Zc:preprocessor` on MSVC.
meson.build	Adds `/Zc:preprocessor` to MSVC project arguments for Meson builds.
CMakeLists.txt	Introduces the `ICEBERG_SPDLOG` option (default ON).
cmake_modules/IcebergThirdpartyToolchain.cmake	Avoids resolving the spdlog dependency when `ICEBERG_SPDLOG` is OFF.

+template <typename MakeMessage>
+void LogToExplicitRuntime(Logger& logger, LogLevel level,
+                          const std::source_location& location,
+                          MakeMessage&& make_message) noexcept {
+  EmitIfEnabled(logger, level, location, std::forward<MakeMessage>(make_message));
+  if (level == LogLevel::kFatal) {
+    logger.Flush();
+    std::abort();
+  }
+}


+Status SpdLogger::Initialize(
+    const std::unordered_map<std::string, std::string>& 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);
+}


+void SpdLogger::Log(LogMessage&& message) noexcept {
+  try {
+    spdlog::source_loc loc{message.location.file_name(),
+                           static_cast<int>(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 (...) {
+  }
+}


+/// 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"


+# 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")


+template <typename MakeMessage>
+void LogToCurrentRuntime(LogLevel level, const std::source_location& location,
+                         MakeMessage&& make_message) noexcept {
+  const std::shared_ptr<Logger>& logger = CurrentLogger();
+  if (logger) {
+    EmitIfEnabled(*logger, level, location, std::forward<MakeMessage>(make_message));
+  }
+  if (level == LogLevel::kFatal) {
+    if (logger) logger->Flush();
+    std::abort();
+  }
+}


wgtmac · 2026-07-23T06:21:15Z

It's time to rebase it :)

The FatalHandler only ran for fixed ICEBERG_LOG_FATAL; reaching kFatal via the runtime-level ICEBERG_LOG(kFatal, ...) or ICEBERG_LOG_TO(sink, kFatal, ...) aborted without invoking it (and only formatted when ShouldLog passed). Extract the fatal sequence into a shared DispatchFatal (format once -> emit-if-enabled -> flush -> run handler -> abort) and route LogFatal, LogToCurrentRuntime, and LogToExplicitRuntime through it. Adds death tests for the two runtime paths. Co-authored-by: Isaac

wgtmac · 2026-08-01T07:55:43Z

+                           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);


This re-formats an already formatted message through fmt for every emitted record, adding another full copy and possibly an allocation for long messages. Please call the raw-message overload with spdlog::string_view_t{message.message.data(), message.message.size()} instead.

wgtmac · 2026-08-01T07:55:45Z

+  // logger_ is a hard precondition: SpdLogger is not consumer-constructible (it is
+  // obtained via the default logger or the "spdlog" registry factory, both of which
+  // pass a real logger), so Initialize/Log/Flush may dereference it unconditionally.
+  ICEBERG_DCHECK(logger_ != nullptr, "SpdLogger requires a non-null spdlog::logger");


ICEBERG_DCHECK disappears under NDEBUG, and the constructor dereferences logger_ immediately afterwards. Since shared_ptr is nullable and the header only documents the synchronous requirement, an empty input becomes a release crash. Please reject null in a factory or encode the non-null requirement in the API.

wgtmac · 2026-08-01T07:55:46Z

+# 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)


CMake supports ICEBERG_SPDLOG=OFF, but Meson always enables this backend and unconditionally links spdlog. Please add a matching Meson feature option so the no-spdlog/Cerr configuration is available in both supported build systems.

wgtmac · 2026-08-01T07:55:48Z

+  EXPECT_TRUE(logger.ShouldLog(LogLevel::kError));
+}
+
+TEST(SpdLoggerTest, ForwardsMessageToSink) {


This only verifies the payload, so the advertised source-location forwarding is untested. Please use a %s:%# %! %v pattern and assert the file, line, and function fields.

Fifth block: the default production backend and the build option that selects it. - SpdLogger wraps spdlog::logger (kCritical/kFatal -> spdlog critical, others 1:1), forwarding the pre-formatted message and source location. Synchronous only in v1 (spdlog's source_loc is a non-owning const char*, unsafe with async sinks). It lives in logging/internal/, is gated by #ifdef ICEBERG_HAS_SPDLOG, and is NOT installed -- consumers obtain it via the default logger or the registry, never by including spdlog headers. - New ICEBERG_SPDLOG CMake option (default ON). config.h is ALWAYS generated (only ICEBERG_HAS_SPDLOG's definedness varies) so logger.cc compiles in both configurations; MakeDefaultLogger() prefers SpdLogger when compiled in, else CerrLogger. - Critically, ICEBERG_SPDLOG=OFF now UNWIRES the previously-unconditional spdlog link (interface-lib lists + resolve_spdlog_dependency), not just the new source -- so an OFF build has no spdlog dependency at all. spdlog_logger_test (compiled only on the ON path) covers the level mapping including fatal->critical and source-location forwarding. Co-authored-by: Isaac

kamcheungting-db changed the title ~~feat: [Iceberg Logger] [Part-5] spdlog backend (ICEBERG_SPDLOG)~~ feat(logging): add spdlog backend behind ICEBERG_SPDLOG Jun 11, 2026

kamcheungting-db changed the title ~~feat(logging): add spdlog backend behind ICEBERG_SPDLOG~~ feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6) Jun 11, 2026

kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from c6831cb to 2b80ee1 Compare June 14, 2026 06:16

kamcheungting-db mentioned this pull request Jun 14, 2026

feat(logging): add Loggers registry (6/6) #737

Open

kamcheungting-db force-pushed the logging-block5-spdlog branch 14 times, most recently from dd50f86 to 17a616e Compare June 22, 2026 10:24

kamcheungting-db force-pushed the logging-block5-spdlog branch 6 times, most recently from 7e81c6d to 2684d7c Compare June 24, 2026 18:39

kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from 289b0fb to ef127cd Compare June 30, 2026 20:36

kamcheungting-db mentioned this pull request Jun 30, 2026

feat(logging): zero-allocation format->dispatch path (LogRecord view) #795

Open

kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from b337dd0 to 56fbf5e Compare July 16, 2026 06:30

kamcheungting-db force-pushed the logging-block5-spdlog branch 3 times, most recently from db413fa to fd92bc2 Compare July 16, 2026 07:20

manuzhang requested a review from Copilot July 20, 2026 07:27

Copilot started reviewing on behalf of manuzhang July 20, 2026 07:28 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

manuzhang added this to the 0.4.0 milestone Jul 21, 2026

kamcheungting-db force-pushed the logging-block5-spdlog branch from fd92bc2 to 4386cf7 Compare July 30, 2026 21:15

wgtmac reviewed Aug 1, 2026

View reviewed changes

kamcheungting-db force-pushed the logging-block5-spdlog branch 2 times, most recently from 878625b to f0d8791 Compare August 6, 2026 00:59

kamcheungting-db mentioned this pull request Aug 6, 2026

fix(logging): run FatalHandler on all kFatal paths #872

Open

kamcheungting-db force-pushed the logging-block5-spdlog branch from f0d8791 to c1bcbce Compare August 6, 2026 01:05

kamcheungting-db force-pushed the logging-block5-spdlog branch from c1bcbce to e9308ff Compare August 6, 2026 01:57

Navigation Menu

Uh oh!

feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6) - #726

feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6)#726
kamcheungting-db wants to merge 1 commit into
apache:mainfrom
kamcheungting-db:logging-block5-spdlog

kamcheungting-db commented Jun 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

wgtmac commented Jul 23, 2026

Uh oh!

wgtmac Aug 1, 2026

Uh oh!

wgtmac Aug 1, 2026

Uh oh!

wgtmac Aug 1, 2026

Uh oh!

wgtmac Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

kamcheungting-db commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

wgtmac commented Jul 23, 2026

Uh oh!

wgtmac Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

wgtmac Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

wgtmac Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

wgtmac Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kamcheungting-db commented Jun 11, 2026 •

edited

Loading