diff --git a/CHANGELOG.md b/CHANGELOG.md index ce808f2be5e..686e61daa2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Only set log template for logging integrations if formatted message differs from template ([#4682](https://github.com/getsentry/sentry-java/pull/4682)) + ### Features - Add support for Spring Boot 4 and Spring 7 ([#4601](https://github.com/getsentry/sentry-java/pull/4601)) diff --git a/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java b/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java index 23cba406b16..812fdf184db 100644 --- a/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java +++ b/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java @@ -154,9 +154,12 @@ protected void captureLog(@NotNull LogRecord loggingEvent) { message = loggingEvent.getResourceBundle().getString(loggingEvent.getMessage()); } - attributes.add(SentryAttribute.stringAttribute("sentry.message.template", message)); - final @NotNull String formattedMessage = maybeFormatted(arguments, message); + + if (!formattedMessage.equals(message)) { + attributes.add(SentryAttribute.stringAttribute("sentry.message.template", message)); + } + final @NotNull SentryLogParameters params = SentryLogParameters.create(attributes); params.setOrigin("auto.log.jul"); diff --git a/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt b/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt index 8f39c0d94a5..a002c986407 100644 --- a/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt +++ b/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt @@ -35,6 +35,7 @@ class SentryHandlerTest { val configureWithLogManager: Boolean = false, val transport: ITransport = mock(), contextTags: List? = null, + printfStyle: Boolean? = null, ) { var logger: Logger var handler: SentryHandler @@ -49,6 +50,9 @@ class SentryHandlerTest { handler.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel) handler.setMinimumEventLevel(minimumEventLevel) handler.setMinimumLevel(minimumLevel) + if (printfStyle == true) { + handler.setPrintfStyle(printfStyle) + } handler.level = Level.ALL logger.handlers.forEach { logger.removeHandler(it) } logger.addHandler(handler) @@ -476,4 +480,79 @@ class SentryHandlerTest { verify(fixture.transport) .send(checkLogs { event -> assertEquals(SentryLogLevel.ERROR, event.items.first().level) }) } + + @Test + fun `does not set template on log when logging message without parameters`() { + fixture = Fixture(minimumLevel = Level.SEVERE) + fixture.logger.severe("testing message without parameters") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message without parameters", log.body) + assertNull(log.attributes?.get("sentry.message.template")) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters`() { + fixture = Fixture(minimumLevel = Level.SEVERE) + fixture.logger.log(Level.SEVERE, "testing message {0}", arrayOf("param")) + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message param", log.body) + assertEquals("testing message {0}", log.attributes?.get("sentry.message.template")?.value) + assertEquals("param", log.attributes?.get("sentry.message.parameter.0")?.value) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters and using printfStyle`() { + fixture = Fixture(minimumLevel = Level.SEVERE, printfStyle = true) + fixture.logger.log(Level.SEVERE, "testing message %s", arrayOf("param")) + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message param", log.body) + assertEquals("testing message %s", log.attributes?.get("sentry.message.template")?.value) + assertEquals("param", log.attributes?.get("sentry.message.parameter.0")?.value) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters and formatting fails`() { + fixture = Fixture(minimumLevel = Level.SEVERE, printfStyle = true) + fixture.logger.log(Level.SEVERE, "testing message %d %d", arrayOf(1)) + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message %d %d", log.body) + assertEquals( + "testing message %d %d", + log.attributes?.get("sentry.message.template")?.value, + ) + assertEquals(1, log.attributes?.get("sentry.message.parameter.0")?.value) + assertNull(log.attributes?.get("sentry.message.parameter.1")) + } + ) + } } diff --git a/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java b/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java index 9e4cf0eed9f..7c35febd169 100644 --- a/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java +++ b/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java @@ -222,11 +222,14 @@ protected void captureLog(@NotNull LogEvent loggingEvent) { final @Nullable Object[] arguments = loggingEvent.getMessage().getParameters(); final @NotNull SentryAttributes attributes = SentryAttributes.of(); - attributes.add( - SentryAttribute.stringAttribute( - "sentry.message.template", loggingEvent.getMessage().getFormat())); - + final @Nullable String nonFormattedMessage = loggingEvent.getMessage().getFormat(); final @NotNull String formattedMessage = loggingEvent.getMessage().getFormattedMessage(); + + if (nonFormattedMessage != null && !formattedMessage.equals(nonFormattedMessage)) { + attributes.add( + SentryAttribute.stringAttribute("sentry.message.template", nonFormattedMessage)); + } + final @NotNull SentryLogParameters params = SentryLogParameters.create(attributes); params.setOrigin("auto.log.log4j2"); diff --git a/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt b/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt index 9ddfdbd43be..185972df905 100644 --- a/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt +++ b/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt @@ -533,4 +533,62 @@ class SentryAppenderTest { fixture.getSut(debug = true) assertTrue(ScopesAdapter.getInstance().options.isDebug) } + + @Test + fun `does not set template on log when logging message without parameters`() { + val logger = fixture.getSut(minimumLevel = Level.ERROR) + logger.error("testing message without parameters") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message without parameters", log.body) + assertNull(log.attributes?.get("sentry.message.template")) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters`() { + val logger = fixture.getSut(minimumLevel = Level.ERROR) + logger.error("testing message {}", "param") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message param", log.body) + assertEquals("testing message {}", log.attributes?.get("sentry.message.template")?.value) + assertEquals("param", log.attributes?.get("sentry.message.parameter.0")?.value) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters and number of parameters is wrong`() { + val logger = fixture.getSut(minimumLevel = Level.ERROR) + logger.error("testing message {} {} {}", "param1", "param2") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message param1 param2 {}", log.body) + assertEquals( + "testing message {} {} {}", + log.attributes?.get("sentry.message.template")?.value, + ) + assertEquals("param1", log.attributes?.get("sentry.message.parameter.0")?.value) + assertEquals("param2", log.attributes?.get("sentry.message.parameter.1")?.value) + assertNull(log.attributes?.get("sentry.message.parameter.2")) + } + ) + } } diff --git a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java index 7d9d53c706d..65a849d72bc 100644 --- a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java +++ b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java @@ -183,15 +183,18 @@ protected void captureLog(@NotNull ILoggingEvent loggingEvent) { @Nullable Object[] arguments = null; final @NotNull SentryAttributes attributes = SentryAttributes.of(); + 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 if (encoder == null || ScopesAdapter.getInstance().getOptions().isSendDefaultPii()) { - attributes.add( - SentryAttribute.stringAttribute("sentry.message.template", loggingEvent.getMessage())); + final @Nullable String nonFormattedMessage = loggingEvent.getMessage(); + if (nonFormattedMessage != null && !formattedMessage.equals(nonFormattedMessage)) { + attributes.add( + SentryAttribute.stringAttribute("sentry.message.template", nonFormattedMessage)); + } arguments = loggingEvent.getArgumentArray(); } - final @NotNull String formattedMessage = formatted(loggingEvent); final @NotNull SentryLogParameters params = SentryLogParameters.create(attributes); params.setOrigin("auto.log.logback"); diff --git a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt index ffe84ed55df..8c7acbc5725 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -711,4 +711,114 @@ class SentryAppenderTest { anyOrNull(), ) } + + @Test + fun `does not set template on log when logging message without parameters`() { + fixture = Fixture(minimumLevel = Level.ERROR, enableLogs = true) + fixture.logger.error("testing message without parameters") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message without parameters", log.body) + assertNull(log.attributes?.get("sentry.message.template")) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters`() { + fixture = Fixture(minimumLevel = Level.ERROR, enableLogs = true) + fixture.logger.error("testing message {}", "param") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message param", log.body) + assertEquals("testing message {}", log.attributes?.get("sentry.message.template")?.value) + assertEquals("param", log.attributes?.get("sentry.message.parameter.0")?.value) + } + ) + } + + @Test + fun `sets template on log when logging message with parameters and number of parameters is wrong`() { + fixture = Fixture(minimumLevel = Level.ERROR, enableLogs = true) + fixture.logger.error("testing message {} {} {}", "param1", "param2") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("testing message param1 param2 {}", log.body) + assertEquals( + "testing message {} {} {}", + log.attributes?.get("sentry.message.template")?.value, + ) + assertEquals("param1", log.attributes?.get("sentry.message.parameter.0")?.value) + assertEquals("param2", log.attributes?.get("sentry.message.parameter.1")?.value) + assertNull(log.attributes?.get("sentry.message.parameter.2")) + } + ) + } + + @Test + fun `does not set template or attributes on log with encoder when sendDefaultPii is false`() { + var encoder = PatternLayoutEncoder() + encoder.pattern = "encoded %msg" + fixture = + Fixture( + minimumLevel = Level.ERROR, + enableLogs = true, + encoder = encoder, + sendDefaultPii = false, + ) + fixture.logger.error("testing message {}", "param") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("encoded testing message param", log.body) + assertNull(log.attributes?.get("sentry.message.template")) + assertNull(log.attributes?.get("sentry.message.parameter.0")) + } + ) + } + + @Test + fun `sets template and attributes on log with encoder when sendDefaultPii is true`() { + var encoder = PatternLayoutEncoder() + encoder.pattern = "encoded %msg" + fixture = + Fixture( + minimumLevel = Level.ERROR, + enableLogs = true, + encoder = encoder, + sendDefaultPii = true, + ) + fixture.logger.error("testing message {}", "param") + + Sentry.flush(1000) + + verify(fixture.transport) + .send( + checkLogs { logs -> + val log = logs.items.first() + assertEquals("encoded testing message param", log.body) + assertEquals("testing message {}", log.attributes?.get("sentry.message.template")?.value) + assertEquals("param", log.attributes?.get("sentry.message.parameter.0")?.value) + } + ) + } }