diff --git a/.cursor/rules/options.mdc b/.cursor/rules/options.mdc index 2d239da7813..06d1c02c44e 100644 --- a/.cursor/rules/options.mdc +++ b/.cursor/rules/options.mdc @@ -4,7 +4,10 @@ description: Adding and modifying SDK options --- # Adding Options to the SDK -New features must be **opt-in by default**. Options control whether a feature is enabled and how it behaves. +New automatic capture features must be **opt-in by default**. Deliberate API calls such as +`Sentry.logger()` and `Sentry.metrics()` capture whenever the SDK is enabled and do not have +aggregate signal enable flags. Options control signal behavior and whether individual automatic +sources are enabled. ## Namespaced Options @@ -12,12 +15,13 @@ Newer features use namespaced option classes nested inside `SentryOptions`, e.g. - `SentryOptions.getLogs()` → `SentryOptions.Logs` - `SentryOptions.getMetrics()` → `SentryOptions.Metrics` -Each namespaced options class is a `public static final class` inside `SentryOptions` with its own fields, getters/setters, and callbacks (e.g. `BeforeSendLogCallback`, `BeforeSendMetricCallback`). +Each namespaced options class is a `public static final class` inside `SentryOptions` with its own fields, getters/setters, factories, and callbacks (e.g. `BeforeSendLogCallback`, `BeforeSendMetricCallback`). -A typical namespaced options class contains: -- `enabled` boolean (default `false` for opt-in) -- `sampleRate` double (if the feature supports sampling) -- `beforeSend` callback interface (nested inside the options class) +Do not assume that a namespaced signal needs an aggregate `enabled` field. In particular, Logs and +Metrics are available through deliberate API calls whenever the SDK is enabled. Their namespaced +options contain signal-specific behavior such as `beforeSend`, limits, and processor factories. Add +options such as sampling only when the signal supports them. Automatic capture integrations use +source-local enable options that default to `false`. To add a new namespaced options class: 1. Create the `public static final class` inside `SentryOptions` with fields, getters/setters, and any callback interfaces @@ -47,21 +51,18 @@ The core options class. Add the field (or nested class) with getter/setter here. Allows setting options via `sentry.properties` file or system properties. Fields use nullable wrapper types (`@Nullable Boolean`, `@Nullable Double`) since unset means "don't override the default." **File:** `sentry/src/main/java/io/sentry/ExternalOptions.java` -- Add `@Nullable` fields with getter/setter for each externally configurable option (e.g. `enableMetrics`, `logsSampleRate`) -- Wire them in the static `from(PropertiesProvider)` method: - - Boolean: `propertiesProvider.getBooleanProperty("metrics.enabled")` - - Double: `propertiesProvider.getDoubleProperty("logs.sample-rate")` +- Add `@Nullable` fields with getter/setter for each externally configurable option (e.g. `logsSampleRate`) +- Wire them in the static `from(PropertiesProvider)` method, for example: + `propertiesProvider.getDoubleProperty("logs.sample-rate")` **File:** `sentry/src/main/java/io/sentry/SentryOptions.java` — `merge()` method - Add null-check blocks to apply each external option onto the namespaced options class: ```java - if (options.isEnableMetrics() != null) { - getMetrics().setEnabled(options.isEnableMetrics()); - } if (options.getLogsSampleRate() != null) { getLogs().setSampleRate(options.getLogsSampleRate()); } ``` +- Do not add or restore `logs.enabled` or `metrics.enabled`; these aggregate options are obsolete. **Tests:** - `sentry/src/test/java/io/sentry/ExternalOptionsTest.kt` — test true/false/null for booleans, valid values and null for doubles @@ -72,9 +73,12 @@ Allows setting options via `sentry.properties` file or system properties. Fields Allows setting options via `AndroidManifest.xml` `` tags. **File:** `sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java` -- Add a `static final String` constant for the key (e.g. `"io.sentry.metrics.enabled"`) +- Add a `static final String` constant for the key - Read it in `applyMetadata()` using `readBool(metadata, logger, CONSTANT, defaultValue)` -- Apply to the namespaced options, e.g. `options.getMetrics().setEnabled(...)` +- Apply automatic-source options directly, for example + `options.setEnableLogcatLogs(...)` for `io.sentry.logcat.logs.enabled` +- Do not add or restore `io.sentry.logs.enabled` or `io.sentry.metrics.enabled`; those aggregate + keys are obsolete. **Tests:** `sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt` - Test default value preserved when not in manifest @@ -83,21 +87,37 @@ Allows setting options via `AndroidManifest.xml` `` tags. ### 4. Spring Boot Properties (Spring Boot only) -`SentryProperties` extends `SentryOptions`, so namespaced options (nested classes) are automatically available as Spring Boot properties without extra code. For example, `SentryOptions.Logs` is automatically mapped to `sentry.logs.enabled` in `application.properties`. +`SentryProperties` extends `SentryOptions`, so bindable namespaced behavior options are available +through the `SentryOptions` class hierarchy. Spring-owned integration controls belong to a Spring +namespace instead. For example, `SentryProperties.Logging.enableLogs` binds to +`sentry.logging.enable-logs` and controls Logs forwarding from the auto-configured Logback +appender. `sentry.logging.enabled` separately controls whether that appender is installed. -No additional code is needed for namespaced options — Spring Boot auto-configuration handles this via property binding on the `SentryOptions` class hierarchy. +Do not add or restore `sentry.logs.enabled` or `sentry.metrics.enabled`; those aggregate properties +are obsolete. **Tests:** `sentry-spring-boot*/src/test/kotlin/.../SentryAutoConfigurationTest.kt` -- Add the property (e.g. `"sentry.logs.enabled=true"`) to the existing `resolves all properties` test +- Add the new property to the existing binding test - Assert the value is set on the resolved `SentryProperties` bean +- Test default, explicit `true`, explicit `false`, and propagation into the owning integration - There are three Spring Boot modules with separate test files: `sentry-spring-boot`, `sentry-spring-boot-jakarta`, `sentry-spring-boot-4` ### 5. Reading Options at Runtime -Features check their options at usage time. For namespaced features the check typically happens in the feature's API class (e.g. `LoggerApi`, `MetricsApi`): -- Check `options.getLogs().isEnabled()` early and return if disabled -- Apply sampling via `options.getLogs().getSampleRate()` if applicable -- Apply `beforeSend` callback in `SentryClient` before sending +Deliberate APIs such as `LoggerApi` and `MetricsApi` do not check aggregate signal enable flags. +They capture whenever their scopes are enabled, then apply signal behavior such as sampling and +`beforeSend`. + +Automatic integrations must check their source-local opt-in without affecting their existing event +or breadcrumb paths. Current Logs controls are: +- Logback: appender `enableLogs` +- Log4j2: appender `enableLogs` +- JUL: handler `enableLogs` +- Spring Boot Logback: `sentry.logging.enable-logs` +- Timber: `enableTimberLogs` / `io.sentry.timber.logs.enabled` +- Logcat: `enableLogcatLogs` / `io.sentry.logcat.logs.enabled` + +All source-local options default to `false` and gate only Sentry Logs forwarding. When a feature has its own capture path (e.g. `captureLog`), the relevant classes are: - `ISentryClient` — add the capture method signature @@ -106,10 +126,11 @@ When a feature has its own capture path (e.g. `captureLog`), the relevant classe ## Checklist for Adding a New Namespaced Option -1. `SentryOptions.java` — nested options class + getter/setter on `SentryOptions` -2. `ExternalOptions.java` — `@Nullable` fields + wiring in `from()` -3. `SentryOptions.java` `merge()` — apply external options to namespaced class -4. `ManifestMetadataReader.java` — Android manifest support (if Android-relevant) -5. `SentryAutoConfigurationTest.kt` — Spring Boot property binding tests (all three Spring Boot modules) -6. Tests for all of the above (`SentryOptionsTest`, `ExternalOptionsTest`, `ManifestMetadataReaderTest`) -7. Run `./gradlew apiDump` — the nested class and its methods appear in `sentry.api` +1. Decide whether the option controls deliberate API behavior or an automatic capture source +2. `SentryOptions.java` — nested behavior option + getter/setter where core ownership is appropriate +3. `ExternalOptions.java` and `SentryOptions.merge()` — add external support if applicable +4. `ManifestMetadataReader.java` — add Android support if applicable +5. Spring properties — use the owning integration namespace and test all three Spring Boot modules +6. Test defaults and every supported configuration layer +7. Verify automatic-source opt-ins default to `false` and do not gate events or breadcrumbs +8. Run `./gradlew spotlessApply apiDump` diff --git a/CHANGELOG.md b/CHANGELOG.md index 5476f6d9831..0f834d3f7fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ ### Features +- Remove the aggregate Sentry Metrics enable flag; `Sentry.metrics()` calls now capture Metrics by default ([#5953](https://github.com/getsentry/sentry-java/pull/5953)) +- Remove the aggregate Sentry Logs enable flag; manual `Sentry.logger()` calls now capture Logs by default ([#5947](https://github.com/getsentry/sentry-java/pull/5947)) +- Add an explicit Logs opt-in to Spring Boot logging auto-configuration ([#5946](https://github.com/getsentry/sentry-java/pull/5946)) +- Add an explicit Logs opt-in to the Android Logcat integration ([#5945](https://github.com/getsentry/sentry-java/pull/5945)) +- Add an explicit Logs opt-in to the Android Timber integration ([#5943](https://github.com/getsentry/sentry-java/pull/5943)) +- Add an explicit Logs opt-in to the JUL handler ([#5942](https://github.com/getsentry/sentry-java/pull/5942)) +- Add an explicit Logs opt-in to the Log4j2 appender ([#5941](https://github.com/getsentry/sentry-java/pull/5941)) +- Add an explicit Logs opt-in to the Logback appender ([#5940](https://github.com/getsentry/sentry-java/pull/5940)) - Deprecate `sendDefaultPii` in favor of `dataCollection` ahead of its removal in 9.0 ([#6158](https://github.com/getsentry/sentry-java/pull/6158)) - Make the tombstone merge time threshold configurable via `SentryAndroidOptions.setTombstoneMergeTimeThresholdMillis` and the `io.sentry.tombstone.merge-time-threshold-millis` manifest option ([#6154](https://github.com/getsentry/sentry-java/pull/6154)) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index efd776cabb8..23604c35cd3 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -453,6 +453,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun isEnableAutoActivityLifecycleTracing ()Z public fun isEnableAutoTraceIdGeneration ()Z public fun isEnableFramesTracking ()Z + public fun isEnableLogcatLogs ()Z public fun isEnableNdk ()Z public fun isEnableNdkAppHangTracking ()Z public fun isEnableNetworkEventBreadcrumbs ()Z @@ -462,6 +463,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun isEnableStandaloneAppStartTracing ()Z public fun isEnableSystemEventBreadcrumbs ()Z public fun isEnableSystemEventBreadcrumbsExtras ()Z + public fun isEnableTimberLogs ()Z public fun isMemoryLimiterEnabled ()Z public fun isReportHistoricalAnrs ()Z public fun isReportHistoricalMemoryLimiterExits ()Z @@ -488,6 +490,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableAutoActivityLifecycleTracing (Z)V public fun setEnableAutoTraceIdGeneration (Z)V public fun setEnableFramesTracking (Z)V + public fun setEnableLogcatLogs (Z)V public fun setEnableNdk (Z)V public fun setEnableNdkAppHangTracking (Z)V public fun setEnableNetworkEventBreadcrumbs (Z)V @@ -497,6 +500,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableStandaloneAppStartTracing (Z)V public fun setEnableSystemEventBreadcrumbs (Z)V public fun setEnableSystemEventBreadcrumbsExtras (Z)V + public fun setEnableTimberLogs (Z)V public fun setFrameMetricsCollector (Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;)V public fun setMemoryLimiterEnabled (Z)V public fun setNativeHandlerStrategy (Lio/sentry/android/core/NdkHandlerStrategy;)V diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index 72a526a7a5e..7ff78d4d578 100644 --- a/sentry-android-core/build.gradle.kts +++ b/sentry-android-core/build.gradle.kts @@ -126,6 +126,7 @@ dependencies { testImplementation(projects.sentrySpotlight) testImplementation(projects.sentryAndroidFragment) testImplementation(projects.sentryAndroidTimber) + testImplementation(libs.timber) testImplementation(projects.sentryAndroidReplay) testImplementation(projects.sentryCompose) testImplementation(projects.sentryAndroidNdk) @@ -136,5 +137,4 @@ dependencies { testImplementation(libs.androidx.compose.foundation.layout) testImplementation(libs.androidx.compose.material3) testRuntimeOnly(libs.androidx.fragment.ktx) - testRuntimeOnly(libs.timber) } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 12ffb90356c..c6c29a323fd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -479,7 +479,7 @@ static void installDefaultIntegrations( } if (isTimberAvailable) { - options.addIntegration(new SentryTimberIntegration()); + options.addIntegration(new SentryTimberIntegration(() -> options.isEnableTimberLogs())); } options.addIntegration(new AppComponentsBreadcrumbsIntegration(context)); options.addIntegration(new SystemEventsBreadcrumbsIntegration(context)); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index 7de5a0c2716..217f4937bf1 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -192,9 +192,9 @@ final class ManifestMetadataReader { static final String IN_APP_EXCLUDES = "io.sentry.in-app-excludes"; - static final String ENABLE_LOGS = "io.sentry.logs.enabled"; + static final String ENABLE_TIMBER_LOGS = "io.sentry.timber.logs.enabled"; - static final String ENABLE_METRICS = "io.sentry.metrics.enabled"; + static final String ENABLE_LOGCAT_LOGS = "io.sentry.logcat.logs.enabled"; static final String ENABLE_AUTO_TRACE_ID_GENERATION = "io.sentry.traces.enable-auto-id-generation"; @@ -750,14 +750,11 @@ static void applyMetadata( } } - options - .getLogs() - .setEnabled(readBool(metadata, logger, ENABLE_LOGS, options.getLogs().isEnabled())); + options.setEnableTimberLogs( + readBool(metadata, logger, ENABLE_TIMBER_LOGS, options.isEnableTimberLogs())); - options - .getMetrics() - .setEnabled( - readBool(metadata, logger, ENABLE_METRICS, options.getMetrics().isEnabled())); + options.setEnableLogcatLogs( + readBool(metadata, logger, ENABLE_LOGCAT_LOGS, options.isEnableLogcatLogs())); final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions(); feedbackOptions.setNameRequired( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index c135172bc5c..995499b72b2 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -75,6 +75,12 @@ public final class SentryAndroidOptions extends SentryOptions { /** Enable or disable automatic breadcrumbs for Network Events Using NetworkCallback */ private boolean enableNetworkEventBreadcrumbs = true; + /** Enable or disable automatic Sentry Logs capture from Timber. Default is disabled. */ + private boolean enableTimberLogs = false; + + /** Enable or disable automatic Sentry Logs capture from Logcat. Default is disabled. */ + private boolean enableLogcatLogs = false; + /** * Enables the Auto instrumentation for Activity lifecycle tracing. * @@ -505,6 +511,22 @@ public void setEnableNetworkEventBreadcrumbs(boolean enableNetworkEventBreadcrum this.enableNetworkEventBreadcrumbs = enableNetworkEventBreadcrumbs; } + public boolean isEnableTimberLogs() { + return enableTimberLogs; + } + + public void setEnableTimberLogs(boolean enableTimberLogs) { + this.enableTimberLogs = enableTimberLogs; + } + + public boolean isEnableLogcatLogs() { + return enableLogcatLogs; + } + + public void setEnableLogcatLogs(boolean enableLogcatLogs) { + this.enableLogcatLogs = enableLogcatLogs; + } + /** * Enable or disable all the automatic breadcrumbs * diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java index 1e649c1783f..998ce58b3db 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java @@ -6,6 +6,7 @@ import io.sentry.Sentry; import io.sentry.SentryLevel; import io.sentry.SentryLogLevel; +import io.sentry.SentryOptions; import io.sentry.logger.SentryLogParameters; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -52,8 +53,9 @@ private static void addAsLog( @Nullable final String msg, @Nullable final Throwable tr) { final @NotNull ScopesAdapter scopes = ScopesAdapter.getInstance(); - // Check if logs are enabled before doing expensive operations - if (!scopes.getOptions().getLogs().isEnabled()) { + final @NotNull SentryOptions options = scopes.getOptions(); + if (!(options instanceof SentryAndroidOptions) + || !((SentryAndroidOptions) options).isEnableLogcatLogs()) { return; } final @Nullable String trMessage = tr != null ? tr.getMessage() : null; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidLoggerBatchProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidLoggerBatchProcessorTest.kt index 369f7f6a148..66ae9f4d664 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidLoggerBatchProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidLoggerBatchProcessorTest.kt @@ -1,12 +1,15 @@ package io.sentry.android.core import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.ISentryClient import io.sentry.SentryLogEvent import io.sentry.SentryLogLevel import io.sentry.SentryOptions import io.sentry.protocol.SentryId import io.sentry.test.ImmediateExecutorService +import io.sentry.test.getProperty +import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -15,6 +18,7 @@ import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -55,6 +59,16 @@ class AndroidLoggerBatchProcessorTest { assertNotNull(AppState.getInstance().lifecycleObserver) } + @Test + fun `onBackground does not flush before first accepted item`() { + val sut = fixture.getSut(useImmediateExecutor = true) + + sut.onBackground() + + assertThat(sut.getProperty("hasScheduled").get()).isFalse() + verify(fixture.client, never()).captureBatchedLogEvents(any()) + } + @Test fun `onBackground schedules flush`() { val sut = fixture.getSut(useImmediateExecutor = true) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMetricsBatchProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMetricsBatchProcessorTest.kt index 7d85502d149..9c35808d0c7 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMetricsBatchProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMetricsBatchProcessorTest.kt @@ -1,11 +1,14 @@ package io.sentry.android.core import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.ISentryClient import io.sentry.SentryMetricsEvent import io.sentry.SentryOptions import io.sentry.protocol.SentryId import io.sentry.test.ImmediateExecutorService +import io.sentry.test.getProperty +import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -14,6 +17,7 @@ import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -54,6 +58,16 @@ class AndroidMetricsBatchProcessorTest { assertNotNull(AppState.getInstance().lifecycleObserver) } + @Test + fun `onBackground does not flush before first accepted item`() { + val sut = fixture.getSut(useImmediateExecutor = true) + + sut.onBackground() + + assertThat(sut.getProperty("hasScheduled").get()).isFalse() + verify(fixture.client, never()).captureBatchedMetricsEvents(any()) + } + @Test fun `onBackground schedules flush`() { val sut = fixture.getSut(useImmediateExecutor = true) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 4610e4bbcb8..3c4766917e2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -706,8 +706,21 @@ class AndroidOptionsInitializerTest { fun `SentryTimberIntegration added to the integration list if available on classpath`() { fixture.initSutWithClassLoader(isTimberAvailable = true) - val actual = fixture.sentryOptions.integrations.firstOrNull { it is SentryTimberIntegration } - assertNotNull(actual) + val actual = + fixture.sentryOptions.integrations.firstOrNull { it is SentryTimberIntegration } + as SentryTimberIntegration + assertFalse(actual.enableLogs) + } + + @Test + fun `SentryTimberIntegration receives Timber logs option`() { + fixture.sentryOptions.isEnableTimberLogs = true + fixture.initSutWithClassLoader(isTimberAvailable = true) + + val actual = + fixture.sentryOptions.integrations.firstOrNull { it is SentryTimberIntegration } + as SentryTimberIntegration + assertTrue(actual.enableLogs) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 463b5d82ddb..4e64dc616b5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -2176,66 +2176,52 @@ class ManifestMetadataReaderTest { } @Test - fun `applyMetadata reads logs enabled and keep default value if not found`() { - // Arrange + fun `applyMetadata keeps Timber logs disabled if not found`() { val context = fixture.getContext() - // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - // Assert - assertFalse(fixture.options.logs.isEnabled) + assertFalse(fixture.options.isEnableTimberLogs) } @Test - fun `applyMetadata reads logs enabled to options`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.ENABLE_LOGS to true) + fun `applyMetadata reads Timber logs enabled to options`() { + val bundle = bundleOf(ManifestMetadataReader.ENABLE_TIMBER_LOGS to true) val context = fixture.getContext(metaData = bundle) - // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - // Assert - assertTrue(fixture.options.logs.isEnabled) + assertTrue(fixture.options.isEnableTimberLogs) } @Test - fun `applyMetadata reads metrics enabled and keep default value if not found`() { - // Arrange + fun `applyMetadata keeps Logcat logs disabled if not found`() { val context = fixture.getContext() - // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - // Assert - assertTrue(fixture.options.metrics.isEnabled) + assertThat(fixture.options.isEnableLogcatLogs).isFalse() } @Test - fun `applyMetadata reads metrics enabled to options`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.ENABLE_METRICS to false) + fun `applyMetadata reads Logcat logs enabled to options`() { + val bundle = bundleOf(ManifestMetadataReader.ENABLE_LOGCAT_LOGS to true) val context = fixture.getContext(metaData = bundle) - // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - // Assert - assertFalse(fixture.options.metrics.isEnabled) + assertThat(fixture.options.isEnableLogcatLogs).isTrue() } @Test - fun `applyMetadata reads metrics enabled to options when set to true`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.ENABLE_METRICS to true) + fun `applyMetadata reads Logcat logs disabled to options`() { + fixture.options.isEnableLogcatLogs = true + val bundle = bundleOf(ManifestMetadataReader.ENABLE_LOGCAT_LOGS to false) val context = fixture.getContext(metaData = bundle) - // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - // Assert - assertTrue(fixture.options.metrics.isEnabled) + assertThat(fixture.options.isEnableLogcatLogs).isFalse() } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt index d580ff54cd0..f60e0dcb035 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt @@ -1,5 +1,6 @@ package io.sentry.android.core +import com.google.common.truth.Truth.assertThat import io.sentry.ITransactionProfiler import io.sentry.NoOpTransactionProfiler import io.sentry.protocol.DebugImage @@ -93,6 +94,34 @@ class SentryAndroidOptionsTest { assertTrue(sentryOptions.isEnableScopeSync) } + @Test + fun `Timber logs are disabled by default`() { + val sentryOptions = SentryAndroidOptions() + + assertFalse(sentryOptions.isEnableTimberLogs) + } + + @Test + fun `Timber logs can be enabled`() { + val sentryOptions = SentryAndroidOptions() + sentryOptions.isEnableTimberLogs = true + + assertTrue(sentryOptions.isEnableTimberLogs) + } + + @Test + fun `Logcat logs are disabled by default`() { + assertThat(SentryAndroidOptions().isEnableLogcatLogs).isFalse() + } + + @Test + fun `Logcat logs can be enabled`() { + val sentryOptions = SentryAndroidOptions() + sentryOptions.isEnableLogcatLogs = true + + assertThat(sentryOptions.isEnableLogcatLogs).isTrue() + } + @Test fun `attach screenshots disabled by default for Android`() { val sentryOptions = SentryAndroidOptions() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index 725a5a61122..d12adc5f0fa 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -23,6 +23,7 @@ import io.sentry.SentryEnvelope import io.sentry.SentryLevel import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.FATAL +import io.sentry.SentryLogEvent import io.sentry.SentryOptions import io.sentry.SentryOptions.BeforeSendCallback import io.sentry.Session @@ -84,6 +85,7 @@ import org.robolectric.annotation.Config import org.robolectric.shadow.api.Shadow import org.robolectric.shadows.ShadowActivityManager import org.robolectric.shadows.ShadowActivityManager.ApplicationExitInfoBuilder +import timber.log.Timber @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.N], shadows = [SentryShadowProcess::class]) @@ -238,6 +240,46 @@ class SentryAndroidTest { assertNotEquals(0, AppStartMetrics.getInstance().appStartTimeSpan.durationMs) } + @Test + fun `auto-installed Timber integration uses Logs option set in configuration callback`() { + val logs = mutableListOf() + fixture.initSut { options -> + options.isEnableTimberLogs = true + options.logs.beforeSend = + SentryOptions.Logs.BeforeSendLogCallback { log -> + logs.add(log) + log + } + } + + Timber.i("message") + + assertEquals(1, logs.size) + } + + @Test + fun `auto-installed Timber integration uses configuration callback override of manifest option`() { + val metadata = + Bundle().apply { + putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123") + putBoolean(ManifestMetadataReader.ENABLE_TIMBER_LOGS, true) + } + val mockContext = ContextUtilsTestHelper.mockMetaData(metaData = metadata) + val logs = mutableListOf() + + initForTest(mockContext) { options -> + options.isEnableTimberLogs = false + options.logs.beforeSend = + SentryOptions.Logs.BeforeSendLogCallback { log -> + logs.add(log) + log + } + } + Timber.i("message") + + assertTrue(logs.isEmpty()) + } + @Test fun `deduplicates fragment, timber and system events integrations`() { var refOptions: SentryAndroidOptions? = null diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt index 0c0c03d71d2..582f475f68d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt @@ -2,6 +2,7 @@ package io.sentry.android.core import android.os.Bundle import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.Breadcrumb import io.sentry.Sentry import io.sentry.SentryLevel @@ -13,8 +14,8 @@ import java.lang.RuntimeException import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertTrue import org.junit.runner.RunWith +import org.robolectric.shadows.ShadowLog @RunWith(AndroidJUnit4::class) class SentryLogcatAdapterTest { @@ -26,16 +27,21 @@ class SentryLogcatAdapterTest { val breadcrumbs = mutableListOf() val logs = mutableListOf() - fun initSut(options: Sentry.OptionsConfiguration? = null) { - val metadata = - Bundle().apply { putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123") } + fun initSut( + enableLogcatLogs: Boolean? = true, + metadata: Bundle = Bundle(), + options: Sentry.OptionsConfiguration? = null, + ) { + metadata.putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123") val mockContext = ContextUtilsTestHelper.mockMetaData(metaData = metadata) initForTest(mockContext) { it.beforeBreadcrumb = SentryOptions.BeforeBreadcrumbCallback { breadcrumb, _ -> breadcrumbs.add(breadcrumb) breadcrumb } - it.logs.isEnabled = true + if (enableLogcatLogs != null) { + it.isEnableLogcatLogs = enableLogcatLogs + } it.logs.beforeSend = SentryOptions.Logs.BeforeSendLogCallback { logEvent -> logs.add(logEvent) @@ -55,6 +61,37 @@ class SentryLogcatAdapterTest { Sentry.close() fixture.breadcrumbs.clear() fixture.logs.clear() + ShadowLog.clear() + } + + @Test + fun `Logcat logs are disabled by default while breadcrumbs and Android Log remain enabled`() { + fixture.initSut(enableLogcatLogs = null) + + SentryLogcatAdapter.d(tag, commonMsg) + + assertThat(fixture.logs).isEmpty() + assertThat(fixture.breadcrumbs).hasSize(1) + assertThat(ShadowLog.getLogs().any { it.tag == tag && it.msg == commonMsg }).isTrue() + } + + @Test + fun `Logcat logs can be enabled through Android options`() { + fixture.initSut(enableLogcatLogs = true) + + SentryLogcatAdapter.d(tag, commonMsg) + + assertThat(fixture.logs).hasSize(1) + } + + @Test + fun `Logcat logs can be enabled through manifest metadata`() { + val metadata = Bundle().apply { putBoolean(ManifestMetadataReader.ENABLE_LOGCAT_LOGS, true) } + fixture.initSut(enableLogcatLogs = null, metadata = metadata) + + SentryLogcatAdapter.d(tag, commonMsg) + + assertThat(fixture.logs).hasSize(1) } @Test @@ -166,26 +203,6 @@ class SentryLogcatAdapterTest { .assert("$commonMsg wtf exception\n${throwable.message}", SentryLogLevel.FATAL) } - @Test - fun `do not send logs if logs is disabled`() { - fixture.initSut { it.logs.isEnabled = false } - - SentryLogcatAdapter.v(tag, "$commonMsg verbose") - SentryLogcatAdapter.i(tag, "$commonMsg info") - SentryLogcatAdapter.d(tag, "$commonMsg debug") - SentryLogcatAdapter.w(tag, "$commonMsg warning") - SentryLogcatAdapter.e(tag, "$commonMsg error") - SentryLogcatAdapter.wtf(tag, "$commonMsg wtf") - SentryLogcatAdapter.e(tag, "$commonMsg error exception", throwable) - SentryLogcatAdapter.v(tag, "$commonMsg verbose exception", throwable) - SentryLogcatAdapter.i(tag, "$commonMsg info exception", throwable) - SentryLogcatAdapter.d(tag, "$commonMsg debug exception", throwable) - SentryLogcatAdapter.w(tag, "$commonMsg warning exception", throwable) - SentryLogcatAdapter.wtf(tag, "$commonMsg wtf exception", throwable) - - assertTrue(fixture.logs.isEmpty()) - } - @Test fun `logs add correct number of breadcrumb`() { fixture.initSut() diff --git a/sentry-android-timber/api/sentry-android-timber.api b/sentry-android-timber/api/sentry-android-timber.api index 8ae2f49c28d..ad5c909836e 100644 --- a/sentry-android-timber/api/sentry-android-timber.api +++ b/sentry-android-timber/api/sentry-android-timber.api @@ -11,7 +11,10 @@ public final class io/sentry/android/timber/SentryTimberIntegration : io/sentry/ public fun ()V public fun (Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;)V public synthetic fun (Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;Z)V + public fun (Z)V public fun close ()V + public final fun getEnableLogs ()Z public final fun getMinBreadcrumbLevel ()Lio/sentry/SentryLevel; public final fun getMinEventLevel ()Lio/sentry/SentryLevel; public final fun getMinLogsLevel ()Lio/sentry/SentryLogLevel; @@ -21,6 +24,7 @@ public final class io/sentry/android/timber/SentryTimberIntegration : io/sentry/ public final class io/sentry/android/timber/SentryTimberTree : timber/log/Timber$Tree { public fun (Lio/sentry/IScopes;Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;)V public synthetic fun (Lio/sentry/IScopes;Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lio/sentry/IScopes;Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;Z)V public fun d (Ljava/lang/String;[Ljava/lang/Object;)V public fun d (Ljava/lang/Throwable;)V public fun d (Ljava/lang/Throwable;Ljava/lang/String;[Ljava/lang/Object;)V diff --git a/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberIntegration.kt b/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberIntegration.kt index 521fe15127a..dabf3531fd1 100644 --- a/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberIntegration.kt +++ b/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberIntegration.kt @@ -9,6 +9,7 @@ import io.sentry.SentryLogLevel import io.sentry.SentryOptions import io.sentry.android.timber.BuildConfig.VERSION_NAME import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion +import io.sentry.util.LazyEvaluator.Evaluator import java.io.Closeable import timber.log.Timber @@ -18,6 +19,28 @@ public class SentryTimberIntegration( public val minBreadcrumbLevel: SentryLevel = SentryLevel.INFO, public val minLogsLevel: SentryLogLevel = SentryLogLevel.INFO, ) : Integration, Closeable { + public val enableLogs: Boolean + get() = enableLogsProvider.evaluate() + + private var enableLogsProvider: Evaluator = Evaluator { false } + + public constructor(enableLogs: Boolean) : this() { + enableLogsProvider = Evaluator { enableLogs } + } + + public constructor( + minEventLevel: SentryLevel, + minBreadcrumbLevel: SentryLevel, + minLogsLevel: SentryLogLevel, + enableLogs: Boolean, + ) : this(minEventLevel, minBreadcrumbLevel, minLogsLevel) { + enableLogsProvider = Evaluator { enableLogs } + } + + internal constructor(enableLogsProvider: Evaluator) : this() { + this.enableLogsProvider = enableLogsProvider + } + private lateinit var tree: SentryTimberTree private lateinit var logger: ILogger @@ -31,7 +54,14 @@ public class SentryTimberIntegration( override fun register(scopes: IScopes, options: SentryOptions) { logger = options.logger - tree = SentryTimberTree(scopes, minEventLevel, minBreadcrumbLevel, minLogsLevel) + tree = + SentryTimberTree( + scopes, + minEventLevel, + minBreadcrumbLevel, + minLogsLevel, + enableLogsProvider.evaluate(), + ) Timber.plant(tree) logger.log(SentryLevel.DEBUG, "SentryTimberIntegration installed.") diff --git a/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberTree.kt b/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberTree.kt index 61b1f99fb16..f63dd7b66d0 100644 --- a/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberTree.kt +++ b/sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberTree.kt @@ -20,6 +20,18 @@ public class SentryTimberTree( private val minBreadcrumbLevel: SentryLevel, private val minLogLevel: SentryLogLevel = SentryLogLevel.INFO, ) : Timber.Tree() { + private var enableLogs: Boolean = false + + public constructor( + scopes: IScopes, + minEventLevel: SentryLevel, + minBreadcrumbLevel: SentryLevel, + minLogLevel: SentryLogLevel, + enableLogs: Boolean, + ) : this(scopes, minEventLevel, minBreadcrumbLevel, minLogLevel) { + this.enableLogs = enableLogs + } + private val pendingTag = ThreadLocal() private fun retrieveTag(): String? { @@ -185,7 +197,9 @@ public class SentryTimberTree( captureEvent(level, tag, sentryMessage, throwable) addBreadcrumb(level, sentryMessage, throwable) - addLog(logLevel, message, tag, throwable, *args) + if (enableLogs) { + addLog(logLevel, message, tag, throwable, *args) + } } /** do not log if it's lower than min. required level. */ diff --git a/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt b/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt index 7c21eca8ef0..6597d528f87 100644 --- a/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt +++ b/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt @@ -1,5 +1,6 @@ package io.sentry.android.timber +import io.sentry.Breadcrumb import io.sentry.IScopes import io.sentry.ITransportFactory import io.sentry.ScopesAdapter @@ -7,33 +8,53 @@ import io.sentry.Sentry import io.sentry.SentryLevel import io.sentry.SentryLogLevel import io.sentry.SentryOptions +import io.sentry.logger.ILoggerApi +import io.sentry.logger.SentryLogParameters import io.sentry.protocol.SdkVersion import io.sentry.transport.ITransport +import io.sentry.util.LazyEvaluator.Evaluator import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertTrue import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever import timber.log.Timber class SentryTimberIntegrationTest { private class Fixture { val scopes = mock() + val logs = mock() val options = SentryOptions().apply { sdkVersion = SdkVersion("test", "1.2.3") } + init { + whenever(scopes.logger()).thenReturn(logs) + } + fun getSut( minEventLevel: SentryLevel = SentryLevel.ERROR, minBreadcrumbLevel: SentryLevel = SentryLevel.INFO, minLogsLevel: SentryLogLevel = SentryLogLevel.INFO, + enableLogs: Boolean? = null, ): SentryTimberIntegration = - SentryTimberIntegration( - minEventLevel = minEventLevel, - minBreadcrumbLevel = minBreadcrumbLevel, - minLogsLevel = minLogsLevel, - ) + if (enableLogs == null) { + SentryTimberIntegration( + minEventLevel = minEventLevel, + minBreadcrumbLevel = minBreadcrumbLevel, + minLogsLevel = minLogsLevel, + ) + } else { + SentryTimberIntegration( + minEventLevel = minEventLevel, + minBreadcrumbLevel = minBreadcrumbLevel, + minLogsLevel = minLogsLevel, + enableLogs = enableLogs, + ) + } } private val fixture = Fixture() @@ -64,6 +85,42 @@ class SentryTimberIntegrationTest { verify(fixture.scopes).captureEvent(any()) } + @Test + fun `Manual integration defaults logs to disabled while capturing events and breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + assertFalse(sut.enableLogs) + Timber.e("message") + + verify(fixture.scopes).captureEvent(any()) + verify(fixture.scopes).addBreadcrumb(any()) + verifyNoInteractions(fixture.logs) + } + + @Test + fun `Manual integration captures logs when enabled`() { + val sut = fixture.getSut(enableLogs = true) + sut.register(fixture.scopes, fixture.options) + + assertTrue(sut.enableLogs) + Timber.i("message") + + verify(fixture.logs).log(any(), any(), any()) + } + + @Test + fun `Integration evaluates Logs provider when registered`() { + var enableLogs = false + val sut = SentryTimberIntegration(Evaluator { enableLogs }) + enableLogs = true + + sut.register(fixture.scopes, fixture.options) + Timber.i("message") + + verify(fixture.logs).log(any(), any(), any()) + } + @Test fun `Integrations removes a tree from Timber on close integration`() { val sut = fixture.getSut() diff --git a/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberTreeTest.kt b/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberTreeTest.kt index f1d6d5a51bd..457ab31d33c 100644 --- a/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberTreeTest.kt +++ b/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberTreeTest.kt @@ -31,11 +31,16 @@ class SentryTimberTreeTest { minEventLevel: SentryLevel = SentryLevel.ERROR, minBreadcrumbLevel: SentryLevel = SentryLevel.INFO, minLogsLevel: SentryLogLevel = SentryLogLevel.INFO, + enableLogs: Boolean? = true, ): SentryTimberTree { logs = mock() scopes = mock() whenever(scopes.logger()).thenReturn(logs) - return SentryTimberTree(scopes, minEventLevel, minBreadcrumbLevel, minLogsLevel) + return if (enableLogs == null) { + SentryTimberTree(scopes, minEventLevel, minBreadcrumbLevel, minLogsLevel) + } else { + SentryTimberTree(scopes, minEventLevel, minBreadcrumbLevel, minLogsLevel, enableLogs) + } } } @@ -296,6 +301,17 @@ class SentryTimberTreeTest { sut.d("test %s, %s", 1, 1) } + @Test + fun `Tree defaults logs to disabled while capturing events and breadcrumbs`() { + val sut = fixture.getSut(enableLogs = null) + + sut.e("message") + + verify(fixture.scopes).captureEvent(any()) + verify(fixture.scopes).addBreadcrumb(any()) + verifyNoInteractions(fixture.logs) + } + @Test fun `Tree adds a log with message and arguments, when provided`() { val sut = fixture.getSut() diff --git a/sentry-jul/api/sentry-jul.api b/sentry-jul/api/sentry-jul.api index 7a7f6a26d32..1fdf4895971 100644 --- a/sentry-jul/api/sentry-jul.api +++ b/sentry-jul/api/sentry-jul.api @@ -15,8 +15,10 @@ public class io/sentry/jul/SentryHandler : java/util/logging/Handler { public fun getMinimumBreadcrumbLevel ()Ljava/util/logging/Level; public fun getMinimumEventLevel ()Ljava/util/logging/Level; public fun getMinimumLevel ()Ljava/util/logging/Level; + public fun isEnableLogs ()Z public fun isPrintfStyle ()Z public fun publish (Ljava/util/logging/LogRecord;)V + public fun setEnableLogs (Z)V public fun setMinimumBreadcrumbLevel (Ljava/util/logging/Level;)V public fun setMinimumEventLevel (Ljava/util/logging/Level;)V public fun setMinimumLevel (Ljava/util/logging/Level;)V 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 4442052dd41..685402e27bd 100644 --- a/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java +++ b/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java @@ -28,6 +28,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.ResourceBundle; import java.util.logging.ErrorManager; import java.util.logging.Filter; import java.util.logging.Handler; @@ -53,6 +54,8 @@ public class SentryHandler extends Handler { */ private boolean printfStyle; + private boolean enableLogs; + private @NotNull Level minimumBreadcrumbLevel = Level.INFO; private @NotNull Level minimumEventLevel = Level.SEVERE; private @NotNull Level minimumLevel = Level.INFO; @@ -112,8 +115,7 @@ public void publish(final @NotNull LogRecord record) { return; } try { - if (ScopesAdapter.getInstance().getOptions().getLogs().isEnabled() - && record.getLevel().intValue() >= minimumLevel.intValue()) { + if (enableLogs && record.getLevel().intValue() >= minimumLevel.intValue()) { captureLog(record); } if (record.getLevel().intValue() >= minimumEventLevel.intValue()) { @@ -149,10 +151,15 @@ protected void captureLog(@NotNull LogRecord loggingEvent) { final @Nullable Object[] arguments = loggingEvent.getParameters(); final @NotNull SentryAttributes attributes = SentryAttributes.of(); - @NotNull String message = loggingEvent.getMessage(); + final @Nullable String messageTemplate = loggingEvent.getMessage(); + if (messageTemplate == null) { + return; + } + + @NotNull String message = messageTemplate; if (loggingEvent.getResourceBundle() != null - && loggingEvent.getResourceBundle().containsKey(loggingEvent.getMessage())) { - message = loggingEvent.getResourceBundle().getString(loggingEvent.getMessage()); + && loggingEvent.getResourceBundle().containsKey(messageTemplate)) { + message = loggingEvent.getResourceBundle().getString(messageTemplate); } final @NotNull String formattedMessage = maybeFormatted(arguments, message); @@ -191,6 +198,7 @@ private void retrieveProperties() { final LogManager manager = LogManager.getLogManager(); final String className = SentryHandler.class.getName(); setPrintfStyle(Boolean.parseBoolean(manager.getProperty(className + ".printfStyle"))); + setEnableLogs(Boolean.parseBoolean(manager.getProperty(className + ".enableLogs"))); setLevel(parseLevelOrDefault(manager.getProperty(className + ".level"))); final String minimumBreadCrumbLevel = manager.getProperty(className + ".minimumBreadcrumbLevel"); @@ -288,13 +296,16 @@ SentryEvent createEvent(final @NotNull LogRecord record) { final Message sentryMessage = new Message(); sentryMessage.setParams(toParams(record.getParameters())); - String message = record.getMessage(); - if (record.getResourceBundle() != null - && record.getResourceBundle().containsKey(record.getMessage())) { - message = record.getResourceBundle().getString(record.getMessage()); + final @Nullable String messageTemplate = record.getMessage(); + final @Nullable ResourceBundle resourceBundle = record.getResourceBundle(); + @Nullable String message = messageTemplate; + if (messageTemplate != null + && resourceBundle != null + && resourceBundle.containsKey(messageTemplate)) { + message = resourceBundle.getString(messageTemplate); } sentryMessage.setMessage(message); - if (record.getParameters() != null) { + if (message != null && record.getParameters() != null) { try { sentryMessage.setFormatted(formatMessage(message, record.getParameters())); } catch (RuntimeException e) { @@ -390,6 +401,14 @@ public void setPrintfStyle(final boolean printfStyle) { this.printfStyle = printfStyle; } + public void setEnableLogs(final boolean enableLogs) { + this.enableLogs = enableLogs; + } + + public boolean isEnableLogs() { + return enableLogs; + } + public void setMinimumBreadcrumbLevel(final @Nullable Level minimumBreadcrumbLevel) { if (minimumBreadcrumbLevel != null) { this.minimumBreadcrumbLevel = minimumBreadcrumbLevel; 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 ab160faac40..028c6759727 100644 --- a/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt +++ b/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt @@ -16,7 +16,9 @@ import io.sentry.transport.ITransport import java.time.Instant import java.time.LocalDateTime import java.time.ZoneId +import java.util.ListResourceBundle import java.util.logging.Level +import java.util.logging.LogRecord import java.util.logging.Logger import kotlin.test.AfterTest import kotlin.test.BeforeTest @@ -28,6 +30,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.slf4j.MDC @@ -40,6 +43,7 @@ class SentryHandlerTest { val transport: ITransport = mock(), contextTags: List? = null, printfStyle: Boolean? = null, + enableLogs: Boolean? = true, ) { var logger: Logger var handler: SentryHandler @@ -58,6 +62,9 @@ class SentryHandlerTest { handler.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel) handler.setMinimumEventLevel(minimumEventLevel) handler.setMinimumLevel(minimumLevel) + if (enableLogs != null) { + handler.setEnableLogs(enableLogs) + } if (printfStyle == true) { handler.setPrintfStyle(printfStyle) } @@ -318,11 +325,22 @@ class SentryHandlerTest { @Test fun `fetches configuration from logging dot properties`() { - fixture = Fixture(configureWithLogManager = true) + fixture = Fixture(configureWithLogManager = true, enableLogs = null) assertEquals(Level.CONFIG, fixture.handler.minimumBreadcrumbLevel) assertEquals(Level.WARNING, fixture.handler.minimumEventLevel) assertEquals(Level.ALL, fixture.handler.level) assertTrue(fixture.handler.isPrintfStyle) + assertTrue(fixture.handler.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 @@ -418,6 +436,116 @@ class SentryHandlerTest { ) } + @Test + fun `does not capture logs by default`() { + fixture = Fixture(enableLogs = null) + + assertFalse(fixture.handler.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 enabled through Java`() { + fixture = Fixture(enableLogs = true) + + assertTrue(fixture.handler.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 logs are disabled`() { + fixture = + Fixture( + minimumBreadcrumbLevel = Level.INFO, + minimumEventLevel = Level.SEVERE, + enableLogs = false, + ) + + fixture.logger.info("this should be a breadcrumb") + fixture.logger.severe("this should be an event") + Sentry.flush(10) + + verify(fixture.transport) + .send( + checkEvent { event -> + assertEquals("this should be an event", event.message?.message) + assertEquals("this should be a breadcrumb", event.breadcrumbs?.single()?.message) + }, + anyOrNull(), + ) + verify(fixture.transport, never()).send(checkLogs {}) + } + + @Test + fun `captures null message as event and breadcrumb when logs are enabled`() { + fixture = + Fixture( + minimumBreadcrumbLevel = Level.INFO, + minimumEventLevel = Level.SEVERE, + enableLogs = true, + ) + + fixture.logger.info(null as String?) + fixture.logger.severe(null as String?) + Sentry.flush(10) + + verify(fixture.transport) + .send( + checkEvent { event -> + assertNull(event.message?.message) + assertEquals(1, event.breadcrumbs?.size) + assertNull(event.breadcrumbs?.single()?.message) + }, + anyOrNull(), + ) + verify(fixture.transport, never()).send(checkLogs {}) + } + + @Test + fun `captures null message as event and breadcrumb when resource bundle is set`() { + fixture = + Fixture( + minimumBreadcrumbLevel = Level.INFO, + minimumEventLevel = Level.SEVERE, + enableLogs = true, + ) + val resourceBundle = + object : ListResourceBundle() { + override fun getContents(): Array> = + arrayOf(arrayOf("message", "localized message")) + } + + fixture.handler.publish( + LogRecord(Level.INFO, null).apply { this.resourceBundle = resourceBundle } + ) + fixture.handler.publish( + LogRecord(Level.SEVERE, null).apply { this.resourceBundle = resourceBundle } + ) + Sentry.flush(10) + + verify(fixture.transport) + .send( + checkEvent { event -> + assertNull(event.message?.message) + assertEquals(1, event.breadcrumbs?.size) + assertNull(event.breadcrumbs?.single()?.message) + }, + anyOrNull(), + ) + verify(fixture.transport, never()).send(checkLogs {}) + } + @Test fun `converts finest log level to Sentry log level`() { fixture = Fixture(minimumLevel = Level.FINEST) diff --git a/sentry-jul/src/test/resources/logging.properties b/sentry-jul/src/test/resources/logging.properties index 9ac994b722a..25ac65e1f66 100644 --- a/sentry-jul/src/test/resources/logging.properties +++ b/sentry-jul/src/test/resources/logging.properties @@ -3,5 +3,6 @@ io.sentry.jul.SentryHandler.minimumEventLevel=WARNING io.sentry.jul.SentryHandler.minimumBreadcrumbLevel=CONFIG io.sentry.jul.SentryHandler.minimumLevel=CONFIG io.sentry.jul.SentryHandler.printfStyle=true +io.sentry.jul.SentryHandler.enableLogs=true jul.SentryHandlerTest.handlers=java.util.logging.ConsoleHandler, io.sentry.jul.SentryHandler diff --git a/sentry-jul/src/test/resources/sentry.properties b/sentry-jul/src/test/resources/sentry.properties index 0163b4f2f84..12c5db4eb9d 100644 --- a/sentry-jul/src/test/resources/sentry.properties +++ b/sentry-jul/src/test/resources/sentry.properties @@ -1,2 +1 @@ release=release from sentry.properties -logs.enabled=true diff --git a/sentry-log4j2/api/sentry-log4j2.api b/sentry-log4j2/api/sentry-log4j2.api index 7eebea7136e..2afe9f1855c 100644 --- a/sentry-log4j2/api/sentry-log4j2.api +++ b/sentry-log4j2/api/sentry-log4j2.api @@ -7,8 +7,10 @@ public class io/sentry/log4j2/SentryAppender : org/apache/logging/log4j/core/app public static final field MECHANISM_TYPE Ljava/lang/String; public fun (Ljava/lang/String;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/Boolean;Lio/sentry/ITransportFactory;Lio/sentry/IScopes;[Ljava/lang/String;)V public fun (Ljava/lang/String;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/Boolean;Lio/sentry/ITransportFactory;Lio/sentry/IScopes;[Ljava/lang/String;)V + public fun (Ljava/lang/String;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;ZLjava/lang/Boolean;Lio/sentry/ITransportFactory;Lio/sentry/IScopes;[Ljava/lang/String;)V public fun append (Lorg/apache/logging/log4j/core/LogEvent;)V protected fun captureLog (Lorg/apache/logging/log4j/core/LogEvent;)V + public static fun createAppender (Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/Boolean;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;)Lio/sentry/log4j2/SentryAppender; public static fun createAppender (Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/String;Ljava/lang/Boolean;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;)Lio/sentry/log4j2/SentryAppender; protected fun createBreadcrumb (Lorg/apache/logging/log4j/core/LogEvent;)Lio/sentry/Breadcrumb; protected fun createEvent (Lorg/apache/logging/log4j/core/LogEvent;)Lio/sentry/SentryEvent; 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 c965784f00f..ab1d59a7925 100644 --- a/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java +++ b/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java @@ -55,6 +55,7 @@ public class SentryAppender extends AbstractAppender { private @NotNull Level minimumBreadcrumbLevel = Level.INFO; private @NotNull Level minimumEventLevel = Level.ERROR; private @NotNull Level minimumLevel = Level.INFO; + private final boolean enableLogs; private final @Nullable Boolean debug; private final @NotNull IScopes scopes; private final @Nullable List contextTags; @@ -104,6 +105,32 @@ public SentryAppender( final @Nullable ITransportFactory transportFactory, final @NotNull IScopes scopes, final @Nullable String[] contextTags) { + this( + name, + filter, + dsn, + minimumBreadcrumbLevel, + minimumEventLevel, + minimumLevel, + false, + debug, + transportFactory, + scopes, + contextTags); + } + + public SentryAppender( + final @NotNull String name, + final @Nullable Filter filter, + final @Nullable String dsn, + final @Nullable Level minimumBreadcrumbLevel, + final @Nullable Level minimumEventLevel, + final @Nullable Level minimumLevel, + final boolean enableLogs, + final @Nullable Boolean debug, + final @Nullable ITransportFactory transportFactory, + final @NotNull IScopes scopes, + final @Nullable String[] contextTags) { super(name, filter, null, true, null); this.dsn = dsn; if (minimumBreadcrumbLevel != null) { @@ -115,6 +142,7 @@ public SentryAppender( if (minimumLevel != null) { this.minimumLevel = minimumLevel; } + this.enableLogs = enableLogs; this.debug = debug; this.transportFactory = transportFactory; this.scopes = scopes; @@ -133,12 +161,34 @@ public SentryAppender( * @param filter The filter, if any, to use. * @return The SentryAppender. */ + public static @Nullable SentryAppender createAppender( + final @Nullable String name, + final @Nullable Level minimumBreadcrumbLevel, + final @Nullable Level minimumEventLevel, + final @Nullable Level minimumLevel, + final @Nullable String dsn, + final @Nullable Boolean debug, + final @Nullable Filter filter, + final @Nullable String contextTags) { + return createAppender( + name, + minimumBreadcrumbLevel, + minimumEventLevel, + minimumLevel, + false, + dsn, + debug, + filter, + contextTags); + } + @PluginFactory public static @Nullable SentryAppender createAppender( @Nullable @PluginAttribute("name") final String name, @Nullable @PluginAttribute("minimumBreadcrumbLevel") final Level minimumBreadcrumbLevel, @Nullable @PluginAttribute("minimumEventLevel") final Level minimumEventLevel, @Nullable @PluginAttribute("minimumLevel") final Level minimumLevel, + @Nullable @PluginAttribute("enableLogs") final Boolean enableLogs, @Nullable @PluginAttribute("dsn") final String dsn, @Nullable @PluginAttribute("debug") final Boolean debug, @Nullable @PluginElement("filter") final Filter filter, @@ -155,6 +205,7 @@ public SentryAppender( minimumBreadcrumbLevel, minimumEventLevel, minimumLevel, + Boolean.TRUE.equals(enableLogs), debug, null, ScopesAdapter.getInstance(), @@ -218,8 +269,7 @@ void start(final @NotNull Sentry.OptionsConfiguration optionsConf @Override public void append(final @NotNull LogEvent eventObject) { - if (scopes.getOptions().getLogs().isEnabled() - && eventObject.getLevel().isMoreSpecificThan(minimumLevel)) { + if (enableLogs && eventObject.getLevel().isMoreSpecificThan(minimumLevel)) { captureLog(eventObject); } if (eventObject.getLevel().isMoreSpecificThan(minimumEventLevel)) { 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 c32459ea022..fe5d3f4112c 100644 --- a/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt +++ b/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt @@ -1,5 +1,6 @@ package io.sentry.log4j2 +import io.sentry.IScopes import io.sentry.ITransportFactory import io.sentry.InitPriority import io.sentry.ScopesAdapter @@ -28,6 +29,7 @@ import org.apache.logging.log4j.Level import org.apache.logging.log4j.LogManager import org.apache.logging.log4j.MarkerManager import org.apache.logging.log4j.ThreadContext +import org.apache.logging.log4j.core.LogEvent import org.apache.logging.log4j.core.LoggerContext import org.apache.logging.log4j.core.config.AppenderRef import org.apache.logging.log4j.core.config.Configuration @@ -35,7 +37,10 @@ import org.apache.logging.log4j.core.config.LoggerConfig import org.apache.logging.log4j.spi.ExtendedLogger import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doNothing import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -57,6 +62,7 @@ class SentryAppenderTest { minimumLevel: Level? = null, debug: Boolean? = null, contextTags: List? = null, + enableLogs: Boolean = true, ): ExtendedLogger { if (transportFactory != null) { this.transportFactory = transportFactory @@ -71,6 +77,7 @@ class SentryAppenderTest { minimumBreadcrumbLevel, minimumEventLevel, minimumLevel, + enableLogs, debug, this.transportFactory, ScopesAdapter.getInstance(), @@ -253,6 +260,166 @@ class SentryAppenderTest { .send(checkEvent { event -> assertEquals(SentryLevel.FATAL, event.level) }, anyOrNull()) } + @Test + fun `does not capture logs when local logs are disabled`() { + val logger = fixture.getSut(enableLogs = false) + + 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 logs are enabled`() { + val logger = fixture.getSut(enableLogs = true) + + 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`() { + val logger = + fixture.getSut( + minimumBreadcrumbLevel = Level.INFO, + minimumEventLevel = Level.ERROR, + enableLogs = false, + ) + + logger.info("this should be a breadcrumb") + 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 `existing constructors default logs to disabled`() { + val scopes = mock() + val event = mock() + whenever(event.level).thenReturn(Level.INFO) + + val deprecatedAppender = + SentryAppender( + "deprecated", + null, + null, + Level.OFF, + Level.OFF, + null, + null, + scopes, + null, + ) + val existingAppender = + SentryAppender( + "existing", + null, + null, + Level.OFF, + Level.OFF, + Level.INFO, + null, + null, + scopes, + null, + ) + + deprecatedAppender.append(event) + existingAppender.append(event) + + verify(scopes, never()).logger() + } + + @Test + fun `existing factory and plugin attribute default logs to disabled`() { + val event = mock() + whenever(event.level).thenReturn(Level.INFO) + val existingAppender = + spy( + assertNotNull( + SentryAppender.createAppender( + "existing", + Level.OFF, + Level.OFF, + Level.INFO, + null, + null, + null, + null, + ) + ) + ) + val pluginDefaultAppender = + spy( + assertNotNull( + SentryAppender.createAppender( + "plugin-default", + Level.OFF, + Level.OFF, + Level.INFO, + null, + null, + null, + null, + null, + ) + ) + ) + + existingAppender.append(event) + pluginDefaultAppender.append(event) + + verify(existingAppender, never()).captureLog(event) + verify(pluginDefaultAppender, never()).captureLog(event) + } + + @Test + fun `plugin attribute enables logs with explicit opt in`() { + initForTest { + it.dsn = "http://key@localhost/proj" + } + val event = mock() + whenever(event.level).thenReturn(Level.INFO) + val appender = + spy( + assertNotNull( + SentryAppender.createAppender( + "enabled", + Level.OFF, + Level.OFF, + Level.INFO, + true, + null, + null, + null, + null, + ) + ) + ) + doNothing().whenever(appender).captureLog(event) + + appender.append(event) + + verify(appender).captureLog(event) + } + @Test fun `converts trace log level to Sentry log level`() { val logger = fixture.getSut(minimumLevel = Level.TRACE) diff --git a/sentry-log4j2/src/test/resources/sentry.properties b/sentry-log4j2/src/test/resources/sentry.properties index 9845650aace..ec87ba75304 100644 --- a/sentry-log4j2/src/test/resources/sentry.properties +++ b/sentry-log4j2/src/test/resources/sentry.properties @@ -1,4 +1,3 @@ release=release from sentry.properties -logs.enabled=true shutdown-timeout-millis=0 session-flush-timeout-millis=0 diff --git a/sentry-logback/api/sentry-logback.api b/sentry-logback/api/sentry-logback.api index c527172116f..4dc2065739e 100644 --- a/sentry-logback/api/sentry-logback.api +++ b/sentry-logback/api/sentry-logback.api @@ -15,6 +15,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 setIncludeUnencodedMessage (Z)V public fun setMinimumBreadcrumbLevel (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 722845e2d70..81335ad02db 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; private boolean includeUnencodedMessage = false; @@ -88,8 +89,7 @@ public void start() { @Override protected void append(@NotNull ILoggingEvent eventObject) { - if (ScopesAdapter.getInstance().getOptions().getLogs().isEnabled() - && eventObject.getLevel().isGreaterOrEqual(minimumLevel)) { + if (enableLogs && eventObject.getLevel().isGreaterOrEqual(minimumLevel)) { captureLog(eventObject); } if (eventObject.getLevel().isGreaterOrEqual(minimumEventLevel)) { @@ -333,6 +333,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 bb10c5ede9c..2bf05b12404 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -39,6 +39,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 @@ -73,7 +74,6 @@ class SentryAppenderTest { this.encoder = encoder options.dsn = dsn options.isSendDefaultPii = sendDefaultPii - options.logs.isEnabled = enableLogs options.logs.loggerBatchProcessorFactory = ILoggerBatchProcessorFactory { options, client -> LoggerBatchProcessor(options, client, ImmediateExecutorService()) } @@ -83,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 @@ -410,6 +411,57 @@ class SentryAppenderTest { ) } + @Test + fun `does not capture logs by default`() { + fixture = Fixture(enableLogs = false) + + 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 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, + enableLogs = false, + ) + + 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-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index 6426d03b814..965e74af57a 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -147,9 +147,14 @@ android:name="io.sentry.debug" android:value="${sentryDebug}" /> - + + + + diff --git a/sentry-samples/sentry-samples-console-otlp/src/main/java/io/sentry/samples/console/Main.java b/sentry-samples/sentry-samples-console-otlp/src/main/java/io/sentry/samples/console/Main.java index d973a68a907..3a21e8220c6 100644 --- a/sentry-samples/sentry-samples-console-otlp/src/main/java/io/sentry/samples/console/Main.java +++ b/sentry-samples/sentry-samples-console-otlp/src/main/java/io/sentry/samples/console/Main.java @@ -133,7 +133,6 @@ public static void main(String[] args) throws InterruptedException { // } // }); - options.getLogs().setEnabled(true); }); Sentry.addBreadcrumb( diff --git a/sentry-samples/sentry-samples-jul/src/main/resources/logging.properties b/sentry-samples/sentry-samples-jul/src/main/resources/logging.properties index 0bdab173235..db3026bfb9f 100644 --- a/sentry-samples/sentry-samples-jul/src/main/resources/logging.properties +++ b/sentry-samples/sentry-samples-jul/src/main/resources/logging.properties @@ -2,6 +2,7 @@ io.sentry.jul.SentryHandler.minimumEventLevel=INFO io.sentry.jul.SentryHandler.minimumBreadcrumbLevel=CONFIG io.sentry.jul.SentryHandler.minimumLevel=INFO io.sentry.jul.SentryHandler.printfStyle=true +io.sentry.jul.SentryHandler.enableLogs=true io.sentry.jul.SentryHandler.level=FINEST java.util.logging.ConsoleHandler.level = FINE handlers=io.sentry.jul.SentryHandler diff --git a/sentry-samples/sentry-samples-jul/src/main/resources/sentry.properties b/sentry-samples/sentry-samples-jul/src/main/resources/sentry.properties index ac73ce04179..390771a4403 100644 --- a/sentry-samples/sentry-samples-jul/src/main/resources/sentry.properties +++ b/sentry-samples/sentry-samples-jul/src/main/resources/sentry.properties @@ -4,4 +4,3 @@ debug=true environment=staging in-app-includes=io.sentry.samples context-tags=userId,requestId -logs.enabled=true diff --git a/sentry-samples/sentry-samples-log4j2/src/main/resources/log4j2.xml b/sentry-samples/sentry-samples-log4j2/src/main/resources/log4j2.xml index 51428b0f1cc..028e449b06a 100644 --- a/sentry-samples/sentry-samples-log4j2/src/main/resources/log4j2.xml +++ b/sentry-samples/sentry-samples-log4j2/src/main/resources/log4j2.xml @@ -12,6 +12,7 @@ minimumBreadcrumbLevel="DEBUG" minimumEventLevel="WARN" minimumLevel="DEBUG" + enableLogs="true" debug="true" contextTags="userId,requestId" /> diff --git a/sentry-samples/sentry-samples-log4j2/src/main/resources/sentry.properties b/sentry-samples/sentry-samples-log4j2/src/main/resources/sentry.properties index b2310e08f89..a7dca6edc4e 100644 --- a/sentry-samples/sentry-samples-log4j2/src/main/resources/sentry.properties +++ b/sentry-samples/sentry-samples-log4j2/src/main/resources/sentry.properties @@ -1,3 +1,2 @@ in-app-includes="io.sentry.samples" -logs.enabled=true debug=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..196486cf807 100644 --- a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml +++ b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml @@ -13,16 +13,14 @@ https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563 userId requestId - - true - + true WARN DEBUG - + INFO diff --git a/sentry-samples/sentry-samples-spring-boot-4-opentelemetry-noagent/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-4-opentelemetry-noagent/src/main/resources/application.properties index d19c33a3d1b..d8b1bcd2bb6 100644 --- a/sentry-samples/sentry-samples-spring-boot-4-opentelemetry-noagent/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-4-opentelemetry-noagent/src/main/resources/application.properties @@ -15,7 +15,7 @@ sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true sentry.enablePrettySerializationOutput=false -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-4-opentelemetry/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-4-opentelemetry/src/main/resources/application.properties index a0808e04fde..bf302c6dd05 100644 --- a/sentry-samples/sentry-samples-spring-boot-4-opentelemetry/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-4-opentelemetry/src/main/resources/application.properties @@ -15,7 +15,7 @@ sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true sentry.enablePrettySerializationOutput=false -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-4-otlp/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-4-otlp/src/main/resources/application.properties index f9b35099062..05a35327d86 100644 --- a/sentry-samples/sentry-samples-spring-boot-4-otlp/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-4-otlp/src/main/resources/application.properties @@ -16,7 +16,7 @@ sentry.enable-backpressure-handling=true sentry.enable-spotlight=true sentry.enablePrettySerializationOutput=false sentry.in-app-includes="io.sentry.samples" -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces sentry.profile-lifecycle=TRACE diff --git a/sentry-samples/sentry-samples-spring-boot-4-webflux/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-4-webflux/src/main/resources/application.properties index 9fc969efd28..2e897e5c714 100644 --- a/sentry-samples/sentry-samples-spring-boot-4-webflux/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-4-webflux/src/main/resources/application.properties @@ -10,7 +10,7 @@ sentry.logging.minimum-breadcrumb-level=debug sentry.reactive.thread-local-accessor-enabled=true sentry.traces-sample-rate=1.0 sentry.enable-backpressure-handling=true -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.enable-spotlight=true sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-4/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-4/src/main/resources/application.properties index 8198059343a..40a5843c134 100644 --- a/sentry-samples/sentry-samples-spring-boot-4/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-4/src/main/resources/application.properties @@ -16,7 +16,7 @@ sentry.enable-backpressure-handling=true sentry.enable-spotlight=true sentry.enablePrettySerializationOutput=false sentry.in-app-includes="io.sentry.samples" -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces sentry.profile-lifecycle=TRACE diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry-noagent/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry-noagent/src/main/resources/application.properties index a3a59d290b1..7f5880b741a 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry-noagent/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry-noagent/src/main/resources/application.properties @@ -15,7 +15,7 @@ sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true sentry.enablePrettySerializationOutput=false -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry/src/main/resources/application.properties index 12a9ca17269..4b80755d846 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-jakarta-opentelemetry/src/main/resources/application.properties @@ -15,7 +15,7 @@ sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true sentry.enablePrettySerializationOutput=false -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties index 20f9463aabc..d71c2c433ab 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties @@ -16,7 +16,7 @@ sentry.enable-backpressure-handling=true sentry.enable-spotlight=false sentry.enablePrettySerializationOutput=false sentry.in-app-includes="io.sentry.samples" -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces sentry.profile-lifecycle=TRACE diff --git a/sentry-samples/sentry-samples-spring-boot-opentelemetry-noagent/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-opentelemetry-noagent/src/main/resources/application.properties index 2225cd5045c..af217277c78 100644 --- a/sentry-samples/sentry-samples-spring-boot-opentelemetry-noagent/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-opentelemetry-noagent/src/main/resources/application.properties @@ -14,7 +14,7 @@ sentry.debug=true sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-opentelemetry/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-opentelemetry/src/main/resources/application.properties index d39f38d7182..404549c12da 100644 --- a/sentry-samples/sentry-samples-spring-boot-opentelemetry/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-opentelemetry/src/main/resources/application.properties @@ -14,7 +14,7 @@ sentry.debug=true sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties index 02eaf0c731c..45d04440f88 100644 --- a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties @@ -10,7 +10,7 @@ sentry.logging.minimum-breadcrumb-level=debug sentry.reactive.thread-local-accessor-enabled=true sentry.traces-sample-rate=1.0 sentry.enable-backpressure-handling=true -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.enable-spotlight=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 diff --git a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties index 6544e24f13b..5e85915a9c9 100644 --- a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties @@ -12,7 +12,7 @@ spring.graphql.graphiql.enabled=true spring.graphql.websocket.path=/graphql spring.graphql.schema.printer.enabled=true sentry.enable-backpressure-handling=true -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.enable-spotlight=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties index 4e97e7a1eb8..bce0ce41f53 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties @@ -14,7 +14,7 @@ sentry.debug=true sentry.graphql.ignored-error-types=SOME_ERROR,ANOTHER_ERROR sentry.enable-backpressure-handling=true sentry.enable-spotlight=true -sentry.logs.enabled=true +sentry.logging.enable-logs=true sentry.in-app-includes="io.sentry.samples" sentry.profile-session-sample-rate=1.0 sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces diff --git a/sentry-spring-boot-4/api/sentry-spring-boot-4.api b/sentry-spring-boot-4/api/sentry-spring-boot-4.api index f3a16d45f57..7ea571b2d13 100644 --- a/sentry-spring-boot-4/api/sentry-spring-boot-4.api +++ b/sentry-spring-boot-4/api/sentry-spring-boot-4.api @@ -71,7 +71,9 @@ public class io/sentry/spring/boot4/SentryProperties$Logging { public fun getMinimumBreadcrumbLevel ()Lorg/slf4j/event/Level; public fun getMinimumEventLevel ()Lorg/slf4j/event/Level; public fun getMinimumLevel ()Lorg/slf4j/event/Level; + public fun isEnableLogs ()Z public fun isEnabled ()Z + public fun setEnableLogs (Z)V public fun setEnabled (Z)V public fun setLoggers (Ljava/util/List;)V public fun setMinimumBreadcrumbLevel (Lorg/slf4j/event/Level;)V diff --git a/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLog4j2Initializer.java b/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLog4j2Initializer.java index efdf9dc255a..fe2d8a301a8 100644 --- a/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLog4j2Initializer.java +++ b/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLog4j2Initializer.java @@ -95,6 +95,7 @@ public void onApplicationEvent(final @NotNull ApplicationEvent event) { toLog4jLevel(sentryProperties.getLogging().getMinimumBreadcrumbLevel()), toLog4jLevel(sentryProperties.getLogging().getMinimumEventLevel()), toLog4jLevel(sentryProperties.getLogging().getMinimumLevel()), + sentryProperties.getLogging().isEnableLogs(), null, null, ScopesAdapter.getInstance(), diff --git a/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLogbackInitializer.java b/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLogbackInitializer.java index 58de0bc4b26..c51e2cda077 100644 --- a/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLogbackInitializer.java +++ b/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryLogbackInitializer.java @@ -45,6 +45,7 @@ public void onApplicationEvent(final @NotNull ApplicationEvent event) { if (!isSentryAppenderRegistered(logger)) { final SentryAppender sentryAppender = getSentryAppender(); + sentryAppender.setEnableLogs(sentryProperties.getLogging().isEnableLogs()); Optional.ofNullable(sentryProperties.getLogging().getMinimumBreadcrumbLevel()) .map(slf4jLevel -> Level.toLevel(slf4jLevel.name())) .ifPresent(sentryAppender::setMinimumBreadcrumbLevel); diff --git a/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryProperties.java b/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryProperties.java index edb8d44cdd3..41358f8cc84 100644 --- a/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryProperties.java +++ b/sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryProperties.java @@ -129,6 +129,9 @@ public static class Logging { /** Enable/Disable logging auto-configuration. */ private boolean enabled = true; + /** Enable/Disable Sentry Logs capture from the auto-configured appender. */ + private boolean enableLogs = false; + /** Minimum logging level for recording breadcrumbs. */ private @Nullable Level minimumBreadcrumbLevel; @@ -149,6 +152,14 @@ public void setEnabled(boolean enabled) { this.enabled = enabled; } + public boolean isEnableLogs() { + return enableLogs; + } + + public void setEnableLogs(boolean enableLogs) { + this.enableLogs = enableLogs; + } + public @Nullable Level getMinimumBreadcrumbLevel() { return minimumBreadcrumbLevel; } diff --git a/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryAutoConfigurationTest.kt b/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryAutoConfigurationTest.kt index c0eb866de02..e8ec76cdbbb 100644 --- a/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryAutoConfigurationTest.kt @@ -244,7 +244,7 @@ class SentryAutoConfigurationTest { "sentry.cron.default-timezone=America/New_York", "sentry.cron.default-failure-issue-threshold=40", "sentry.cron.default-recovery-threshold=50", - "sentry.logs.enabled=true", + "sentry.logging.enable-logs=true", "sentry.strict-trace-continuation=true", "sentry.org-id=12345", ) @@ -301,7 +301,7 @@ class SentryAutoConfigurationTest { assertThat(options.cron!!.defaultTimezone).isEqualTo("America/New_York") assertThat(options.cron!!.defaultFailureIssueThreshold).isEqualTo(40L) assertThat(options.cron!!.defaultRecoveryThreshold).isEqualTo(50L) - assertThat(options.logs.isEnabled).isEqualTo(true) + assertThat(options.logging.isEnableLogs).isTrue() assertThat(options.isStrictTraceContinuation).isEqualTo(true) assertThat(options.orgId).isEqualTo("12345") } diff --git a/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLog4j2AppenderAutoConfigurationTest.kt b/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLog4j2AppenderAutoConfigurationTest.kt index b9bcee11d38..2518b11a402 100644 --- a/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLog4j2AppenderAutoConfigurationTest.kt +++ b/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLog4j2AppenderAutoConfigurationTest.kt @@ -6,7 +6,10 @@ import ch.qos.logback.core.read.ListAppender import io.sentry.ITransportFactory import io.sentry.NoOpTransportFactory import io.sentry.ScopesAdapter +import io.sentry.Sentry +import io.sentry.checkLogs import io.sentry.log4j2.SentryAppender +import io.sentry.transport.ITransport import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -17,6 +20,11 @@ import org.apache.logging.log4j.core.LoggerContext import org.apache.logging.log4j.core.config.DefaultConfiguration import org.apache.logging.log4j.core.config.LoggerConfig import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import org.slf4j.LoggerFactory import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.FilteredClassLoader @@ -63,6 +71,16 @@ class SentryLog4j2AppenderAutoConfigurationTest { private val dsnEnabledRunner = dsnOnlyRunner.withPropertyValues("sentry.logging.enabled=true") + private val logsRunner = + baseContextRunner + .withLog4j2CoreProvider() + .withPropertyValues( + "sentry.dsn=http://key@localhost/proj", + "sentry.logging.enabled=true", + "sentry.logs.enabled=true", + ) + .withUserConfiguration(MockTransportConfiguration::class.java) + // Hide the Log4j2 Core provider so LogManager uses the Log4j-to-SLF4J bridge. private val log4j2BridgeDsnEnabledRunner = baseContextRunner @@ -181,6 +199,29 @@ class SentryLog4j2AppenderAutoConfigurationTest { } } + @Test + fun `forwards Sentry Logs when enabled`() { + logsRunner.withPropertyValues("sentry.logging.enable-logs=true").run { + LogManager.getLogger("io.sentry.spring.boot4.logs-enabled").error("enabled log") + Sentry.flush(1000) + + val transport = it.getBean(ITransport::class.java) + verify(transport) + .send(checkLogs { logs -> assertThat(logs.items.single().body).isEqualTo("enabled log") }) + } + } + + @Test + fun `does not forward Sentry Logs by default`() { + logsRunner.run { + LogManager.getLogger("io.sentry.spring.boot4.logs-disabled").error("disabled log") + Sentry.flush(1000) + + val transport = it.getBean(ITransport::class.java) + verify(transport, never()).send(checkLogs {}) + } + } + @Test fun `does not configure SentryAppender when logging is disabled`() { dsnEnabledRunner.withPropertyValues("sentry.logging.enabled=false").run { @@ -251,6 +292,21 @@ class SentryLog4j2AppenderAutoConfigurationTest { .run { assertThat(rootLogger.getAppenders(SentryAppender::class.java)).isEmpty() } } + @Configuration(proxyBeanMethods = false) + open class MockTransportConfiguration { + + private val transport = mock() + + @Bean + open fun mockTransportFactory(): ITransportFactory { + val factory = mock() + whenever(factory.create(any(), any())).thenReturn(transport) + return factory + } + + @Bean open fun sentryTransport() = transport + } + @Configuration(proxyBeanMethods = false) open class NoOpTransportConfiguration { diff --git a/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLogbackAppenderAutoConfigurationTest.kt b/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLogbackAppenderAutoConfigurationTest.kt index 681932e6f89..be3f7863b03 100644 --- a/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLogbackAppenderAutoConfigurationTest.kt +++ b/sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryLogbackAppenderAutoConfigurationTest.kt @@ -112,6 +112,7 @@ class SentryLogbackAppenderAutoConfigurationTest { "sentry.logging.minimum-event-level=info", "sentry.logging.minimum-breadcrumb-level=debug", "sentry.logging.minimum-level=error", + "sentry.logging.enable-logs=true", ) .run { val appenders = rootLogger.getAppenders(SentryAppender::class.java) @@ -121,9 +122,19 @@ class SentryLogbackAppenderAutoConfigurationTest { assertThat(sentryAppender.minimumBreadcrumbLevel).isEqualTo(Level.DEBUG) assertThat(sentryAppender.minimumEventLevel).isEqualTo(Level.INFO) assertThat(sentryAppender.minimumLevel).isEqualTo(Level.ERROR) + assertThat(sentryAppender.isEnableLogs).isTrue() } } + @Test + fun `SentryAppender Logs are disabled by default`() { + dsnEnabledRunner.run { + val sentryAppender = rootLogger.getAppenders(SentryAppender::class.java).single() + + assertThat((sentryAppender as SentryAppender).isEnableLogs).isFalse() + } + } + @Test fun `does not configure SentryAppender when logging is disabled`() { contextRunner.withPropertyValues("sentry.logging.enabled=false").run { diff --git a/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api b/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api index 52633965cda..c8f55d822a2 100644 --- a/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api +++ b/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api @@ -71,7 +71,9 @@ public class io/sentry/spring/boot/jakarta/SentryProperties$Logging { public fun getMinimumBreadcrumbLevel ()Lorg/slf4j/event/Level; public fun getMinimumEventLevel ()Lorg/slf4j/event/Level; public fun getMinimumLevel ()Lorg/slf4j/event/Level; + public fun isEnableLogs ()Z public fun isEnabled ()Z + public fun setEnableLogs (Z)V public fun setEnabled (Z)V public fun setLoggers (Ljava/util/List;)V public fun setMinimumBreadcrumbLevel (Lorg/slf4j/event/Level;)V diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLog4j2Initializer.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLog4j2Initializer.java index 141f6a8ba00..4b2588da8e7 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLog4j2Initializer.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLog4j2Initializer.java @@ -95,6 +95,7 @@ public void onApplicationEvent(final @NotNull ApplicationEvent event) { toLog4jLevel(sentryProperties.getLogging().getMinimumBreadcrumbLevel()), toLog4jLevel(sentryProperties.getLogging().getMinimumEventLevel()), toLog4jLevel(sentryProperties.getLogging().getMinimumLevel()), + sentryProperties.getLogging().isEnableLogs(), null, null, ScopesAdapter.getInstance(), diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLogbackInitializer.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLogbackInitializer.java index be222eae1bf..fa6cd7a76ce 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLogbackInitializer.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryLogbackInitializer.java @@ -45,6 +45,7 @@ public void onApplicationEvent(final @NotNull ApplicationEvent event) { if (!isSentryAppenderRegistered(logger)) { final SentryAppender sentryAppender = getSentryAppender(); + sentryAppender.setEnableLogs(sentryProperties.getLogging().isEnableLogs()); Optional.ofNullable(sentryProperties.getLogging().getMinimumBreadcrumbLevel()) .map(slf4jLevel -> Level.toLevel(slf4jLevel.name())) .ifPresent(sentryAppender::setMinimumBreadcrumbLevel); diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java index 7813c2e5512..223dcce8696 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java @@ -129,6 +129,9 @@ public static class Logging { /** Enable/Disable logging auto-configuration. */ private boolean enabled = true; + /** Enable/Disable Sentry Logs capture from the auto-configured appender. */ + private boolean enableLogs = false; + /** Minimum logging level for recording breadcrumbs. */ private @Nullable Level minimumBreadcrumbLevel; @@ -149,6 +152,14 @@ public void setEnabled(boolean enabled) { this.enabled = enabled; } + public boolean isEnableLogs() { + return enableLogs; + } + + public void setEnableLogs(boolean enableLogs) { + this.enableLogs = enableLogs; + } + public @Nullable Level getMinimumBreadcrumbLevel() { return minimumBreadcrumbLevel; } diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt index 31876882c4d..6d584c9d609 100644 --- a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt @@ -246,7 +246,7 @@ class SentryAutoConfigurationTest { "sentry.cron.default-timezone=America/New_York", "sentry.cron.default-failure-issue-threshold=40", "sentry.cron.default-recovery-threshold=50", - "sentry.logs.enabled=true", + "sentry.logging.enable-logs=true", "sentry.profile-session-sample-rate=1.0", "sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces", "sentry.profile-lifecycle=TRACE", @@ -305,7 +305,7 @@ class SentryAutoConfigurationTest { assertThat(options.cron!!.defaultTimezone).isEqualTo("America/New_York") assertThat(options.cron!!.defaultFailureIssueThreshold).isEqualTo(40L) assertThat(options.cron!!.defaultRecoveryThreshold).isEqualTo(50L) - assertThat(options.logs.isEnabled).isEqualTo(true) + assertThat(options.logging.isEnableLogs).isTrue() assertThat(options.profileSessionSampleRate).isEqualTo(1.0) assertThat(options.profilingTracesDirPath) .startsWith(File("tmp/sentry/profiling-traces").absolutePath) diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLog4j2AppenderAutoConfigurationTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLog4j2AppenderAutoConfigurationTest.kt index 08eb6ac45c7..993398923be 100644 --- a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLog4j2AppenderAutoConfigurationTest.kt +++ b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLog4j2AppenderAutoConfigurationTest.kt @@ -6,7 +6,10 @@ import ch.qos.logback.core.read.ListAppender import io.sentry.ITransportFactory import io.sentry.NoOpTransportFactory import io.sentry.ScopesAdapter +import io.sentry.Sentry +import io.sentry.checkLogs import io.sentry.log4j2.SentryAppender +import io.sentry.transport.ITransport import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -17,6 +20,11 @@ import org.apache.logging.log4j.core.LoggerContext import org.apache.logging.log4j.core.config.DefaultConfiguration import org.apache.logging.log4j.core.config.LoggerConfig import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import org.slf4j.LoggerFactory import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.FilteredClassLoader @@ -63,6 +71,16 @@ class SentryLog4j2AppenderAutoConfigurationTest { private val dsnEnabledRunner = dsnOnlyRunner.withPropertyValues("sentry.logging.enabled=true") + private val logsRunner = + baseContextRunner + .withLog4j2CoreProvider() + .withPropertyValues( + "sentry.dsn=http://key@localhost/proj", + "sentry.logging.enabled=true", + "sentry.logs.enabled=true", + ) + .withUserConfiguration(MockTransportConfiguration::class.java) + // Hide the Log4j2 Core provider so LogManager uses the Log4j-to-SLF4J bridge. private val log4j2BridgeDsnEnabledRunner = baseContextRunner @@ -181,6 +199,29 @@ class SentryLog4j2AppenderAutoConfigurationTest { } } + @Test + fun `forwards Sentry Logs when enabled`() { + logsRunner.withPropertyValues("sentry.logging.enable-logs=true").run { + LogManager.getLogger("io.sentry.spring.boot.jakarta.logs-enabled").error("enabled log") + Sentry.flush(1000) + + val transport = it.getBean(ITransport::class.java) + verify(transport) + .send(checkLogs { logs -> assertThat(logs.items.single().body).isEqualTo("enabled log") }) + } + } + + @Test + fun `does not forward Sentry Logs by default`() { + logsRunner.run { + LogManager.getLogger("io.sentry.spring.boot.jakarta.logs-disabled").error("disabled log") + Sentry.flush(1000) + + val transport = it.getBean(ITransport::class.java) + verify(transport, never()).send(checkLogs {}) + } + } + @Test fun `does not configure SentryAppender when logging is disabled`() { dsnEnabledRunner.withPropertyValues("sentry.logging.enabled=false").run { @@ -251,6 +292,21 @@ class SentryLog4j2AppenderAutoConfigurationTest { .run { assertThat(rootLogger.getAppenders(SentryAppender::class.java)).isEmpty() } } + @Configuration(proxyBeanMethods = false) + open class MockTransportConfiguration { + + private val transport = mock() + + @Bean + open fun mockTransportFactory(): ITransportFactory { + val factory = mock() + whenever(factory.create(any(), any())).thenReturn(transport) + return factory + } + + @Bean open fun sentryTransport() = transport + } + @Configuration(proxyBeanMethods = false) open class NoOpTransportConfiguration { diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLogbackAppenderAutoConfigurationTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLogbackAppenderAutoConfigurationTest.kt index d8982d995c1..5dfcd5cd324 100644 --- a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLogbackAppenderAutoConfigurationTest.kt +++ b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryLogbackAppenderAutoConfigurationTest.kt @@ -112,6 +112,7 @@ class SentryLogbackAppenderAutoConfigurationTest { "sentry.logging.minimum-event-level=info", "sentry.logging.minimum-breadcrumb-level=debug", "sentry.logging.minimum-level=error", + "sentry.logging.enable-logs=true", ) .run { val appenders = rootLogger.getAppenders(SentryAppender::class.java) @@ -121,9 +122,19 @@ class SentryLogbackAppenderAutoConfigurationTest { assertThat(sentryAppender.minimumBreadcrumbLevel).isEqualTo(Level.DEBUG) assertThat(sentryAppender.minimumEventLevel).isEqualTo(Level.INFO) assertThat(sentryAppender.minimumLevel).isEqualTo(Level.ERROR) + assertThat(sentryAppender.isEnableLogs).isTrue() } } + @Test + fun `SentryAppender Logs are disabled by default`() { + dsnEnabledRunner.run { + val sentryAppender = rootLogger.getAppenders(SentryAppender::class.java).single() + + assertThat((sentryAppender as SentryAppender).isEnableLogs).isFalse() + } + } + @Test fun `does not configure SentryAppender when logging is disabled`() { contextRunner.withPropertyValues("sentry.logging.enabled=false").run { diff --git a/sentry-spring-boot/api/sentry-spring-boot.api b/sentry-spring-boot/api/sentry-spring-boot.api index ef726c4fc25..3a34fcc542d 100644 --- a/sentry-spring-boot/api/sentry-spring-boot.api +++ b/sentry-spring-boot/api/sentry-spring-boot.api @@ -56,7 +56,9 @@ public class io/sentry/spring/boot/SentryProperties$Logging { public fun getMinimumBreadcrumbLevel ()Lorg/slf4j/event/Level; public fun getMinimumEventLevel ()Lorg/slf4j/event/Level; public fun getMinimumLevel ()Lorg/slf4j/event/Level; + public fun isEnableLogs ()Z public fun isEnabled ()Z + public fun setEnableLogs (Z)V public fun setEnabled (Z)V public fun setLoggers (Ljava/util/List;)V public fun setMinimumBreadcrumbLevel (Lorg/slf4j/event/Level;)V diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryLogbackInitializer.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryLogbackInitializer.java index 94ba7b743fe..6997aca3fc8 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryLogbackInitializer.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryLogbackInitializer.java @@ -45,6 +45,7 @@ public void onApplicationEvent(final @NotNull ApplicationEvent event) { if (!isSentryAppenderRegistered(logger)) { final SentryAppender sentryAppender = getSentryAppender(); + sentryAppender.setEnableLogs(sentryProperties.getLogging().isEnableLogs()); Optional.ofNullable(sentryProperties.getLogging().getMinimumBreadcrumbLevel()) .map(slf4jLevel -> Level.toLevel(slf4jLevel.name())) .ifPresent(sentryAppender::setMinimumBreadcrumbLevel); diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java index f959fc930ba..876cb552571 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -103,6 +103,9 @@ public static class Logging { /** Enable/Disable logging auto-configuration. */ private boolean enabled = true; + /** Enable/Disable Sentry Logs capture from the auto-configured appender. */ + private boolean enableLogs = false; + /** Minimum logging level for recording breadcrumbs. */ private @Nullable Level minimumBreadcrumbLevel; @@ -123,6 +126,14 @@ public void setEnabled(boolean enabled) { this.enabled = enabled; } + public boolean isEnableLogs() { + return enableLogs; + } + + public void setEnableLogs(boolean enableLogs) { + this.enableLogs = enableLogs; + } + public @Nullable Level getMinimumBreadcrumbLevel() { return minimumBreadcrumbLevel; } diff --git a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 31be74477fd..bdc166c15e3 100644 --- a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -244,7 +244,7 @@ class SentryAutoConfigurationTest { "sentry.cron.default-timezone=America/New_York", "sentry.cron.default-failure-issue-threshold=40", "sentry.cron.default-recovery-threshold=50", - "sentry.logs.enabled=true", + "sentry.logging.enable-logs=true", "sentry.profile-session-sample-rate=1.0", "sentry.profiling-traces-dir-path=tmp/sentry/profiling-traces", "sentry.profile-lifecycle=TRACE", @@ -303,7 +303,7 @@ class SentryAutoConfigurationTest { assertThat(options.cron!!.defaultTimezone).isEqualTo("America/New_York") assertThat(options.cron!!.defaultFailureIssueThreshold).isEqualTo(40L) assertThat(options.cron!!.defaultRecoveryThreshold).isEqualTo(50L) - assertThat(options.logs.isEnabled).isEqualTo(true) + assertThat(options.logging.isEnableLogs).isTrue() assertThat(options.profileSessionSampleRate).isEqualTo(1.0) assertThat(options.profilingTracesDirPath) .startsWith(File("tmp/sentry/profiling-traces").absolutePath) diff --git a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryLogbackAppenderAutoConfigurationTest.kt b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryLogbackAppenderAutoConfigurationTest.kt index f68cad0ff90..6117ccf3f8d 100644 --- a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryLogbackAppenderAutoConfigurationTest.kt +++ b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryLogbackAppenderAutoConfigurationTest.kt @@ -112,6 +112,7 @@ class SentryLogbackAppenderAutoConfigurationTest { "sentry.logging.minimum-event-level=info", "sentry.logging.minimum-breadcrumb-level=debug", "sentry.logging.minimum-level=error", + "sentry.logging.enable-logs=true", ) .run { val appenders = rootLogger.getAppenders(SentryAppender::class.java) @@ -121,9 +122,19 @@ class SentryLogbackAppenderAutoConfigurationTest { assertThat(sentryAppender.minimumBreadcrumbLevel).isEqualTo(Level.DEBUG) assertThat(sentryAppender.minimumEventLevel).isEqualTo(Level.INFO) assertThat(sentryAppender.minimumLevel).isEqualTo(Level.ERROR) + assertThat(sentryAppender.isEnableLogs).isTrue() } } + @Test + fun `SentryAppender Logs are disabled by default`() { + dsnEnabledRunner.run { + val sentryAppender = rootLogger.getAppenders(SentryAppender::class.java).single() + + assertThat((sentryAppender as SentryAppender).isEnableLogs).isFalse() + } + } + @Test fun `does not configure SentryAppender when logging is disabled`() { contextRunner.withPropertyValues("sentry.logging.enabled=false").run { diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 53452c4c3b9..9b988911417 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -590,8 +590,6 @@ public final class io/sentry/ExternalOptions { public fun isEnableBackpressureHandling ()Ljava/lang/Boolean; public fun isEnableCacheTracing ()Ljava/lang/Boolean; public fun isEnableDatabaseTransactionTracing ()Ljava/lang/Boolean; - public fun isEnableLogs ()Ljava/lang/Boolean; - public fun isEnableMetrics ()Ljava/lang/Boolean; public fun isEnablePrettySerializationOutput ()Ljava/lang/Boolean; public fun isEnableQueueTracing ()Ljava/lang/Boolean; public fun isEnableSpotlight ()Ljava/lang/Boolean; @@ -611,8 +609,6 @@ public final class io/sentry/ExternalOptions { public fun setEnableCacheTracing (Ljava/lang/Boolean;)V public fun setEnableDatabaseTransactionTracing (Ljava/lang/Boolean;)V public fun setEnableDeduplication (Ljava/lang/Boolean;)V - public fun setEnableLogs (Ljava/lang/Boolean;)V - public fun setEnableMetrics (Ljava/lang/Boolean;)V public fun setEnablePrettySerializationOutput (Ljava/lang/Boolean;)V public fun setEnableQueueTracing (Ljava/lang/Boolean;)V public fun setEnableSpotlight (Ljava/lang/Boolean;)V @@ -4100,9 +4096,7 @@ public final class io/sentry/SentryOptions$Logs { public fun ()V public fun getBeforeSend ()Lio/sentry/SentryOptions$Logs$BeforeSendLogCallback; public fun getLoggerBatchProcessorFactory ()Lio/sentry/logger/ILoggerBatchProcessorFactory; - public fun isEnabled ()Z public fun setBeforeSend (Lio/sentry/SentryOptions$Logs$BeforeSendLogCallback;)V - public fun setEnabled (Z)V public fun setLoggerBatchProcessorFactory (Lio/sentry/logger/ILoggerBatchProcessorFactory;)V } @@ -4114,9 +4108,7 @@ public final class io/sentry/SentryOptions$Metrics { public fun ()V public fun getBeforeSend ()Lio/sentry/SentryOptions$Metrics$BeforeSendMetricCallback; public fun getMetricsBatchProcessorFactory ()Lio/sentry/metrics/IMetricsBatchProcessorFactory; - public fun isEnabled ()Z public fun setBeforeSend (Lio/sentry/SentryOptions$Metrics$BeforeSendMetricCallback;)V - public fun setEnabled (Z)V public fun setMetricsBatchProcessorFactory (Lio/sentry/metrics/IMetricsBatchProcessorFactory;)V } @@ -5568,13 +5560,6 @@ public final class io/sentry/logger/NoOpLoggerApi : io/sentry/logger/ILoggerApi public fun warn (Ljava/lang/String;[Ljava/lang/Object;)V } -public final class io/sentry/logger/NoOpLoggerBatchProcessor : io/sentry/logger/ILoggerBatchProcessor { - public fun add (Lio/sentry/SentryLogEvent;)V - public fun close (Z)V - public fun flush (J)V - public static fun getInstance ()Lio/sentry/logger/NoOpLoggerBatchProcessor; -} - public final class io/sentry/logger/SentryLogParameters { public fun ()V public static fun create (Lio/sentry/SentryAttributes;)Lio/sentry/logger/SentryLogParameters; @@ -5693,13 +5678,6 @@ public final class io/sentry/metrics/NoOpMetricsApi : io/sentry/metrics/IMetrics public static fun getInstance ()Lio/sentry/metrics/NoOpMetricsApi; } -public final class io/sentry/metrics/NoOpMetricsBatchProcessor : io/sentry/metrics/IMetricsBatchProcessor { - public fun add (Lio/sentry/SentryMetricsEvent;)V - public fun close (Z)V - public fun flush (J)V - public static fun getInstance ()Lio/sentry/metrics/NoOpMetricsBatchProcessor; -} - public final class io/sentry/metrics/SentryMetricsParameters { public fun ()V public static fun create (Lio/sentry/SentryAttributes;)Lio/sentry/metrics/SentryMetricsParameters; diff --git a/sentry/src/main/java/io/sentry/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java index 272cf1c13a9..83497c2fb06 100644 --- a/sentry/src/main/java/io/sentry/ExternalOptions.java +++ b/sentry/src/main/java/io/sentry/ExternalOptions.java @@ -46,8 +46,6 @@ public final class ExternalOptions { private @Nullable Boolean enabled; private @Nullable Boolean enablePrettySerializationOutput; private @Nullable Boolean enableSpotlight; - private @Nullable Boolean enableLogs; - private @Nullable Boolean enableMetrics; private @Nullable String spotlightConnectionUrl; private @Nullable List ignoredCheckIns; @@ -178,10 +176,6 @@ public final class ExternalOptions { options.setCaptureOpenTelemetryEvents( propertiesProvider.getBooleanProperty("capture-open-telemetry-events")); - options.setEnableLogs(propertiesProvider.getBooleanProperty("logs.enabled")); - - options.setEnableMetrics(propertiesProvider.getBooleanProperty("metrics.enabled")); - for (final String ignoredExceptionType : propertiesProvider.getList("ignored-exceptions-for-type")) { try { @@ -717,22 +711,6 @@ public void setCaptureOpenTelemetryEvents(final @Nullable Boolean captureOpenTel return captureOpenTelemetryEvents; } - public void setEnableLogs(final @Nullable Boolean enableLogs) { - this.enableLogs = enableLogs; - } - - public @Nullable Boolean isEnableLogs() { - return enableLogs; - } - - public void setEnableMetrics(final @Nullable Boolean enableMetrics) { - this.enableMetrics = enableMetrics; - } - - public @Nullable Boolean isEnableMetrics() { - return enableMetrics; - } - public @Nullable Double getProfileSessionSampleRate() { return profileSessionSampleRate; } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 4bba195feea..012587eaa59 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -9,9 +9,7 @@ import io.sentry.hints.DiskFlushNotification; import io.sentry.hints.TransactionEnd; import io.sentry.logger.ILoggerBatchProcessor; -import io.sentry.logger.NoOpLoggerBatchProcessor; import io.sentry.metrics.IMetricsBatchProcessor; -import io.sentry.metrics.NoOpMetricsBatchProcessor; import io.sentry.protocol.Contexts; import io.sentry.protocol.DebugMeta; import io.sentry.protocol.FeatureFlags; @@ -60,18 +58,9 @@ public SentryClient(final @NotNull SentryOptions options) { final RequestDetailsResolver requestDetailsResolver = new RequestDetailsResolver(options); transport = transportFactory.create(options, requestDetailsResolver.resolve()); - if (options.getLogs().isEnabled()) { - loggerBatchProcessor = - options.getLogs().getLoggerBatchProcessorFactory().create(options, this); - } else { - loggerBatchProcessor = NoOpLoggerBatchProcessor.getInstance(); - } - if (options.getMetrics().isEnabled()) { - metricsBatchProcessor = - options.getMetrics().getMetricsBatchProcessorFactory().create(options, this); - } else { - metricsBatchProcessor = NoOpMetricsBatchProcessor.getInstance(); - } + loggerBatchProcessor = options.getLogs().getLoggerBatchProcessorFactory().create(options, this); + metricsBatchProcessor = + options.getMetrics().getMetricsBatchProcessorFactory().create(options, this); } private boolean shouldApplyScopeData( diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 6dacb527276..36b34630d4e 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -3834,14 +3834,6 @@ public void merge(final @NotNull ExternalOptions options) { } } - if (options.isEnableLogs() != null) { - getLogs().setEnabled(options.isEnableLogs()); - } - - if (options.isEnableMetrics() != null) { - getMetrics().setEnabled(options.isEnableMetrics()); - } - if (options.getProfileSessionSampleRate() != null) { setProfileSessionSampleRate(options.getProfileSessionSampleRate()); } @@ -4092,9 +4084,6 @@ public void setDefaultRecoveryThreshold(@Nullable Long defaultRecoveryThreshold) public static final class Logs { - /** Whether Sentry Logs feature is enabled and Sentry.logger() usages are sent to Sentry. */ - private boolean enable = false; - /** * This function is called with an SDK specific log event object and can return a modified event * object or nothing to skip reporting the log item @@ -4104,24 +4093,6 @@ public static final class Logs { private @NotNull ILoggerBatchProcessorFactory loggerBatchProcessorFactory = new DefaultLoggerBatchProcessorFactory(); - /** - * Whether Sentry Logs feature is enabled and Sentry.logger() usages are sent to Sentry. - * - * @return true if Sentry Logs should be enabled - */ - public boolean isEnabled() { - return enable; - } - - /** - * Whether Sentry Logs feature is enabled and Sentry.logger() usages are sent to Sentry. - * - * @param enableLogs true if Sentry Logs should be enabled - */ - public void setEnabled(boolean enableLogs) { - this.enable = enableLogs; - } - /** * Returns the BeforeSendLog callback * @@ -4171,9 +4142,6 @@ public interface BeforeSendLogCallback { public static final class Metrics { - /** Whether Sentry Metrics feature is enabled and metrics are sent to Sentry. */ - private boolean enable = true; - /** * This function is called with a metric key and tags and can return false to skip sending the * metric @@ -4183,24 +4151,6 @@ public static final class Metrics { private @NotNull IMetricsBatchProcessorFactory metricsBatchProcessorFactory = new DefaultMetricsBatchProcessorFactory(); - /** - * Whether Sentry Metrics feature is enabled and metrics are sent to Sentry. - * - * @return true if Sentry Metrics should be enabled - */ - public boolean isEnabled() { - return enable; - } - - /** - * Whether Sentry Metrics feature is enabled and metrics are sent to Sentry. - * - * @param enableMetrics true if Sentry Metrics should be enabled - */ - public void setEnabled(final boolean enableMetrics) { - this.enable = enableMetrics; - } - /** * Returns the BeforeSendMetric callback * diff --git a/sentry/src/main/java/io/sentry/logger/LoggerApi.java b/sentry/src/main/java/io/sentry/logger/LoggerApi.java index c203dcbfb8f..3741ddc6209 100644 --- a/sentry/src/main/java/io/sentry/logger/LoggerApi.java +++ b/sentry/src/main/java/io/sentry/logger/LoggerApi.java @@ -104,13 +104,6 @@ private void captureLog( return; } - if (!options.getLogs().isEnabled()) { - options - .getLogger() - .log(SentryLevel.WARNING, "Sentry Log is disabled and this 'logger' call is a no-op."); - return; - } - if (message == null) { return; } diff --git a/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java b/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java index 71877c21dae..bdfe7281268 100644 --- a/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java +++ b/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java @@ -36,6 +36,7 @@ public class LoggerBatchProcessor implements ILoggerBatchProcessor { private final @NotNull Queue queue; private final @NotNull ISentryExecutorService executorService; private final @NotNull AtomicBoolean hasScheduled = new AtomicBoolean(false); + private volatile boolean hasAcceptedItem = false; private volatile boolean isShuttingDown = false; private final @NotNull ReusableCountLatch pendingCount = new ReusableCountLatch(); @@ -75,6 +76,7 @@ public void add(final @NotNull SentryLogEvent logEvent) { } pendingCount.increment(); queue.offer(logEvent); + hasAcceptedItem = true; maybeSchedule(false); } @@ -82,14 +84,14 @@ public void add(final @NotNull SentryLogEvent logEvent) { @Override public void close(final boolean isRestarting) { isShuttingDown = true; - if (isRestarting) { + if (isRestarting && hasAcceptedItem) { maybeSchedule(true); executorService.submit(() -> executorService.close(options.getShutdownTimeoutMillis())); - } else { - executorService.close(options.getShutdownTimeoutMillis()); - while (!queue.isEmpty()) { - flushBatch(); - } + return; + } + executorService.close(options.getShutdownTimeoutMillis()); + while (!queue.isEmpty()) { + flushBatch(); } } @@ -114,6 +116,9 @@ private void maybeSchedule(boolean immediately) { @Override public void flush(long timeoutMillis) { + if (!hasAcceptedItem) { + return; + } maybeSchedule(true); try { pendingCount.waitTillZero(timeoutMillis, TimeUnit.MILLISECONDS); diff --git a/sentry/src/main/java/io/sentry/logger/NoOpLoggerBatchProcessor.java b/sentry/src/main/java/io/sentry/logger/NoOpLoggerBatchProcessor.java deleted file mode 100644 index 68dc4ecf937..00000000000 --- a/sentry/src/main/java/io/sentry/logger/NoOpLoggerBatchProcessor.java +++ /dev/null @@ -1,32 +0,0 @@ -package io.sentry.logger; - -import io.sentry.SentryLogEvent; -import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.NotNull; - -@ApiStatus.Internal -public final class NoOpLoggerBatchProcessor implements ILoggerBatchProcessor { - - private static final NoOpLoggerBatchProcessor instance = new NoOpLoggerBatchProcessor(); - - private NoOpLoggerBatchProcessor() {} - - public static NoOpLoggerBatchProcessor getInstance() { - return instance; - } - - @Override - public void add(@NotNull SentryLogEvent event) { - // do nothing - } - - @Override - public void close(final boolean isRestarting) { - // do nothing - } - - @Override - public void flush(long timeoutMillis) { - // do nothing - } -} diff --git a/sentry/src/main/java/io/sentry/metrics/MetricsApi.java b/sentry/src/main/java/io/sentry/metrics/MetricsApi.java index cebcad9735c..d70b4ab5c97 100644 --- a/sentry/src/main/java/io/sentry/metrics/MetricsApi.java +++ b/sentry/src/main/java/io/sentry/metrics/MetricsApi.java @@ -118,15 +118,6 @@ private void captureMetrics( return; } - if (!options.getMetrics().isEnabled()) { - options - .getLogger() - .log( - SentryLevel.WARNING, - "Sentry Metrics is disabled and this 'metrics' call is a no-op."); - return; - } - if (name == null) { return; } diff --git a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java index 3c744dbe3c5..8df9dc653f4 100644 --- a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java +++ b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java @@ -34,16 +34,24 @@ public class MetricsBatchProcessor implements IMetricsBatchProcessor { private final @NotNull Queue queue; private final @NotNull ISentryExecutorService executorService; private final @NotNull AtomicBoolean hasScheduled = new AtomicBoolean(false); + private volatile boolean hasAcceptedItem = false; private volatile boolean isShuttingDown = false; private final @NotNull ReusableCountLatch pendingCount = new ReusableCountLatch(); public MetricsBatchProcessor( final @NotNull SentryOptions options, final @NotNull ISentryClient client) { + this(options, client, new SentryExecutorService(options)); + } + + MetricsBatchProcessor( + final @NotNull SentryOptions options, + final @NotNull ISentryClient client, + final @NotNull ISentryExecutorService executorService) { this.options = options; this.client = client; this.queue = new ConcurrentLinkedQueue<>(); - this.executorService = new SentryExecutorService(options); + this.executorService = executorService; } @Override @@ -65,6 +73,7 @@ public void add(final @NotNull SentryMetricsEvent metricsEvent) { } pendingCount.increment(); queue.offer(metricsEvent); + hasAcceptedItem = true; maybeSchedule(false); } @@ -72,14 +81,14 @@ public void add(final @NotNull SentryMetricsEvent metricsEvent) { @Override public void close(final boolean isRestarting) { isShuttingDown = true; - if (isRestarting) { + if (isRestarting && hasAcceptedItem) { maybeSchedule(true); executorService.submit(() -> executorService.close(options.getShutdownTimeoutMillis())); - } else { - executorService.close(options.getShutdownTimeoutMillis()); - while (!queue.isEmpty()) { - flushBatch(); - } + return; + } + executorService.close(options.getShutdownTimeoutMillis()); + while (!queue.isEmpty()) { + flushBatch(); } } @@ -106,6 +115,9 @@ private void maybeSchedule(boolean immediately) { @Override public void flush(long timeoutMillis) { + if (!hasAcceptedItem) { + return; + } maybeSchedule(true); try { pendingCount.waitTillZero(timeoutMillis, TimeUnit.MILLISECONDS); diff --git a/sentry/src/main/java/io/sentry/metrics/NoOpMetricsBatchProcessor.java b/sentry/src/main/java/io/sentry/metrics/NoOpMetricsBatchProcessor.java deleted file mode 100644 index 021bed5ee32..00000000000 --- a/sentry/src/main/java/io/sentry/metrics/NoOpMetricsBatchProcessor.java +++ /dev/null @@ -1,32 +0,0 @@ -package io.sentry.metrics; - -import io.sentry.SentryMetricsEvent; -import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.NotNull; - -@ApiStatus.Internal -public final class NoOpMetricsBatchProcessor implements IMetricsBatchProcessor { - - private static final NoOpMetricsBatchProcessor instance = new NoOpMetricsBatchProcessor(); - - private NoOpMetricsBatchProcessor() {} - - public static NoOpMetricsBatchProcessor getInstance() { - return instance; - } - - @Override - public void add(@NotNull SentryMetricsEvent event) { - // do nothing - } - - @Override - public void close(final boolean isRestarting) { - // do nothing - } - - @Override - public void flush(long timeoutMillis) { - // do nothing - } -} diff --git a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt index cdd184b181f..349058e4b3b 100644 --- a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt @@ -530,30 +530,6 @@ class ExternalOptionsTest { } } - @Test - fun `creates options with enableLogs set to true`() { - withPropertiesFile("logs.enabled=true") { options -> assertTrue(options.isEnableLogs == true) } - } - - @Test - fun `creates options with enableMetrics set to true`() { - withPropertiesFile("metrics.enabled=true") { options -> - assertTrue(options.isEnableMetrics == true) - } - } - - @Test - fun `creates options with enableMetrics set to false`() { - withPropertiesFile("metrics.enabled=false") { options -> - assertTrue(options.isEnableMetrics == false) - } - } - - @Test - fun `creates options with enableMetrics set to null when not set`() { - withPropertiesFile { assertNull(it.isEnableMetrics) } - } - @Test fun `creates options with profileSessionSampleRate set to 0_8`() { withPropertiesFile("profile-session-sample-rate=0.8") { options -> diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index d1cb38c6495..324a097a7fd 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -2528,16 +2528,8 @@ class ScopesTest { @Test fun `when captureLog is called on disabled client, do nothing`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } - sut.close() - - sut.logger().warn("test message") - verify(mockClient, never()).captureLog(any(), anyOrNull()) - } - - @Test - fun `when logging is not enabled, do nothing`() { val (sut, mockClient) = getEnabledScopes() + sut.close() sut.logger().warn("test message") verify(mockClient, never()).captureLog(any(), anyOrNull()) @@ -2545,7 +2537,7 @@ class ScopesTest { @Test fun `capturing null log does nothing`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().warn(null) verify(mockClient, never()).captureLog(any(), anyOrNull()) @@ -2553,7 +2545,7 @@ class ScopesTest { @Test fun `creating trace log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().trace("trace log message") @@ -2570,7 +2562,7 @@ class ScopesTest { @Test fun `creating debug log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().debug("debug log message") @@ -2587,7 +2579,7 @@ class ScopesTest { @Test fun `creating a info log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().info("info log message") @@ -2604,7 +2596,7 @@ class ScopesTest { @Test fun `creating warn log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().warn("warn log message") @@ -2621,7 +2613,7 @@ class ScopesTest { @Test fun `creating error log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().error("error log message") @@ -2638,7 +2630,7 @@ class ScopesTest { @Test fun `creating fatal log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().fatal("fatal log message") @@ -2655,7 +2647,7 @@ class ScopesTest { @Test fun `creating log works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, "log message") @@ -2672,7 +2664,7 @@ class ScopesTest { @Test fun `log with manual origin does not have origin attribute`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, "log message") @@ -2688,7 +2680,7 @@ class ScopesTest { @Test fun `log with non manual origin does have origin attribute`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut .logger() @@ -2711,7 +2703,6 @@ class ScopesTest { fun `creating log with format string works`() { val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.environment = "testenv" it.release = "1.0" it.serverName = "srv1" @@ -2752,7 +2743,7 @@ class ScopesTest { @Test fun `creating log with timestamp works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, SentryLongDate(123), "log message") @@ -2770,7 +2761,7 @@ class ScopesTest { @Test fun `creating log with attributes from map works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut .logger() @@ -2797,7 +2788,7 @@ class ScopesTest { @Test fun `creating log with attributes works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut .logger() @@ -2873,7 +2864,7 @@ class ScopesTest { @Test fun `creating log with attributes and timestamp works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut .logger() @@ -2904,7 +2895,7 @@ class ScopesTest { @Test fun `creating log with attributes and timestamp and format string works`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut .logger() @@ -2959,7 +2950,7 @@ class ScopesTest { @Test fun `creating log with without args does not add template attribute`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, "log %s") @@ -2982,7 +2973,7 @@ class ScopesTest { @Test fun `captures format string on format error`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, "log %d", "arg1") @@ -3009,7 +3000,6 @@ class ScopesTest { fun `adds user fields to log attributes if sendDefaultPii is true`() { val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.distinctId = "distinctId" it.isSendDefaultPii = true } @@ -3049,7 +3039,6 @@ class ScopesTest { fun `adds user fields to log attributes even if sendDefaultPii is false`() { val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.distinctId = "distinctId" } @@ -3088,7 +3077,6 @@ class ScopesTest { fun `unset user does provide distinct-id as user-id`() { val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.distinctId = "distinctId" } @@ -3111,7 +3099,6 @@ class ScopesTest { fun `unset user does provide null user-id when distinct-id is missing`() { val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.distinctId = null } @@ -3133,7 +3120,6 @@ class ScopesTest { fun `missing user fields do not break attributes`() { val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.isSendDefaultPii = true it.distinctId = "distinctId" } @@ -3156,7 +3142,7 @@ class ScopesTest { @Test fun `adds session replay id to log attributes`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() val replayId = SentryId() sut.scope.replayId = replayId sut.logger().log(SentryLogLevel.WARN, "log message") @@ -3174,7 +3160,7 @@ class ScopesTest { @Test fun `missing session replay id do not break attributes`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, "log message") verify(mockClient) @@ -3190,7 +3176,7 @@ class ScopesTest { @Test fun `does not add session replay buffering to log attributes if no replay id in scope and in controller`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut.logger().log(SentryLogLevel.WARN, "log message") assertEquals(SentryId.EMPTY_ID, sut.options.replayController.replayId) @@ -3210,7 +3196,7 @@ class ScopesTest { @Test fun `does not add session replay buffering to log attributes if replay id in scope`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() val replayId = SentryId() sut.scope.replayId = replayId @@ -3234,7 +3220,6 @@ class ScopesTest { val mockReplayController = mock() val (sut, mockClient) = getEnabledScopes { - it.logs.isEnabled = true it.setReplayController(mockReplayController) } val replayId = SentryId() @@ -3258,7 +3243,7 @@ class ScopesTest { @Test fun `log event has spanId from active span`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() val transaction = sut.startTransaction( @@ -3284,7 +3269,7 @@ class ScopesTest { @Test fun `log event has spanId from propagation context when no active span`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() var propagationContext: PropagationContext? = null sut.configureScope { propagationContext = it.propagationContext } @@ -3315,14 +3300,6 @@ class ScopesTest { verify(mockClient, never()).captureMetric(any(), anyOrNull(), anyOrNull()) } - @Test - fun `when metrics is not enabled, do nothing`() { - val (sut, mockClient) = getEnabledScopes { it.metrics.isEnabled = false } - - sut.metrics().count("metric name") - verify(mockClient, never()).captureMetric(any(), anyOrNull(), anyOrNull()) - } - @Test fun `creating count metric works`() { val (sut, mockClient) = getEnabledScopes() @@ -3396,7 +3373,7 @@ class ScopesTest { @Test fun `metric with non manual origin does have origin attribute`() { - val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() sut .metrics() @@ -4252,7 +4229,7 @@ class ScopesTest { @Test fun `metric event has spanId from active span`() { - val (sut, mockClient) = getEnabledScopes { it.metrics.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() val transaction = sut.startTransaction( @@ -4279,7 +4256,7 @@ class ScopesTest { @Test fun `metric event has spanId from propagation context when no active span`() { - val (sut, mockClient) = getEnabledScopes { it.metrics.isEnabled = true } + val (sut, mockClient) = getEnabledScopes() var propagationContext: PropagationContext? = null sut.configureScope { propagationContext = it.propagationContext } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 61181ee96a6..aaabd541995 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -153,6 +153,13 @@ class SentryClientTest { assertTrue(sut.isEnabled) } + @Test + fun `when client is created, metrics batch processor is created`() { + val sut = fixture.getSut() + + verify(fixture.metricsBatchProcessorFactory).create(fixture.sentryOptions, sut) + } + @Test fun `when dsn is an invalid string, client throws`() { fixture.sentryOptions.dsn = "invalid-dsn" @@ -181,7 +188,7 @@ class SentryClientTest { @Test fun `when client is closed with isRestarting false, transport waits`() { - val sut = fixture.getSut { options -> options.logs.isEnabled = true } + val sut = fixture.getSut() assertTrue(sut.isEnabled) sut.close(false) assertNotEquals(0, fixture.sentryOptions.shutdownTimeoutMillis) @@ -195,7 +202,7 @@ class SentryClientTest { @Test fun `when client is closed with isRestarting true, transport does not wait`() { - val sut = fixture.getSut { options -> options.logs.isEnabled = true } + val sut = fixture.getSut() assertTrue(sut.isEnabled) sut.close(true) verify(fixture.transport).flush(eq(0)) @@ -297,7 +304,6 @@ class SentryClientTest { @Test fun `when beforeSend captures a log, the nested log is dropped`() { val scope = createScope() - fixture.sentryOptions.logs.isEnabled = true lateinit var sut: SentryClient fixture.sentryOptions.setBeforeSend { e, _ -> sut.captureLog( @@ -318,7 +324,6 @@ class SentryClientTest { @Test fun `when beforeSendLog logs again, the nested log is dropped and does not recurse`() { val scope = createScope() - fixture.sentryOptions.logs.isEnabled = true var invocations = 0 lateinit var sut: SentryClient fixture.sentryOptions.logs.setBeforeSend { l -> diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 6b47d36e53d..3d38f48d08d 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -579,8 +579,6 @@ class SentryOptionsTest { externalOptions.isEnableSpotlight = true externalOptions.spotlightConnectionUrl = "http://local.sentry.io:1234" externalOptions.isGlobalHubMode = true - externalOptions.isEnableLogs = true - externalOptions.isEnableMetrics = false externalOptions.profileSessionSampleRate = 0.8 externalOptions.profilingTracesDirPath = "/profiling-traces" externalOptions.profileLifecycle = ProfileLifecycle.TRACE @@ -643,8 +641,6 @@ class SentryOptionsTest { assertTrue(options.isEnableSpotlight) assertEquals("http://local.sentry.io:1234", options.spotlightConnectionUrl) assertTrue(options.isGlobalHubMode!!) - assertTrue(options.logs.isEnabled!!) - assertFalse(options.metrics.isEnabled) assertEquals(0.8, options.profileSessionSampleRate) assertEquals("/profiling-traces${File.separator}${hash}", options.profilingTracesDirPath) assertEquals(ProfileLifecycle.TRACE, options.profileLifecycle) @@ -658,14 +654,6 @@ class SentryOptionsTest { assertTrue(options.isEnableUncaughtExceptionHandler) } - @Test - fun `merging options when enableMetrics is not set preserves the default value`() { - val externalOptions = ExternalOptions() - val options = SentryOptions() - options.merge(externalOptions) - assertTrue(options.metrics.isEnabled) - } - @Test fun `merging options merges and overwrites existing tag values`() { val externalOptions = ExternalOptions() @@ -897,11 +885,6 @@ class SentryOptionsTest { assertFalse(SentryOptions().isEnableQueueTracing) } - @Test - fun `when options are initialized, metrics is enabled by default`() { - assertTrue(SentryOptions().metrics.isEnabled) - } - @Test fun `when options are initialized, enableSpotlight is set to false by default`() { assertFalse(SentryOptions().isEnableSpotlight) diff --git a/sentry/src/test/java/io/sentry/logger/LoggerBatchProcessorTest.kt b/sentry/src/test/java/io/sentry/logger/LoggerBatchProcessorTest.kt index 01b8afdb4a3..21506c2443c 100644 --- a/sentry/src/test/java/io/sentry/logger/LoggerBatchProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/logger/LoggerBatchProcessorTest.kt @@ -3,6 +3,7 @@ package io.sentry.logger import com.google.common.truth.Truth.assertThat import io.sentry.DataCategory import io.sentry.ISentryClient +import io.sentry.ISentryExecutorService import io.sentry.SentryLogEvent import io.sentry.SentryLogEvents import io.sentry.SentryLogLevel @@ -13,19 +14,106 @@ import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.DiscardedEvent import io.sentry.protocol.SentryId import io.sentry.test.DeferredExecutorService +import io.sentry.test.getProperty import io.sentry.test.injectForField +import io.sentry.transport.ReusableCountLatch import io.sentry.util.JsonSerializationUtils import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.atLeast import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions class LoggerBatchProcessorTest { + @Test + fun `constructor does not submit processor work`() { + val mockExecutor = mock() + + LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `empty flush does not submit processor work`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.flush(0) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `close before first accepted item does not submit processor work`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.close(false) + + verify(mockExecutor).close(any()) + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + + @Test + fun `restart close before first accepted item does not submit processor work`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.close(true) + + verify(mockExecutor).close(any()) + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + + @Test + fun `item rejected during shutdown does not mark processor as used`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + processor.close(false) + + processor.add(logEvent("rejected")) + processor.flush(0) + + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + + @Test + fun `item rejected due to queue capacity does not mark processor as used`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + val pendingCount = processor.getProperty("pendingCount") + repeat(LoggerBatchProcessor.MAX_QUEUE_SIZE) { pendingCount.increment() } + + processor.add(logEvent("rejected")) + processor.flush(0) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `flush and restart close submit processor work after first accepted item`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + processor.add(logEvent("accepted")) + + processor.flush(0) + processor.close(true) + + verify(mockExecutor, times(3)).schedule(any(), any()) + verify(mockExecutor).submit(any()) + } + @Test fun `schedules another flush after previous flush has run`() { val mockClient = mock() @@ -46,6 +134,9 @@ class LoggerBatchProcessorTest { .inOrder() } + private fun logEvent(body: String) = + SentryLogEvent(SentryId(), SentryNanotimeDate(), body, SentryLogLevel.INFO) + @Test fun `drops log events after reaching MAX_QUEUE_SIZE limit`() { // given diff --git a/sentry/src/test/java/io/sentry/metrics/MetricsBatchProcessorTest.kt b/sentry/src/test/java/io/sentry/metrics/MetricsBatchProcessorTest.kt index d8320d9b1a6..99b8deba2a0 100644 --- a/sentry/src/test/java/io/sentry/metrics/MetricsBatchProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/metrics/MetricsBatchProcessorTest.kt @@ -3,6 +3,7 @@ package io.sentry.metrics import com.google.common.truth.Truth.assertThat import io.sentry.DataCategory import io.sentry.ISentryClient +import io.sentry.ISentryExecutorService import io.sentry.SentryMetricsEvent import io.sentry.SentryMetricsEvents import io.sentry.SentryNanotimeDate @@ -12,19 +13,106 @@ import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.DiscardedEvent import io.sentry.protocol.SentryId import io.sentry.test.DeferredExecutorService +import io.sentry.test.getProperty import io.sentry.test.injectForField +import io.sentry.transport.ReusableCountLatch import io.sentry.util.JsonSerializationUtils import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.atLeast import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions class MetricsBatchProcessorTest { + @Test + fun `constructor does not submit processor work`() { + val mockExecutor = mock() + + MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `empty flush does not submit processor work`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.flush(0) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `close before first accepted item does not submit processor work`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.close(false) + + verify(mockExecutor).close(any()) + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + + @Test + fun `restart close before first accepted item does not submit processor work`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.close(true) + + verify(mockExecutor).close(any()) + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + + @Test + fun `item rejected during shutdown does not mark processor as used`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + processor.close(false) + + processor.add(metricsEvent("rejected")) + processor.flush(0) + + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + + @Test + fun `item rejected due to queue capacity does not mark processor as used`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + val pendingCount = processor.getProperty("pendingCount") + repeat(MetricsBatchProcessor.MAX_QUEUE_SIZE) { pendingCount.increment() } + + processor.add(metricsEvent("rejected")) + processor.flush(0) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `flush and restart close submit processor work after first accepted item`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock(), mockExecutor) + processor.add(metricsEvent("accepted")) + + processor.flush(0) + processor.close(true) + + verify(mockExecutor, times(3)).schedule(any(), any()) + verify(mockExecutor).submit(any()) + } + @Test fun `schedules another flush after previous flush has run`() { val mockClient = mock() @@ -46,6 +134,9 @@ class MetricsBatchProcessorTest { .inOrder() } + private fun metricsEvent(name: String) = + SentryMetricsEvent(SentryId(), SentryNanotimeDate(), name, "gauge", 1.0) + @Test fun `drops metrics events after reaching MAX_QUEUE_SIZE limit`() { // given