BP-69: Convert bookkeeper-common to slog (phase 1)#4754
Open
merlimat wants to merge 8 commits intoapache:masterfrom
Open
BP-69: Convert bookkeeper-common to slog (phase 1)#4754merlimat wants to merge 8 commits intoapache:masterfrom
merlimat wants to merge 8 commits intoapache:masterfrom
Conversation
First phase of the slog migration for BP-69. Adds the slog dependency, Lombok @CustomLog configuration, and converts the bookkeeper-common module.
CI flagged 4 ImportOrder violations in the Phase 1 conversion: - SafeRunnable.java: io.github.merlimat.slog.Logger placed after java.* - OrderedExecutor.java: lombok.CustomLog placed after org.* - AbstractLifecycleComponent.java: same - TestOrderedExecutorDecorators.java: same Move each import to its correct alphabetical position per the project's checkstyle rule (`io.` < `java.`; `lombok.` < `org.`).
CI check-binary-license flagged the new io.github.merlimat.slog-slog-0.9.7.jar as unaccounted for. Add it to the Apache-2.0 bundled-jars list in both LICENSE-all.bin.txt and LICENSE-bkctl.bin.txt, with a new numbered source reference pointing at https://github.com/merlimat/slog/tree/v0.9.7.
The previous LICENSE fix only updated LICENSE-all.bin.txt and LICENSE-bkctl.bin.txt, but the bin-server.xml assembly descriptor packages LICENSE-server.bin.txt into the server tarball that the CI license-check script inspects.
Both tests exercise SLF4J MDC context propagation through the OrderedExecutor / OrderedScheduler decorators. With the slog migration the logger API is different and these tests are not straightforward to keep working against the new logger while still testing MDC behaviour as seen by the underlying SLF4J backend. Drop them for now. The MDC propagation code itself (MdcUtils, OrderedExecutor's mdcContextMap snapshot/restore) is unchanged, so end-to-end MDC-in-logs behaviour under Logback/Log4j2 backends is preserved.
This was referenced Apr 24, 2026
…textTest" This reverts commit 790d495.
Pulls in the MDC propagation fix from merlimat/slog#6, which makes log4j2 ThreadContext entries visible on slog events emitted via the Log4j2Logger backend (so %X{key} layouts and appenders that read event.getContextData().getValue(key) see the caller's MDC).
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.
First phase of the slog migration BP-69.
Tracking issue: #4750.
This PR adds the
slogdependency and@CustomLogwiring, and converts thebookkeeper-commonmodule. Subsequent PRs will convert the rest of the codebase module by module (stats+allocator,bookkeeper-server,bookkeeper-http+tools,stream/distributedlog, peripheral modules, and a final cleanup).Summary
io.github.merlimat.slog:slog:0.9.7topom.xml(alongside the existing SLF4J API — SLF4J stays on the classpath as the rendering backend; no log-config change required to upgrade).lombok.configat the repo root so@CustomLoggenerates a slogLoggerinstead of an SLF4J one:bookkeeper-common:AbstractLifecycleComponent,AutoCloseableLifecycleComponentLogExceptionRunnable,OrderedExecutor,SafeRunnableTestOrderedExecutorDecorators(test class)Conventions (to be applied throughout the migration)
@Slf4j→@CustomLog;LoggerFactory.getLogger(X.class)→Logger.get(X.class); naming: static =LOG, instance-bound =log.log.info("text {} {}", a, b)becomeslog.info().attr("a", a).attr("b", b).log("text")— SLF4J{}params turn into named structured attrs, not printf-style format specifiers.if (log.isDebugEnabled())guards are removed; slog returns a no-opEventwhen the level is disabled.log.debug(e -> ...)or.attr(k, () -> expensive)) so the work is only performed when the level is actually enabled.Not included in this PR
bookkeeper-commonis converted.withLoggerContext(Map<String, Object>)onCreateBuilder/OpenBuilder/DeleteBuilder) are deferred to a later PR in the BP-69 series.Test plan
mvn -pl bookkeeper-common -am compile -DskipTestspassesbookkeeper-commonunit tests still pass