diff --git a/CHANGELOG.md b/CHANGELOG.md index 50e013db548..e6dd7a0301e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Add an explicit Logs opt-in to the Logback appender ([#5940](https://github.com/getsentry/sentry-java/pull/5940)) - Make `ISpan.startChild` overloads with `SpanOptions` public ([#5927](https://github.com/getsentry/sentry-java/pull/5927)) ### Improvements diff --git a/sentry-logback/api/sentry-logback.api b/sentry-logback/api/sentry-logback.api index 84a3c90d5cf..8d697f1b945 100644 --- a/sentry-logback/api/sentry-logback.api +++ b/sentry-logback/api/sentry-logback.api @@ -14,6 +14,8 @@ public class io/sentry/logback/SentryAppender : ch/qos/logback/core/Unsynchroniz public fun getMinimumBreadcrumbLevel ()Lch/qos/logback/classic/Level; public fun getMinimumEventLevel ()Lch/qos/logback/classic/Level; public fun getMinimumLevel ()Lch/qos/logback/classic/Level; + public fun isEnableLogs ()Z + public fun setEnableLogs (Z)V public fun setEncoder (Lch/qos/logback/core/encoder/Encoder;)V public fun setMinimumBreadcrumbLevel (Lch/qos/logback/classic/Level;)V public fun setMinimumEventLevel (Lch/qos/logback/classic/Level;)V 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 20fdf304bda..f36d988c97b 100644 --- a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java +++ b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java @@ -52,6 +52,7 @@ public class SentryAppender extends UnsynchronizedAppenderBase { private @NotNull Level minimumBreadcrumbLevel = Level.INFO; private @NotNull Level minimumEventLevel = Level.ERROR; private @NotNull Level minimumLevel = Level.INFO; + private boolean enableLogs = false; private @Nullable Encoder encoder; static { @@ -87,7 +88,8 @@ public void start() { @Override protected void append(@NotNull ILoggingEvent eventObject) { - if (ScopesAdapter.getInstance().getOptions().getLogs().isEnabled() + if (enableLogs + && ScopesAdapter.getInstance().getOptions().getLogs().isEnabled() && eventObject.getLevel().isGreaterOrEqual(minimumLevel)) { captureLog(eventObject); } @@ -323,6 +325,14 @@ public void setMinimumLevel(final @Nullable Level minimumLevel) { return minimumLevel; } + public void setEnableLogs(final boolean enableLogs) { + this.enableLogs = enableLogs; + } + + public boolean isEnableLogs() { + return enableLogs; + } + @ApiStatus.Internal void setTransportFactory(final @Nullable ITransportFactory transportFactory) { this.transportFactory = transportFactory; 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 877d2a23d75..7aafe705277 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -38,6 +38,7 @@ import kotlin.test.assertTrue import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.slf4j.Logger @@ -55,6 +56,7 @@ class SentryAppenderTest { encoder: Encoder? = null, sendDefaultPii: Boolean = false, enableLogs: Boolean = false, + enableGlobalLogs: Boolean = enableLogs, options: SentryOptions = SentryOptions(), startLater: Boolean = false, ) { @@ -71,7 +73,7 @@ class SentryAppenderTest { this.encoder = encoder options.dsn = dsn options.isSendDefaultPii = sendDefaultPii - options.logs.isEnabled = enableLogs + options.logs.isEnabled = enableGlobalLogs options.logs.loggerBatchProcessorFactory = ILoggerBatchProcessorFactory { options, client -> LoggerBatchProcessor(options, client, ImmediateExecutorService()) } @@ -81,6 +83,7 @@ class SentryAppenderTest { appender.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel) appender.setMinimumEventLevel(minimumEventLevel) appender.setMinimumLevel(minimumLevel) + appender.setEnableLogs(enableLogs) appender.context = loggerContext appender.setTransportFactory(transportFactory) encoder?.context = loggerContext @@ -322,6 +325,57 @@ class SentryAppenderTest { ) } + @Test + fun `does not capture logs by default when aggregate logs are enabled`() { + fixture = Fixture(enableGlobalLogs = true) + + assertFalse(fixture.appender.isEnableLogs) + fixture.logger.info("this should not be captured as a log") + Sentry.flush(10) + + verify(fixture.transport, never()).send(checkLogs {}) + } + + @Test + fun `captures logs when local and aggregate logs are enabled`() { + fixture = Fixture(enableLogs = true) + + assertTrue(fixture.appender.isEnableLogs) + fixture.logger.info("this should be captured as a log") + Sentry.flush(10) + + verify(fixture.transport) + .send( + checkLogs { logs -> + assertEquals("this should be captured as a log", logs.items.first().body) + } + ) + } + + @Test + fun `captures events and breadcrumbs when local logs are disabled`() { + fixture = + Fixture( + minimumBreadcrumbLevel = Level.INFO, + minimumEventLevel = Level.ERROR, + enableGlobalLogs = true, + ) + + fixture.logger.info("this should be a breadcrumb") + fixture.logger.error("this should be an event") + Sentry.flush(10) + + verify(fixture.transport) + .send( + checkEvent { event -> + assertEquals("this should be an event", event.message?.formatted) + assertEquals("this should be a breadcrumb", event.breadcrumbs?.single()?.message) + }, + anyOrNull(), + ) + verify(fixture.transport, never()).send(checkLogs {}) + } + @Test fun `converts trace log level to Sentry log level`() { fixture = Fixture(minimumLevel = Level.TRACE, enableLogs = true) diff --git a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml index 8082af4483b..7b70bcda1c0 100644 --- a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml +++ b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml @@ -17,12 +17,13 @@ true + true WARN DEBUG - + INFO