BP-69: Allow caller to pass a parent slog Logger to created/opened ledgers#4766
Open
merlimat wants to merge 1 commit intoapache:masterfrom
Open
BP-69: Allow caller to pass a parent slog Logger to created/opened ledgers#4766merlimat wants to merge 1 commit intoapache:masterfrom
merlimat wants to merge 1 commit intoapache:masterfrom
Conversation
…dgers Add CreateBuilder.withLoggerContext(io.github.merlimat.slog.Logger) and OpenBuilder.withLoggerContext(io.github.merlimat.slog.Logger) so an application that already has a per-request / per-tenant slog Logger can pass it as the parent of the per-handle Logger that bookkeeper builds. The LedgerHandle's logger inherits the parent's context via slog's LoggerBuilder.ctx(Logger), then layers on the always-present `ledgerId`. Passing a Logger (vs. a Map<String, Object>) avoids the need to extract/serialize attrs out of slog: the application's Logger is typically the natural source of context already. Implementation: - New default no-op method on the @Public/@unstable CreateBuilder / OpenBuilder interfaces to keep source compatibility for any out-of-tree implementor. - CreateBuilderImpl, OpenBuilderBase store the parent Logger and pass it through to LedgerCreateOp / LedgerOpenOp, which forward it to LedgerHandle / LedgerHandleAdv / ReadOnlyLedgerHandle constructor. - New constructor overloads on the three handle types take a parentLogger; existing constructors delegate with null. - LedgerOpenOp's own contextual log is built with the same parent so log events emitted during the open path also carry the attrs. Tests: new LoggerContextTest covers compile-time API contract (chainable, accepts a Logger), runtime happy-path (null is no-op, non-null context map executes cleanly), and a deterministic check via a log4j2 capturing appender that the parent's attrs flow into the event's ContextData on the resulting handle's Logger via the LoggerBuilder.ctx(Logger) hook.
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.
Summary
Add
withLoggerContext(Logger parentLogger)toCreateBuilderandOpenBuilderso an application that already has a per-request / per-tenant slogLoggercan pass it as the parent of the per-handleLoggerthat bookkeeper builds. TheLedgerHandle's logger inherits the parent's context via slog'sLoggerBuilder.ctx(Logger), then layers on the always-presentledgerId.Passing a
Logger(vs. aMap<String, Object>) avoids the need to extract / serialize attrs out of slog: the application'sLoggeris the natural source of context.Implementation
defaultno-op method on the@Public/@UnstableCreateBuilderandOpenBuilderinterfaces — preserves source compatibility for any out-of-tree implementor.CreateBuilderImplandOpenBuilderBasestore the parentLoggerand pass it through toLedgerCreateOp/LedgerOpenOp, which forward it to theLedgerHandle/LedgerHandleAdv/ReadOnlyLedgerHandleconstructor.Logger parentLogger; existing constructors delegate withnull.LedgerOpenOp's own contextuallogis built with the same parent so log events emitted during the open path also carry the attrs.Test plan
New
LoggerContextTestcovers:Compile-time API contract:
withLoggerContext(Logger)on both builders is chainable and returns the same builder type.Runtime: passing
nullis a no-op; non-null context map executes cleanly through toWriteHandle.getId().Open-on-non-existent-ledger end-to-end test under a log4j2 capturing appender.
Deterministic unit test of the slog
LoggerBuilder.ctx(parent).attr("ledgerId", ...).build()chain — verifies the parent's attrs andledgerIdboth appear inevent.getContextData()on the resultingLogger.mvn -pl bookkeeper-server compile checkstyle:checkcleanLoggerContextTest(5 tests) pass locallyExisting
BookKeeperBuildersTest/BookKeeperBuildersOpenLedgerTest(33 tests) unaffectedFull CI matrix green