feat(logging): add Loggers registry (6/6) - #737
Open
kamcheungting-db wants to merge 3 commits into
Open
Conversation
kamcheungting-db
force-pushed
the
logging-block6-registry
branch
15 times, most recently
from
June 22, 2026 10:24
19003d8 to
51e997b
Compare
kamcheungting-db
force-pushed
the
logging-block6-registry
branch
6 times, most recently
from
June 24, 2026 18:39
faed95d to
d0066d4
Compare
kamcheungting-db
force-pushed
the
logging-block6-registry
branch
2 times, most recently
from
June 30, 2026 20:37
49a68c1 to
b68bd93
Compare
Member
|
@kamcheungting-db can you please rebase your PR? |
kamcheungting-db
force-pushed
the
logging-block6-registry
branch
5 times, most recently
from
July 16, 2026 07:21
8d4515e to
aaa1ebf
Compare
There was a problem hiding this comment.
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, andspdlogwhen compiled in) and a compiled-backend default. - Introduced
iceberg/logging/log_macros.h(andshort_log_macros.h) plus aFatalHandlerhook 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:preprocessorfor__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. |
kamcheungting-db
force-pushed
the
logging-block6-registry
branch
6 times, most recently
from
August 6, 2026 02:01
7ba75ce to
dc3648f
Compare
kamcheungting-db
force-pushed
the
logging-block6-registry
branch
from
August 17, 2026 08:36
dc3648f to
0bd2978
Compare
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
force-pushed
the
logging-block6-registry
branch
from
August 17, 2026 08:41
0bd2978 to
66f29f2
Compare
…y lock Co-authored-by: Isaac
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-implproperty; built-ins arenoop,cerr, andspdlog(when compiled in).API
Loggers::Load(properties)— build a logger, choosing the type fromlogger-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-implset, the default isspdlogwhen compiled in, otherwisecerr— so logging works out of the box.Tests —
logging_end_to_end_testdrives 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.