Skip to content

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

Open
kamcheungting-db wants to merge 3 commits into
apache:mainfrom
kamcheungting-db:logging-block6-registry
Open

feat(logging): add Loggers registry (6/6)#737
kamcheungting-db wants to merge 3 commits into
apache:mainfrom
kamcheungting-db:logging-block6-registry

Conversation

@kamcheungting-db

@kamcheungting-db kamcheungting-db commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Part 6 of the logging stack (builds on #726). Lets applications pick and install a logging backend from configuration instead of writing code — the same pattern as MetricsReporters.

How it works

Choose a backend with the logger-impl property; built-ins are noop, cerr, and spdlog (when compiled in).

// Build a logger from properties and install it as the process default.
iceberg::Loggers::LoadAndSetDefault({{"logger-impl", "spdlog"}, {"level", "info"}});

// ...then anywhere in the code:
ICEBERG_LOG_INFO("scan planned: {} files", n);

API

  • Loggers::Load(properties) — build a logger, choosing the type from logger-impl.
  • Loggers::LoadAndSetDefault(properties) — build one and install it as the process default.
  • Loggers::Register(type, factory) — register a custom backend under a new name.

With no logger-impl set, the default is spdlog when compiled in, otherwise cerr — so logging works out of the box.

Testslogging_end_to_end_test drives the full path an app uses: configure a backend + level via properties, install it, log through the macros, and check the real output. Covers the default backend and a macro reaching a real spdlog sink, with spdlog ON and OFF.

This pull request and its description were written by Isaac.

@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch 15 times, most recently from 19003d8 to 51e997b Compare June 22, 2026 10:24
@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch 6 times, most recently from faed95d to d0066d4 Compare June 24, 2026 18:39
@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch 2 times, most recently from 49a68c1 to b68bd93 Compare June 30, 2026 20:37
@manuzhang

Copy link
Copy Markdown
Member

@kamcheungting-db can you please rebase your PR?

@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch 5 times, most recently from 8d4515e to aaa1ebf Compare July 16, 2026 07:21
@manuzhang manuzhang added this to the 0.4.0 milestone Jul 21, 2026
@manuzhang
manuzhang requested a review from Copilot July 21, 2026 04:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the logging stack by adding a configuration-driven Loggers registry/factory (mirroring MetricsReporters), installing a process-default logger selected by properties, and introducing public ICEBERG_LOG_* macros (plus opt-in bare LOG_* aliases) with end-to-end and unit test coverage across spdlog-on/off builds.

Changes:

  • Added Loggers::{Register, Load, LoadAndSetDefault} with built-in backends (noop, cerr, and spdlog when compiled in) and a compiled-backend default.
  • Introduced iceberg/logging/log_macros.h (and short_log_macros.h) plus a FatalHandler hook for fatal logging.
  • Added comprehensive tests (macros behavior, registry behavior, spdlog backend, and end-to-end logging output), and wired builds (CMake + Meson) including MSVC /Zc:preprocessor for __VA_OPT__.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/iceberg/test/spdlog_logger_test.cc Adds unit tests for the spdlog-backed logger implementation.
src/iceberg/test/meson.build Registers new logging-related tests in Meson.
src/iceberg/test/macros_test.cc Adds runtime behavior + death tests for logging macros (formatting, gating, fatal semantics, handler).
src/iceberg/test/macros_active_level_test.cc Tests compile-time active-level stripping behavior.
src/iceberg/test/logging_end_to_end_test.cc End-to-end validation of registry/config/default/macro integration and real output.
src/iceberg/test/loggers_test.cc Adds unit tests for the Loggers registry and property handling.
src/iceberg/test/CMakeLists.txt Enables MSVC conforming preprocessor for tests; adds new test sources.
src/iceberg/meson.build Adds logging sources (including registry + spdlog backend) to Meson build.
src/iceberg/logging/short_log_macros.h Provides opt-in bare LOG_* aliases for ICEBERG_LOG_*.
src/iceberg/logging/meson.build Installs new public logging headers; generates Meson-only config.h.
src/iceberg/logging/loggers.h Declares the public Loggers registry/factory API and property keys.
src/iceberg/logging/loggers.cc Implements the registry, built-in factories, and load/initialize semantics.
src/iceberg/logging/logger.h Extends logging API docs; adds FatalHandler API surface.
src/iceberg/logging/logger.cc Implements compiled-backend default selection and fatal handler storage/access.
src/iceberg/logging/log_macros.h Adds the ICEBERG_LOG_* macro layer and supporting helpers.
src/iceberg/logging/internal/spdlog_logger.h Introduces internal spdlog-backed SpdLogger sink.
src/iceberg/logging/internal/spdlog_logger.cc Implements spdlog sink behavior including pattern support and level mapping.
src/iceberg/logging/config.h.in Adds generated build-time logging backend configuration header template.
src/iceberg/CMakeLists.txt Generates config.h; gates spdlog compilation/link; exports /Zc:preprocessor for consumers.
meson.build Enables /Zc:preprocessor via Meson supported-args on MSVC.
CMakeLists.txt Adds ICEBERG_SPDLOG option.
cmake_modules/IcebergThirdpartyToolchain.cmake Gates spdlog dependency resolution behind ICEBERG_SPDLOG.

Comment thread src/iceberg/logging/log_macros.h Outdated
Comment thread src/iceberg/logging/log_macros.h Outdated
Comment thread src/iceberg/logging/internal/spdlog_logger.cc Outdated
@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch 6 times, most recently from 7ba75ce to dc3648f Compare August 6, 2026 02:01
@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch from dc3648f to 0bd2978 Compare August 17, 2026 08:36
Final block: configuration-driven backend selection, mirroring MetricsReporters.

- Loggers::Register(type, factory) registers a named backend; Loggers::Load(props)
  builds one, selecting the type from the "logger-impl" property key.
- Built-in factories: "noop", "cerr", and (only when built with ICEBERG_SPDLOG)
  "spdlog". With no logger-impl set, the default is spdlog when compiled in, else
  cerr -- logs by default, an intentional divergence from the metrics registry's
  noop default.
- Loggers::LoadAndSetDefault(props) loads a logger and installs it as the process
  default.

This completes the system end to end: levels -> Logger interface + default logger
-> CerrLogger/SpdLogger backends -> macros -> configuration-driven selection.
loggers_test covers load default/noop/cerr, unknown-type errors, empty-factory
rejection, custom Register, and LoadAndSetDefault.

Adds logging_end_to_end_test, which drives the public surface as an application
does -- now that every layer is present: configure a backend via the registry,
install it as the default, log through the LOG_* macros, and observe real output.
Covers registry -> default-slot -> macro -> backend -> std::cerr output, level
filtering through the full macro path, the compiled-backend identity of the
default (spdlog when ON, cerr when OFF), the "spdlog" factory by name, and a macro
statement reaching a real spdlog sink.

Co-authored-by: Isaac
@kamcheungting-db
kamcheungting-db force-pushed the logging-block6-registry branch from 0bd2978 to 66f29f2 Compare August 17, 2026 08:41
loggers.cc had its own copy of NoopLogger (identical to logger.cc's) because the
registry factory returns unique_ptr while Logger::Noop() hands out a shared
singleton. Expose internal::MakeNoopLogger() returning unique_ptr, used by both
Logger::Noop() and the "noop" factory, and delete the duplicate class.

Co-authored-by: Isaac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants