feat(logback): [Data Collection 26] Add unencoded message opt-in - #6076
Open
adinauer wants to merge 1 commit into
Open
feat(logback): [Data Collection 26] Add unencoded message opt-in#6076adinauer wants to merge 1 commit into
adinauer wants to merge 1 commit into
Conversation
Allow encoder-equipped Logback appenders to include original message templates and parameters without relying on sendDefaultPii. Keep sendDefaultPii as a temporary compatibility exception while Data Collection replaces its other behavior. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
This was referenced Sep 9, 2026
📲 Install BuildsAndroid
|
adinauer
marked this pull request as ready for review
September 9, 2026 07:52
adinauer
requested review from
0xadam-brown,
markushi and
runningcode
as code owners
September 9, 2026 07:52
runningcode
approved these changes
Sep 9, 2026
runningcode
left a comment
runningcode
left a comment
Contributor
There was a problem hiding this comment.
Looks good. I won't block merging but worth taking a look at the kotlin synthetic properties.
| fixture = Fixture(minimumEventLevel = Level.DEBUG, includeUnencodedMessage = true) | ||
| fixture.logger.info("testing encoding {}", "param1") | ||
|
|
||
| assertThat(SentryAppender().isIncludeUnencodedMessage).isFalse() |
Contributor
There was a problem hiding this comment.
Looks like we have a mismatch on the kotlin synthetic properties. I think renaming this to get would fix it.
| /** | ||
| * Whether to include the original message template and parameters when an encoder is configured. | ||
| */ | ||
| public boolean isIncludeUnencodedMessage() { |
Contributor
There was a problem hiding this comment.
Based on the tests, it looks like we have a kotlin synthetic property mismatch. I think this should fix it. (see my comment below in the test)
Suggested change
| public boolean isIncludeUnencodedMessage() { | |
| public boolean getIncludeUnencodedMessage() { |
| @@ -177,7 +178,7 @@ protected void captureLog(@NotNull ILoggingEvent loggingEvent) { | |||
| final @NotNull String formattedMessage = formatted(loggingEvent); | |||
|
|
|||
| // if encoder is set we treat message+params as PII as encoders may be used to mask/strip PII | |||
Contributor
There was a problem hiding this comment.
should we update the comment here (and on line 122) about includeUnencodedMessage ?
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.
PR Stack (Data Collection)
📜 Description
Add
includeUnencodedMessageto the LogbackSentryAppender. When an encoder is configured, this explicit integration option includes the original message template and parameters alongside the encoded output.Keep
sendDefaultPii=trueas a temporary compatibility opt-in, including when Data Collection is configured. Without an encoder, the appender preserves its existing behavior and includes the original message data.💡 Motivation and Context
Logback encoders can mask or remove PII before Sentry receives the formatted message. The existing guard relies only on the broad legacy
sendDefaultPiioption. This adds a narrow Logback-specific opt-in without introducing a Data Collection category for raw log messages.Refs #5666
💚 How did you test it?
./gradlew :sentry-logback:check./gradlew spotlessApply apiDump.venv/bin/python test/system-test-runner.py test --module sentry-samples-logback --agent false --auto-init true --build trueincludeUnencodedMessage=true, verifying encoded bodies and original event/log template parametersgit diff --check📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Merge this PR into the previous stack branch before merging the remaining Data Collection stack into the collection branch.
#skip-changelog