Repository navigation
[improve] PIP-467: Convert pulsar-broker module logging from SLF4J to slog - #25535
Merged
Merged
Conversation
2 of 3 tasks
The '%s%' sequences were parsed as '%s' followed by an incomplete format specifier '%', causing DuplicateFormatFlagsException at runtime. Replace with '%s%%' to emit a literal percent sign.
The original SLF4J code was guarded by if(log.isDebugEnabled()), which prevented servletRequest.getServletContext().getContextPath() from being called. In tests using Mockito.mock(HttpServletRequest.class), getServletContext() returns null, so the eager call NPEs. Use lambda suppliers so the value is only computed when debug is actually enabled.
snapshot.getSnapshotId() throws IllegalStateException when the required field is not set. Use a method reference for lazy evaluation since the call was previously guarded by if(log.isDebugEnabled()).
In ServerCnx.handleConnect, authState.getAuthRole() throws AuthenticationException when called before authentication completes. The original code was inside if(log.isDebugEnabled()) so it was never called in production (INFO level). Use a lambda supplier. Also fix ReplicatedSubscriptionSnapshotCache where snapshot_id is a required LightProto field that throws when not set.
The original code used @slf4j on SnapshotBuilder giving it its own static logger. The slog conversion incorrectly changed it to use controller.log, which is null when the controller is a Mockito mock (mocks skip constructors and field initializers). Restore @CustomLog on SnapshotBuilder so it has its own logger.
lhotari
approved these changes
Apr 16, 2026
1 of 15 tasks
Merged
11 tasks
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
2 participants
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
pulsar-brokermodule from SLF4J to slog structured loggingtopic,subscription,state) so that related log lines carry the same structured context automaticallyprotected final Logger logfrom their parent now declare their own logger that chains the parent's context viaLogger.get(SubClass.class).with().ctx(super.log).build(), so the logger name matches the actual runtime class while still inheriting parent attributesargNattrs to meaningful names; strip stale SLF4J{}placeholder text from log messageslog.level("msg")when no attributes are presentTest plan
./gradlew :pulsar-broker:compileJavapasses./gradlew :pulsar-broker:checkstyleMainpasses./gradlew :pulsar-broker:compileTestJavapasses./gradlew :pulsar-broker:checkstyleTestpassesMotivation
PIP-467
Note
Please label this PR with
ready-to-test