fix(core): No longer guard direct usage of Sentry.metrics() and Sentry.logger() with enable flags

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+### Breaking Changes
+
+- Capture direct `Sentry.logger()` and `Sentry.metrics()` calls regardless of `options.getLogs().isEnabled()` and `options.getMetrics().isEnabled()` ([#6184](https://github.com/getsentry/sentry-java/pull/6184))
+  - Automatic logging integrations still only capture logs when `options.getLogs().isEnabled()` is `true`. `options.getMetrics().setEnabled(...)` currently has no effect because there are no automatic Metrics integrations. Both enable options will be removed in the upcoming major release, and each logging integration will then have a separate opt-in flag.
+  - Use `options.getLogs().setBeforeSend(...)` and `options.getMetrics().setBeforeSend(...)` to filter manually emitted telemetry. Return `null` from either callback to drop it.
+
 ### Fixes
 
 - Keep the videos of already captured session replay segments when the replay stops, so segments that are still queued are no longer sent without their video ([#6177](https://github.com/getsentry/sentry-java/pull/6177))

sentry-android-core/src/main/java/io/sentry/android/core/AndroidLoggerBatchProcessor.java

@@ -24,6 +24,9 @@ public void onForeground() {
 
   @Override
   public void onBackground() {
+    if (!hasAcceptedItem) {
+      return;
+    }
     try {
       options
           .getExecutorService()

sentry-android-core/src/main/java/io/sentry/android/core/AndroidMetricsBatchProcessor.java

@@ -24,6 +24,9 @@ public void onForeground() {
 
   @Override
   public void onBackground() {
+    if (!hasAcceptedItem) {
+      return;
+    }
     try {
       options
           .getExecutorService()

sentry-android-core/src/test/java/io/sentry/android/core/AndroidLoggerBatchProcessorTest.kt

@@ -2,6 +2,7 @@ package io.sentry.android.core
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.ISentryClient
+import io.sentry.ISentryExecutorService
 import io.sentry.SentryLogEvent
 import io.sentry.SentryLogLevel
 import io.sentry.SentryOptions
@@ -16,6 +17,7 @@ import org.junit.runner.RunWith
 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
 
 @RunWith(AndroidJUnit4::class)
@@ -66,13 +68,24 @@ class AndroidLoggerBatchProcessorTest {
     verify(fixture.client).captureBatchedLogEvents(any())
   }
 
+  @Test
+  fun `onBackground does not submit shared executor work without accepted items`() {
+    val sharedExecutor = mock<ISentryExecutorService>()
+    val sut = fixture.getSut { options -> options.executorService = sharedExecutor }
+
+    sut.onBackground()
+
+    verifyNoInteractions(sharedExecutor)
+  }
+
   @Test
   fun `onBackground handles executor exception gracefully`() {
     val sut = fixture.getSut { options ->
-      val rejectingExecutor = mock<io.sentry.ISentryExecutorService>()
+      val rejectingExecutor = mock<ISentryExecutorService>()
       whenever(rejectingExecutor.submit(any())).thenThrow(RuntimeException("Rejected"))
       options.executorService = rejectingExecutor
     }
+    sut.add(SentryLogEvent(SentryId(), 1.0, "test", SentryLogLevel.INFO))
 
     // Should not throw
     sut.onBackground()

sentry-android-core/src/test/java/io/sentry/android/core/AndroidMetricsBatchProcessorTest.kt

@@ -2,6 +2,7 @@ package io.sentry.android.core
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.ISentryClient
+import io.sentry.ISentryExecutorService
 import io.sentry.SentryMetricsEvent
 import io.sentry.SentryOptions
 import io.sentry.protocol.SentryId
@@ -15,6 +16,7 @@ import org.junit.runner.RunWith
 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
 
 @RunWith(AndroidJUnit4::class)
@@ -65,13 +67,24 @@ class AndroidMetricsBatchProcessorTest {
     verify(fixture.client).captureBatchedMetricsEvents(any())
   }
 
+  @Test
+  fun `onBackground does not submit shared executor work without accepted items`() {
+    val sharedExecutor = mock<ISentryExecutorService>()
+    val sut = fixture.getSut { options -> options.executorService = sharedExecutor }
+
+    sut.onBackground()
+
+    verifyNoInteractions(sharedExecutor)
+  }
+
   @Test
   fun `onBackground handles executor exception gracefully`() {
     val sut = fixture.getSut { options ->
-      val rejectingExecutor = mock<io.sentry.ISentryExecutorService>()
+      val rejectingExecutor = mock<ISentryExecutorService>()
       whenever(rejectingExecutor.submit(any())).thenThrow(RuntimeException("Rejected"))
       options.executorService = rejectingExecutor
     }
+    sut.add(SentryMetricsEvent(SentryId(), 1.0, "test", "counter", 3.0))
 
     // Should not throw
     sut.onBackground()

sentry-android-timber/src/main/java/io/sentry/android/timber/SentryTimberTree.kt

@@ -246,6 +246,10 @@ public class SentryTimberTree(
     throwable: Throwable?,
     vararg args: Any?,
   ) {
+    if (!scopes.options.logs.isEnabled) {
+      return
+    }
+
     // checks the log level
     if (isLoggable(sentryLogLevel, minLogLevel)) {
       val attributes = tag?.let {

sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt

@@ -24,6 +24,10 @@ class SentryTimberIntegrationTest {
     val scopes = mock<IScopes>()
     val options = SentryOptions().apply { sdkVersion = SdkVersion("test", "1.2.3") }
 
+    init {
+      whenever(scopes.options).thenReturn(options)
+    }
+
     fun getSut(
       minEventLevel: SentryLevel = SentryLevel.ERROR,
       minBreadcrumbLevel: SentryLevel = SentryLevel.INFO,

sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberTreeTest.kt

@@ -5,6 +5,7 @@ import io.sentry.Breadcrumb
 import io.sentry.Scopes
 import io.sentry.SentryLevel
 import io.sentry.SentryLogLevel
+import io.sentry.SentryOptions
 import io.sentry.logger.ILoggerApi
 import io.sentry.logger.SentryLogParameters
 import kotlin.test.BeforeTest
@@ -31,9 +32,12 @@ class SentryTimberTreeTest {
       minEventLevel: SentryLevel = SentryLevel.ERROR,
       minBreadcrumbLevel: SentryLevel = SentryLevel.INFO,
       minLogsLevel: SentryLogLevel = SentryLogLevel.INFO,
+      logsEnabled: Boolean = true,
     ): SentryTimberTree {
       logs = mock<ILoggerApi>()
       scopes = mock<Scopes>()
+      val options = SentryOptions().apply { logs.isEnabled = logsEnabled }
+      whenever(scopes.options).thenReturn(options)
       whenever(scopes.logger()).thenReturn(logs)
       return SentryTimberTree(scopes, minEventLevel, minBreadcrumbLevel, minLogsLevel)
     }
@@ -332,6 +336,17 @@ class SentryTimberTreeTest {
     verifyNoInteractions(fixture.logs)
   }
 
+  @Test
+  fun `Tree does not add a log when automatic Logs are disabled`() {
+    val sut = fixture.getSut(logsEnabled = false)
+
+    sut.e("message")
+
+    verifyNoInteractions(fixture.logs)
+    verify(fixture.scopes).captureEvent(any())
+    verify(fixture.scopes).addBreadcrumb(any<Breadcrumb>())
+  }
+
   @Test
   fun `Tree adds an info log`() {
     val sut = fixture.getSut()

sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml

@@ -147,7 +147,7 @@
       android:name="io.sentry.debug"
       android:value="${sentryDebug}" />
 
-    <!--    how to enable Sentry Logs (Sentry.logger() calls are dropped unless this is on)-->
+    <!--    how to enable automatic Sentry Logs from Logcat and Timber-->
     <meta-data
       android:name="io.sentry.logs.enabled"
       android:value="true" />

sentry/api/sentry.api

@@ -5548,6 +5548,7 @@ public class io/sentry/logger/LoggerBatchProcessor : io/sentry/logger/ILoggerBat
 	public static final field FLUSH_AFTER_MS I
 	public static final field MAX_BATCH_SIZE I
 	public static final field MAX_QUEUE_SIZE I
+	protected field hasAcceptedItem Z
 	protected final field options Lio/sentry/SentryOptions;
 	public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;)V
 	public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;Lio/sentry/ISentryExecutorService;)V
@@ -5636,6 +5637,7 @@ public class io/sentry/metrics/MetricsBatchProcessor : io/sentry/metrics/IMetric
 	public static final field FLUSH_AFTER_MS I
 	public static final field MAX_BATCH_SIZE I
 	public static final field MAX_QUEUE_SIZE I
+	protected field hasAcceptedItem Z
 	protected final field options Lio/sentry/SentryOptions;
 	public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;)V
 	public fun add (Lio/sentry/SentryMetricsEvent;)V

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(

sentry/src/main/java/io/sentry/SentryOptions.java

@@ -4092,7 +4092,7 @@ 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. */
+    /** Whether automatic logging integrations send Logs to Sentry. */
     private boolean enable = false;
 
     /**
@@ -4105,18 +4105,20 @@ public static final class Logs {
         new DefaultLoggerBatchProcessorFactory();
 
     /**
-     * Whether Sentry Logs feature is enabled and Sentry.logger() usages are sent to Sentry.
+     * Whether automatic logging integrations send Logs to Sentry. Direct {@code Sentry.logger()}
+     * calls are always captured when the SDK is enabled.
      *
-     * @return true if Sentry Logs should be enabled
+     * @return true if automatic Logs should be enabled
      */
     public boolean isEnabled() {
       return enable;
     }
 
     /**
-     * Whether Sentry Logs feature is enabled and Sentry.logger() usages are sent to Sentry.
+     * Whether automatic logging integrations send Logs to Sentry. Direct {@code Sentry.logger()}
+     * calls are always captured when the SDK is enabled.
      *
-     * @param enableLogs true if Sentry Logs should be enabled
+     * @param enableLogs true if automatic Logs should be enabled
      */
     public void setEnabled(boolean enableLogs) {
       this.enable = enableLogs;
@@ -4171,7 +4173,7 @@ public interface BeforeSendLogCallback {
 
   public static final class Metrics {
 
-    /** Whether Sentry Metrics feature is enabled and metrics are sent to Sentry. */
+    /** The configured state for automatic Metrics integrations, which currently do not exist. */
     private boolean enable = true;
 
     /**
@@ -4184,18 +4186,24 @@ public static final class Metrics {
         new DefaultMetricsBatchProcessorFactory();
 
     /**
-     * Whether Sentry Metrics feature is enabled and metrics are sent to Sentry.
+     * Returns the configured state for automatic Metrics integrations.
      *
-     * @return true if Sentry Metrics should be enabled
+     * <p>This option currently has no effect because there are no automatic Metrics integrations.
+     * Direct {@code Sentry.metrics()} calls are always captured when the SDK is enabled.
+     *
+     * @return the configured value for automatic Metrics integrations
      */
     public boolean isEnabled() {
       return enable;
     }
 
     /**
-     * Whether Sentry Metrics feature is enabled and metrics are sent to Sentry.
+     * Configures whether automatic Metrics integrations send Metrics to Sentry.
+     *
+     * <p>This option currently has no effect because there are no automatic Metrics integrations.
+     * Direct {@code Sentry.metrics()} calls are always captured when the SDK is enabled.
      *
-     * @param enableMetrics true if Sentry Metrics should be enabled
+     * @param enableMetrics whether automatic Metrics integrations should be enabled
      */
     public void setEnabled(final boolean enableMetrics) {
       this.enable = enableMetrics;

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;
       }

sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java

@@ -36,6 +36,7 @@ public class LoggerBatchProcessor implements ILoggerBatchProcessor {
   private final @NotNull Queue<SentryLogEvent> queue;
   private final @NotNull ISentryExecutorService executorService;
   private final @NotNull AtomicBoolean hasScheduled = new AtomicBoolean(false);
+  protected volatile boolean hasAcceptedItem = false;
   private volatile boolean isShuttingDown = false;
 
   private final @NotNull ReusableCountLatch pendingCount = new ReusableCountLatch();
@@ -75,21 +76,24 @@ public void add(final @NotNull SentryLogEvent logEvent) {
     }
     pendingCount.increment();
     queue.offer(logEvent);
+    hasAcceptedItem = true;
     maybeSchedule(false);
   }
 
   @SuppressWarnings("FutureReturnValueIgnored")
   @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;
+    }
+    // On restart, reaching this path means the executor never had anything scheduled, so
+    // synchronous shutdown should be immediate.
+    executorService.close(options.getShutdownTimeoutMillis());
+    while (!queue.isEmpty()) {
+      flushBatch();
     }
   }
 
@@ -114,6 +118,9 @@ private void maybeSchedule(boolean immediately) {
 
   @Override
   public void flush(long timeoutMillis) {
+    if (!hasAcceptedItem) {
+      return;
+    }
     maybeSchedule(true);
     try {
       pendingCount.waitTillZero(timeoutMillis, TimeUnit.MILLISECONDS);

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;
       }