Skip to content

feat(logging): add logging macros (4/6)#725

Open
kamcheungting-db wants to merge 1 commit into
apache:mainfrom
kamcheungting-db:logging-block4-macros
Open

feat(logging): add logging macros (4/6)#725
kamcheungting-db wants to merge 1 commit into
apache:mainfrom
kamcheungting-db:logging-block4-macros

Conversation

@kamcheungting-db

@kamcheungting-db kamcheungting-db commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Part 4 of the logging stack (builds on #724). Adds the application-facing macros — the part most callers actually use.

What's here

  • ICEBERG_LOG_{TRACE,DEBUG,INFO,WARN,ERROR,CRITICAL,FATAL}, the generic ICEBERG_LOG(level, …), ICEBERG_LOG_TO(logger, …), and ICEBERG_LOG_RUNTIME_FMT for runtime format strings.
  • ICEBERG_LOG_ACTIVE_LEVEL is a compile-time floor: statements below it are removed entirely. ICEBERG_LOG_FATAL always emits, flushes, then aborts.
  • Filtering is decided only by ShouldLog(); formatting happens only on the taken path and never throws (a bad format falls back safely).
  • Generic/runtime-format macros use the same lock-free fast path as the fixed-severity ones (the slot lock is taken only on the fatal/abort path).
  • Bare LOG_* aliases are opt-in via ICEBERG_LOG_SHORT_MACROS.

Build note — sets /Zc:preprocessor on MSVC, required for the __VA_OPT__ used by the macros.

Examples — every macro takes a std::format string + args. The rendered line depends on the active backend; with the default CerrLogger it looks like:

ICEBERG_LOG_TRACE("entering scan for {}", table);
  2026-06-16T10:59:41.186Z trace [12345] table_scan.cc:88] entering scan for db.t
ICEBERG_LOG_DEBUG("cache miss key={}", key);
  2026-06-16T10:59:41.186Z debug [12345] cache.cc:42] cache miss key=manifest-7
ICEBERG_LOG_INFO("loaded {} manifests in {} ms", n, ms);
  2026-06-16T10:59:41.186Z info [12345] table_scan.cc:91] loaded 5 manifests in 12 ms
ICEBERG_LOG_WARN("retry {} after {}", attempt, err);
  2026-06-16T10:59:41.186Z warn [12345] io.cc:51] retry 2 after timeout
ICEBERG_LOG_ERROR("commit failed: {}", status);
  2026-06-16T10:59:41.186Z error [12345] txn.cc:77] commit failed: conflict
ICEBERG_LOG_CRITICAL("metadata unreadable at {}", path);
  2026-06-16T10:59:41.186Z critical [12345] meta.cc:30] metadata unreadable at s3://b/m.json
ICEBERG_LOG_FATAL("unrecoverable: {}", reason);   // emits, flushes, then std::abort()
  2026-06-16T10:59:41.186Z fatal [12345] boot.cc:19] unrecoverable: bad config

Less common forms: ICEBERG_LOG(level, …) (runtime severity), ICEBERG_LOG_TO(logger, level, …) (explicit logger), ICEBERG_LOG_RUNTIME_FMT(level, fmt_string, args…) (non-literal format). The same examples are documented inline in logger.h.

Tests — macros_test / macros_active_level_test: formatting, level gating, "argument not evaluated when disabled", compile-time stripping, never-throws, and FATAL-aborts death tests. clang/libc++.

This pull request and its description were written by Isaac.

@kamcheungting-db kamcheungting-db mentioned this pull request Jun 11, 2026
Open
@kamcheungting-db kamcheungting-db changed the title feat: [Iceberg Logger] [Part-4] Logging Macros feat(logging): add logging macros Jun 11, 2026
@kamcheungting-db kamcheungting-db changed the title feat(logging): add logging macros feat(logging): add logging macros (4/6) Jun 11, 2026
@kamcheungting-db kamcheungting-db force-pushed the logging-block4-macros branch 9 times, most recently from 152f086 to a655149 Compare June 16, 2026 03:58
@kamcheungting-db kamcheungting-db mentioned this pull request Jun 16, 2026
Merged
kamcheungting-db added a commit to kamcheungting-db/iceberg-cpp that referenced this pull request Jun 21, 2026
@kamcheungting-db
fix(logging): address review feedback on the Logger interface
796ff0d 
Resolves @wgtmac's review comments on PR apache#723:

- Mark ShouldLog()/SetLevel()/level() noexcept so a custom logger cannot throw
  from the hot filter path; propagated to NoopLogger and CapturingLogger.
- Make CurrentLogger() safe during thread teardown without UB: the per-thread
  cache is reached via a trivially-destructible thread_local pointer to a
  never-freed heap slot (kept LSan-reachable by a registry), so logging from a
  thread_local destructor in any order is safe. Replaces the AliveFlag guard.
- Add ICEBERG_EXPORT to LogAttribute/LogMessage, matching Error/ScanReport.
- Trim the logger.h \file brief (the macros it referenced arrive in PR apache#725).
- Tests: use the IsError matcher and std::ignore; strengthen the teardown test
  to log from a thread_local destroyed after the cache.

Co-authored-by: Isaac
@kamcheungting-db kamcheungting-db force-pushed the logging-block4-macros branch 4 times, most recently from 7e99738 to 0942681 Compare June 22, 2026 08:19
wgtmac pushed a commit that referenced this pull request Jun 22, 2026
@kamcheungting-db
feat(logging): add Logger interface and default logger (#723)
a38e0cd 
Part 2 of the logging stack (builds on #722). Adds the logging API and a
swappable default logger — the foundation the backends and macros plug
into.

**What's here**
- `Logger`: the pluggable sink interface (`ShouldLog` / `Log` /
`SetLevel` / `Flush` / `Initialize`). `ShouldLog()` is the single source
of truth for runtime filtering.
- `LogMessage` owns its formatted text so a sink can safely keep a
record; reserves an attributes field for future structured logging.
- Process-global default logger: `GetDefaultLogger` / `SetDefaultLogger`
/ `SetDefaultLevel`, with a lock-free thread-local fast path so logging
stays cheap.
- `Initialize` applies the `level` property, so config-driven levels
actually work. `CurrentLogger()` is safe to call even from a
`thread_local` destructor during thread shutdown.
- `logger.h` stays backend-agnostic (never includes the build config
header), so consumers see one stable API regardless of backend.

**Examples** — using the API directly (the `LOG_*` macros that wrap it
arrive in #725):

```cpp
// A custom sink, installed as the process default.
class MySink : public Logger {
 public:
  bool ShouldLog(LogLevel level) const override { return level >= level_; }
  void Log(LogMessage&& m) noexcept override { write_line(m.message); }
  void SetLevel(LogLevel level) override { level_ = level; }
  LogLevel level() const override { return level_; }
 private:
  std::atomic<LogLevel> level_{LogLevel::kInfo};
};

SetDefaultLogger(std::make_shared<MySink>());   // install process-wide
SetDefaultLevel(LogLevel::kDebug);              // adjust the threshold

auto logger = GetDefaultLogger();               // borrow the current default
if (logger->ShouldLog(LogLevel::kInfo)) {
  logger->Log(LogMessage{.level = LogLevel::kInfo, .message = "scan ready"});
}

// Or configure from catalog-style properties (applies the "level" key):
auto sink = std::make_shared<MySink>();
auto status = sink->Initialize({{std::string(kLevelProperty), "warn"}});  // -> kWarn
```

The same example is documented inline in `logger.h`.

**Tests** — `logger_test`: default-logger API, level-from-property,
invalid level rejected, concurrent swap/read, and logging during thread
teardown. Built and run with clang/libc++ (spdlog ON and OFF).

This pull request and its description were written by Isaac.
@kamcheungting-db kamcheungting-db force-pushed the logging-block4-macros branch 8 times, most recently from bb6a95d to 6c3a85e Compare June 24, 2026 17:58
@kamcheungting-db
feat(logging): add LOG_* logging macros (4/6)
a9878cb 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@kamcheungting-db