Skip to content

feat(logback): [Data Collection 26] Add unencoded message opt-in - #6076

Merged
adinauer merged 3 commits into
feat/data-collectionfrom
feat/data-collection-logback-unencoded-message
Sep 10, 2026
Merged

feat(logback): [Data Collection 26] Add unencoded message opt-in#6076
adinauer merged 3 commits into
feat/data-collectionfrom
feat/data-collection-logback-unencoded-message

Conversation

@adinauer

@adinauer adinauer commented Sep 9, 2026

Copy link
Copy Markdown
Member

PR Stack (Data Collection)


📜 Description

Add includeUnencodedMessage to the Logback SentryAppender. When an encoder is configured, this explicit integration option includes the original message template and parameters alongside the encoded output.

Keep sendDefaultPii=true as 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 sendDefaultPii option. 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 true
  • One-off Logback system test with a regex-masking encoder and includeUnencodedMessage=true, verifying encoded bodies and original event/log template parameters
  • git diff --check

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

Merge this PR into the previous stack branch before merging the remaining Data Collection stack into the collection branch.

#skip-changelog

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

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>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 8975006

This was referenced Sep 9, 2026
@sentry

sentry Bot commented Sep 9, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.55.0 (1) release

⚙️ sentry-android Build Distribution Settings

@adinauer
adinauer marked this pull request as ready for review September 9, 2026 07:52
@adinauer
adinauer requested a review from romtsn as a code owner September 9, 2026 07:52

@runningcode runningcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we update the comment here (and on line 122) about includeUnencodedMessage ?

Base automatically changed from feat/data-collection-file-paths to feat/data-collection September 10, 2026 04:19
adinauer and others added 2 commits September 10, 2026 07:10
Describe how encoder use, the integration opt-in, and the legacy PII option control original message data.

Refs #5666

Co-Authored-By: Claude <noreply@anthropic.com>
Align the includeUnencodedMessage getter and setter names so Kotlin exposes the option as a mutable synthetic property.

Refs #5666

Co-Authored-By: Claude <noreply@anthropic.com>
@adinauer
adinauer merged commit 85b919f into feat/data-collection Sep 10, 2026
57 checks passed
@adinauer
adinauer deleted the feat/data-collection-logback-unencoded-message branch September 10, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants