merlimat opened a new pull request, #25995: URL: https://github.com/apache/pulsar/pull/25995
### Motivation With the upgrade to BookKeeper 4.18 (#25886), the BK client supports BP-69: `CreateBuilder.withLoggerContext(Logger)` / `OpenBuilder.withLoggerContext(Logger)` let an application pass a parent slog `Logger` whose context attributes are inherited by the per-handle logger inside the BookKeeper client. Until now, BK client log lines only carried `ledgerId`, which makes it hard to correlate them with the topic / subscription / schema that the ledger belongs to. This PR plumbs Pulsar's slog context into every BK ledger create/open that the builder API can express, and adds the equivalent mechanism at the ManagedLedger level so the broker can pass context when opening a managed ledger. ### Modifications **ManagedLedger level** - New `ManagedLedgerConfig.setLoggerContext(Logger)`: `ManagedLedgerImpl` builds its instance logger with `ctx(config.getLoggerContext())`, so the caller's attributes are layered under the existing `managedLedger` attribute. `ManagedCursorImpl` now inherits the managed ledger logger (instead of rebuilding it) and adds its `cursor` attribute. - The managed ledger / cursor contextual loggers are forwarded with `withLoggerContext(...)` on: data-ledger and cursor-ledger creation, the read-path opens in `getLedgerHandle`, `ReadOnlyManagedLedgerImpl`'s last-ledger probe, and the data-ledger inspection open in `ManagedLedgerFactoryImpl`. - Ledger creation migrates from the classic callback API (`BookKeeper.asyncCreateLedger`) to `newCreateLedgerOp()`, since BP-69 only covers the builder API. Create-timeout handling and orphan-ledger cleanup semantics are preserved (`ctx` is still the `ledgerFutureHook`). **Context set by callers** | Call site | Attributes | |---|---| | `BrokerService` topic ML open | `topic` (for `segment://` topics: parent topic + `segment` descriptor) | | Cursor ledgers (inherited) | `topic`, `segment`, `managedLedger`, `cursor` | | `MLPendingAckStoreProvider` | `topic`, `subscription` | | `MLTransactionLogImpl` | `tcId` | | `BookkeeperSchemaStorage` | `schemaId` | | `AbstractTwoPhaseCompactor` / `StrategicTwoPhaseCompactor` | `topic` | | `BookkeeperBucketSnapshotStorage` | `topic`, `cursor` | **Test support** - `PulsarMockBookKeeper` implements `newCreateLedgerOp()` (delegating to its own `asyncCreateLedger`, so programmed failures keep working); two tests that stubbed the classic create API now stub the builder. - New `ManagedLedgerLoggerContextTest` asserts via a log4j2 capturing appender that attributes set on `ManagedLedgerConfig` flow into the context data of managed ledger and cursor log events. **Not covered (BK 4.18 API limitations)** - Recovery opens passing `keepUpdateMetadata=true` (ML init, cursor recovery, LAC re-check, schema/compacted-topic/bucket-snapshot opens) stay on the classic API: `OpenBuilder` has no `withKeepUpdateMetadata`. Likewise `DeleteBuilder` has no `withLoggerContext`. Both are proposed as a BP-69 follow-up in BookKeeper; the remaining call sites can migrate once that ships. - `ShadowManagedLedgerImpl`'s source-ledger probes assign the opened handle to `currentLedger` (`LedgerHandle`), which the builder API's `ReadHandle` cannot satisfy under `PulsarMockBookKeeper`. ### Verifying this change - New `ManagedLedgerLoggerContextTest` covers the context inheritance end-to-end. - `ManagedLedgerTest` (137), `ManagedCursorTest` (171), `ManagedLedgerFactoryShutdownTest`, `SchemaServiceTest`, `BookkeeperBucketSnapshotStorageTest`, `CompactorTest` all pass, exercising the migrated create paths through the mock builder. -- 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]
