Skip to content

Commit 85b919f

Browse files
adinaueradinauer
authored
Merge pull request #6076 from getsentry/feat/data-collection-logback-unencoded-message
feat(logback): [Data Collection 26] Add unencoded message opt-in
2 parents 2243aff + 8975006 commit 85b919f

5 files changed

Lines changed: 149 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- Add `dataCollection`, a fine-grained replacement for `sendDefaultPii`, for controlling data collected automatically by SDK integrations ([#5759](https://github.com/getsentry/sentry-java/pull/5759))
88
- `sendDefaultPii` remains supported for backwards compatibility. When `dataCollection` is not configured, the SDK preserves the existing `sendDefaultPii` behavior.
99
- Configuring any `dataCollection` option makes it the source of truth. `sendDefaultPii` is then ignored, and omitted `dataCollection` options use the defaults below.
10+
- The Logback appender is a compatibility exception. When an encoder is configured, `sendDefaultPii=true` continues to include the original message template and parameters. To opt in independently of `sendDefaultPii`, set `<includeUnencodedMessage>true</includeUnencodedMessage>` on the Sentry appender in `logback.xml` or `logback-spring.xml`.
1011
- Data explicitly supplied through APIs such as `Sentry.setUser`, scopes, event processors, or `beforeSend` is not affected.
1112

1213
| Option | Default | Behavior |

‎sentry-logback/api/sentry-logback.api‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ public class io/sentry/logback/SentryAppender : ch/qos/logback/core/Unsynchroniz
1111
protected fun captureLog (Lch/qos/logback/classic/spi/ILoggingEvent;)V
1212
protected fun createBreadcrumb (Lch/qos/logback/classic/spi/ILoggingEvent;)Lio/sentry/Breadcrumb;
1313
protected fun createEvent (Lch/qos/logback/classic/spi/ILoggingEvent;)Lio/sentry/SentryEvent;
14+
public fun getIncludeUnencodedMessage ()Z
1415
public fun getMinimumBreadcrumbLevel ()Lch/qos/logback/classic/Level;
1516
public fun getMinimumEventLevel ()Lch/qos/logback/classic/Level;
1617
public fun getMinimumLevel ()Lch/qos/logback/classic/Level;
1718
public fun setEncoder (Lch/qos/logback/core/encoder/Encoder;)V
19+
public fun setIncludeUnencodedMessage (Z)V
1820
public fun setMinimumBreadcrumbLevel (Lch/qos/logback/classic/Level;)V
1921
public fun setMinimumEventLevel (Lch/qos/logback/classic/Level;)V
2022
public fun setMinimumLevel (Lch/qos/logback/classic/Level;)V

‎sentry-logback/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ dependencies {
2929
testImplementation(projects.sentryTestSupport)
3030
testImplementation(kotlin(Config.kotlinStdLib))
3131
testImplementation(libs.kotlin.test.junit)
32+
testImplementation(libs.google.truth)
3233
testImplementation(libs.logback.classic)
3334
testImplementation(libs.mockito.kotlin)
3435
}

‎sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public class SentryAppender extends UnsynchronizedAppenderBase<ILoggingEvent> {
5353
private @NotNull Level minimumEventLevel = Level.ERROR;
5454
private @NotNull Level minimumLevel = Level.INFO;
5555
private @Nullable Encoder<ILoggingEvent> encoder;
56+
private boolean includeUnencodedMessage = false;
5657

5758
static {
5859
SentryIntegrationPackageStorage.getInstance()
@@ -117,8 +118,9 @@ protected void append(@NotNull ILoggingEvent eventObject) {
117118
final SentryEvent event = new SentryEvent(DateUtils.getDateTime(loggingEvent.getTimeStamp()));
118119
final Message message = new Message();
119120

120-
// if encoder is set we treat message+params as PII as encoders may be used to mask/strip PII
121-
if (encoder == null || ScopesAdapter.getInstance().getOptions().isSendDefaultPii()) {
121+
// Encoders may mask or strip PII. When one is configured, include the original message and
122+
// parameters only if includeUnencodedMessage or the legacy sendDefaultPii option is enabled.
123+
if (shouldIncludeUnencodedMessage()) {
122124
message.setMessage(loggingEvent.getMessage());
123125
message.setParams(toParams(loggingEvent.getArgumentArray()));
124126
}
@@ -176,8 +178,9 @@ protected void captureLog(@NotNull ILoggingEvent loggingEvent) {
176178
final @NotNull SentryAttributes attributes = SentryAttributes.of();
177179
final @NotNull String formattedMessage = formatted(loggingEvent);
178180

179-
// if encoder is set we treat message+params as PII as encoders may be used to mask/strip PII
180-
if (encoder == null || ScopesAdapter.getInstance().getOptions().isSendDefaultPii()) {
181+
// Encoders may mask or strip PII. When one is configured, include the original message and
182+
// parameters only if includeUnencodedMessage or the legacy sendDefaultPii option is enabled.
183+
if (shouldIncludeUnencodedMessage()) {
181184
final @Nullable String nonFormattedMessage = loggingEvent.getMessage();
182185
if (nonFormattedMessage != null && !formattedMessage.equals(nonFormattedMessage)) {
183186
attributes.add(
@@ -197,6 +200,12 @@ protected void captureLog(@NotNull ILoggingEvent loggingEvent) {
197200
Sentry.logger().log(sentryLevel, params, formattedMessage, arguments);
198201
}
199202

203+
private boolean shouldIncludeUnencodedMessage() {
204+
return encoder == null
205+
|| includeUnencodedMessage
206+
|| ScopesAdapter.getInstance().getOptions().isSendDefaultPii();
207+
}
208+
200209
private String formatted(@NotNull ILoggingEvent loggingEvent) {
201210
if (encoder != null) {
202211
try {
@@ -331,4 +340,21 @@ void setTransportFactory(final @Nullable ITransportFactory transportFactory) {
331340
public void setEncoder(Encoder<ILoggingEvent> encoder) {
332341
this.encoder = encoder;
333342
}
343+
344+
/**
345+
* Whether to include the original message template and parameters when an encoder is configured.
346+
*/
347+
public boolean getIncludeUnencodedMessage() {
348+
return includeUnencodedMessage;
349+
}
350+
351+
/**
352+
* Sets whether to include the original message template and parameters when an encoder is
353+
* configured.
354+
*
355+
* @param includeUnencodedMessage whether to include the original message and parameters
356+
*/
357+
public void setIncludeUnencodedMessage(final boolean includeUnencodedMessage) {
358+
this.includeUnencodedMessage = includeUnencodedMessage;
359+
}
334360
}

‎sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import ch.qos.logback.classic.spi.ThrowableProxy
1010
import ch.qos.logback.core.encoder.Encoder
1111
import ch.qos.logback.core.encoder.EncoderBase
1212
import ch.qos.logback.core.status.Status
13+
import com.google.common.truth.Truth.assertThat
1314
import io.sentry.ITransportFactory
1415
import io.sentry.InitPriority
1516
import io.sentry.Sentry
@@ -53,6 +54,7 @@ class SentryAppenderTest {
5354
minimumLevel: Level? = null,
5455
contextTags: List<String>? = null,
5556
encoder: Encoder<ILoggingEvent>? = null,
57+
includeUnencodedMessage: Boolean = false,
5658
sendDefaultPii: Boolean = false,
5759
enableLogs: Boolean = false,
5860
options: SentryOptions = SentryOptions(),
@@ -85,6 +87,7 @@ class SentryAppenderTest {
8587
appender.setTransportFactory(transportFactory)
8688
encoder?.context = loggerContext
8789
appender.setEncoder(encoder)
90+
appender.setIncludeUnencodedMessage(includeUnencodedMessage)
8891
val rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME)
8992
rootLogger.level = Level.TRACE
9093
rootLogger.addAppender(appender)
@@ -205,6 +208,91 @@ class SentryAppenderTest {
205208
)
206209
}
207210

211+
@Test
212+
fun `includes unencoded message and params if explicitly enabled`() {
213+
val encoder = PatternLayoutEncoder()
214+
encoder.pattern = "encoded %msg"
215+
fixture =
216+
Fixture(
217+
minimumEventLevel = Level.DEBUG,
218+
encoder = encoder,
219+
includeUnencodedMessage = true,
220+
)
221+
fixture.logger.info("testing encoding {}", "param1")
222+
223+
verify(fixture.transport)
224+
.send(
225+
checkEvent { event ->
226+
assertThat(event.message?.formatted).isEqualTo("encoded testing encoding param1")
227+
assertThat(event.message?.message).isEqualTo("testing encoding {}")
228+
assertThat(event.message?.params).containsExactly("param1")
229+
},
230+
anyOrNull(),
231+
)
232+
}
233+
234+
@Test
235+
fun `data collection does not include unencoded message`() {
236+
val encoder = PatternLayoutEncoder()
237+
encoder.pattern = "encoded %msg"
238+
val options = SentryOptions().apply { dataCollection.setFilePaths(false) }
239+
fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder, options = options)
240+
fixture.logger.info("testing encoding {}", "param1")
241+
242+
verify(fixture.transport)
243+
.send(
244+
checkEvent { event ->
245+
assertThat(event.message?.formatted).isEqualTo("encoded testing encoding param1")
246+
assertThat(event.message?.message).isNull()
247+
assertThat(event.message?.params).isNull()
248+
},
249+
anyOrNull(),
250+
)
251+
}
252+
253+
@Test
254+
fun `sendDefaultPii includes unencoded message when data collection is configured`() {
255+
val encoder = PatternLayoutEncoder()
256+
encoder.pattern = "encoded %msg"
257+
val options = SentryOptions().apply { dataCollection.setFilePaths(false) }
258+
fixture =
259+
Fixture(
260+
minimumEventLevel = Level.DEBUG,
261+
encoder = encoder,
262+
sendDefaultPii = true,
263+
options = options,
264+
)
265+
fixture.logger.info("testing encoding {}", "param1")
266+
267+
verify(fixture.transport)
268+
.send(
269+
checkEvent { event ->
270+
assertThat(event.message?.formatted).isEqualTo("encoded testing encoding param1")
271+
assertThat(event.message?.message).isEqualTo("testing encoding {}")
272+
assertThat(event.message?.params).containsExactly("param1")
273+
},
274+
anyOrNull(),
275+
)
276+
}
277+
278+
@Test
279+
fun `include unencoded message defaults to false and has no effect without encoder`() {
280+
fixture = Fixture(minimumEventLevel = Level.DEBUG, includeUnencodedMessage = true)
281+
fixture.logger.info("testing encoding {}", "param1")
282+
283+
assertThat(SentryAppender().includeUnencodedMessage).isFalse()
284+
assertThat(fixture.appender.includeUnencodedMessage).isTrue()
285+
verify(fixture.transport)
286+
.send(
287+
checkEvent { event ->
288+
assertThat(event.message?.formatted).isEqualTo("testing encoding param1")
289+
assertThat(event.message?.message).isEqualTo("testing encoding {}")
290+
assertThat(event.message?.params).containsExactly("param1")
291+
},
292+
anyOrNull(),
293+
)
294+
}
295+
208296
class ThrowingEncoder : EncoderBase<ILoggingEvent> {
209297
constructor() : super()
210298

@@ -806,6 +894,33 @@ class SentryAppenderTest {
806894
)
807895
}
808896

897+
@Test
898+
fun `sets template and attributes on log with encoder when unencoded message is included`() {
899+
val encoder = PatternLayoutEncoder()
900+
encoder.pattern = "encoded %msg"
901+
fixture =
902+
Fixture(
903+
minimumLevel = Level.ERROR,
904+
enableLogs = true,
905+
encoder = encoder,
906+
includeUnencodedMessage = true,
907+
)
908+
fixture.logger.error("testing message {}", "param")
909+
910+
Sentry.flush(1000)
911+
912+
verify(fixture.transport)
913+
.send(
914+
checkLogs { logs ->
915+
val log = logs.items.first()
916+
assertThat(log.body).isEqualTo("encoded testing message param")
917+
assertThat(log.attributes?.get("sentry.message.template")?.value)
918+
.isEqualTo("testing message {}")
919+
assertThat(log.attributes?.get("sentry.message.parameter.0")?.value).isEqualTo("param")
920+
}
921+
)
922+
}
923+
809924
@Test
810925
fun `sets template and attributes on log with encoder when sendDefaultPii is true`() {
811926
var encoder = PatternLayoutEncoder()

0 commit comments

Comments
 (0)