feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6)#726
Open
kamcheungting-db wants to merge 6 commits into
Open
feat(logging): add spdlog backend behind ICEBERG_SPDLOG (5/6)#726kamcheungting-db wants to merge 6 commits into
kamcheungting-db wants to merge 6 commits into
Conversation
kamcheungting-db
changed the title
kamcheungting-db
changed the title
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
2 times, most recently
from
c6831cb to
2b80ee1
Compare
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
14 times, most recently
from
dd50f86 to
17a616e
Compare
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
6 times, most recently
from
7e81c6d to
2684d7c
Compare
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
2 times, most recently
from
289b0fb to
ef127cd
Compare
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
from
ef127cd to
b337dd0
Compare
Fourth block: the application-facing macros, the only part most callers touch.
- ICEBERG_LOG_{TRACE,DEBUG,INFO,WARN,ERROR,CRITICAL,FATAL} plus the generic
ICEBERG_LOG(level, ...), ICEBERG_LOG_TO(logger, level, ...) for an explicit
logger, and ICEBERG_LOG_RUNTIME_FMT for a runtime (non-literal) format string.
- ICEBERG_LOG_ACTIVE_LEVEL is a compile-time severity floor: statements below it
are removed entirely via `if constexpr` (no format call site, no source
location emitted). ICEBERG_LOG_FATAL is never gated by the floor -- its abort
is always compiled in; it emits, best-effort Flush()es the same logger it
emitted to, then std::abort().
- Filtering is decided solely by Logger::ShouldLog(); formatting is wrapped in
try/catch so logging never throws (a format failure routes to EmitFormatError).
- Bare Java-style aliases (LOG_INFO, ...) are opt-in via ICEBERG_LOG_SHORT_MACROS
to avoid polluting consumers / colliding with glog/abseil.
Header-only addition to logger.h. macros_test covers injection, the
guard-before-format short-circuit, never-throws, and FATAL aborts;
macros_active_level_test verifies compile-time stripping in a kOff translation unit.
Co-authored-by: Isaac
Note in FormatAndEmit and the ICEBERG_INTERNAL_LOG comment that the only throws are std::format_error/std::bad_alloc (same recovery), and that catching std::exception rather than (...) lets a non-std unwind such as abi::__forced_unwind (thread cancellation) propagate. Co-authored-by: Isaac
… docs, FATAL test) - Revert the 6 catch sites to catch(...) so the noexcept "logging never throws" guarantee holds even when a user-defined std::formatter throws a non-std type (catch(const std::exception&) would let it reach std::terminate). - Apply /Zc:preprocessor to the iceberg library targets PUBLICly on MSVC, not only to tests: logger.h is public and uses __VA_OPT__, so the library build and consumers need the conforming preprocessor too. - Soften the compile-time-floor doc: `if constexpr` discards the emit but the statement must still be well-formed (bad format string is still a compile error). - Add the missing opening '[' to the logger.h example output lines to match CerrLogger's [file:line] layout. - Add MacrosDeathTest.FatalRoutesThroughScopedLogger locking in that ICEBERG_LOG_FATAL routes through the active ScopedLogger (GetCurrentLogger). Co-authored-by: Isaac
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
from
b337dd0 to
56fbf5e
Compare
Adopt the structure from the apache#824 prototype (keeping our if constexpr gating): - logger.h is now the C++ API only; ICEBERG_LOG_* macros move to log_macros.h, and the bare LOG_* aliases move behind an opt-in short_log_macros.h (replacing the ICEBERG_LOG_SHORT_MACROS define). - Dedup the five macro bodies onto shared internal helpers (EmitIfEnabled + LogToCurrent / LogToCurrentRuntime / LogToExplicitRuntime / LogFatal) that take a lazy [&]()->std::string message thunk invoked only past ShouldLog, so disabled logs still don't evaluate their args. Keeps catch(...), the GetCurrentLogger() fatal path, and if constexpr compile-time floor. - VFormat moves to log_macros.h (only ICEBERG_LOG_RUNTIME_FMT uses it). Co-authored-by: Isaac
kamcheungting-db
force-pushed
the
logging-block5-spdlog
branch
2 times, most recently
from
864daa0 to
db413fa
Compare
…review ⑤) Address the extensibility comment: let an embedder (JNI, Python host, crash reporter) run a hook on the fatal path before std::abort() to flush resources or print a stack trace. - Add FatalHandler = std::function<void(const std::source_location&, std::string_view)> plus thread-safe SetFatalHandler/GetFatalHandler (leaked, teardown-safe slot in logger.cc). - Wire it into LogFatal: format the message once, emit-if-enabled + flush, run the handler (message passed even when the record is filtered out), then abort. A throwing handler cannot prevent the abort. - Death tests: handler runs with the formatted message, receives the call-site location, and still fires when the record is suppressed. Co-authored-by: Isaac
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
force-pushed
the
logging-block5-spdlog
branch
from
db413fa to
fd92bc2
Compare
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
1 participant
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 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
SpdLoggerwrapsspdlog::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.ICEBERG_SPDLOGCMake option (ON by default). When OFF, the build has no spdlog dependency at all andCerrLoggeris the default.patternproperty is honored here viaspdlog set_pattern(the cerr backend keeps its fixed layout);levelworks on both backends.SpdLoggerlives inlogging/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 thepatternproperty. clang/libc++ with spdlog 1.15.3.This pull request and its description were written by Isaac.