merlimat opened a new pull request, #4834: URL: https://github.com/apache/bookkeeper/pull/4834
Descriptions of the changes in this PR: Follow-up to BP-69 (#4766), which added `CreateBuilder.withLoggerContext` / `OpenBuilder.withLoggerContext` so applications can attach their slog logging context to created/opened ledgers. ### Motivation Two gaps block applications (specifically Apache Pulsar) from carrying application logging context on all ledger operations through the builder API: 1. Only the classic callback API `BookKeeper.asyncOpenLedger(lId, digestType, passwd, cb, ctx, keepUpdateMetadata)` can set `keepUpdateMetadata`. Pulsar's recovery-open call sites (`ManagedLedgerImpl` initialize, `ManagedCursorImpl` cursor recovery, `BookkeeperSchemaStorage` / `CompactedTopicImpl` / `BookkeeperBucketSnapshotStorage` opens) all pass `keepUpdateMetadata=true`, so they cannot migrate to the builder API and therefore cannot use `withLoggerContext`. 2. `DeleteBuilder` had no `withLoggerContext`, so deletes could not carry application context. ### Changes **`OpenBuilder.withKeepUpdateMetadata(boolean)`** (first commit) - New default method returning `this` on the `@Public`/`@Unstable` interface, keeping source compatibility for out-of-tree implementors (same approach as `withLoggerContext`). - `OpenBuilderBase` stores the flag; `LedgerOpenOp.OpenBuilderImpl` calls `initiateWithKeepUpdateMetadata()` when the flag is set on a recovery open. The flag only has an effect for recovery opens — non-recovery opens always register the metadata listener immediately — matching the classic API, which only exposes the flag on the recovery-open overload. - Tests in `BookKeeperBuildersTest` verify the ledger metadata listener is registered when opening with recovery and the flag set, and not registered without it. **`DeleteBuilder.withLoggerContext(Logger)`** (second commit) - New default no-op method mirroring BP-69's CreateBuilder/OpenBuilder support. - `DeleteBuilderImpl` passes the parent logger to `LedgerDeleteOp`, whose logger is now a per-op contextual logger (parent context via slog's `LoggerBuilder.ctx(Logger)` plus the always-present `ledgerId` attribute), like `LedgerOpenOp`. The lombok `@CustomLog` static logger moved to `DeleteBuilderImpl`, its only remaining user (builder validation runs before an op exists). - One behavioral addition: delete failures are now logged at debug level through the contextual logger — the delete path previously had no per-op log statements, so the context would have had nothing to ride on. Failures keep being reported to the caller via the callback, and debug level keeps the log silent at default levels (double deletes are routine for some applications). - `LoggerContextTest` gains the DeleteBuilder chain and null-is-no-op tests, plus a deterministic check that a failed delete's debug event carries the parent logger's attributes and the `ledgerId`. Verified with `BookKeeperBuildersTest` (31 tests) and `LoggerContextTest` (8 tests), and `checkstyle:check` — all green. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
