diff --git a/CHANGELOG.md b/CHANGELOG.md index 3147779ad42..61dec289bd3 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidLoggerBatchProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidLoggerBatchProcessor.java index 13b12dc702a..b81666e2f38 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidLoggerBatchProcessor.java +++ b/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() diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMetricsBatchProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMetricsBatchProcessor.java index 290f2a9d4ed..d92c4369139 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMetricsBatchProcessor.java +++ b/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() 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..18bf67cd11c 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 @@ -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() + 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() + val rejectingExecutor = mock() whenever(rejectingExecutor.submit(any())).thenThrow(RuntimeException("Rejected")) options.executorService = rejectingExecutor } + sut.add(SentryLogEvent(SentryId(), 1.0, "test", SentryLogLevel.INFO)) // Should not throw sut.onBackground() 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..a965c066279 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 @@ -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() + 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() + val rejectingExecutor = mock() 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() 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..707fc28d965 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 @@ -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 { 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..758e6568c10 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 @@ -24,6 +24,10 @@ class SentryTimberIntegrationTest { val scopes = mock() 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, 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..8ef43a9b691 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 @@ -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() scopes = mock() + 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()) + } + @Test fun `Tree adds an info log`() { val sut = fixture.getSut() diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index 6426d03b814..cf0bc55e5e0 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -147,7 +147,7 @@ android:name="io.sentry.debug" android:value="${sentryDebug}" /> - + diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index a3e33b52866..9f2a599c17e 100644 --- a/sentry/api/sentry.api +++ b/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 (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;)V public fun (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 (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;)V public fun add (Lio/sentry/SentryMetricsEvent;)V 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..2b2f9d370a0 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/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 + *

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. + * + *

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; 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..b9c32f4c0ff 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); + protected 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,16 @@ 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; + } + // 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); 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..8ac387c8b8e 100644 --- a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java +++ b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java @@ -34,6 +34,7 @@ public class MetricsBatchProcessor implements IMetricsBatchProcessor { private final @NotNull Queue 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(); @@ -65,6 +66,7 @@ public void add(final @NotNull SentryMetricsEvent metricsEvent) { } pendingCount.increment(); queue.offer(metricsEvent); + hasAcceptedItem = true; maybeSchedule(false); } @@ -72,14 +74,16 @@ 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; + } + // 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(); } } @@ -106,6 +110,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/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index d1cb38c6495..13a57ebe70a 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -2536,11 +2536,21 @@ class ScopesTest { } @Test - fun `when logging is not enabled, do nothing`() { + fun `when Logs option is omitted, capture manual log`() { val (sut, mockClient) = getEnabledScopes() sut.logger().warn("test message") - verify(mockClient, never()).captureLog(any(), anyOrNull()) + + verify(mockClient).captureLog(any(), anyOrNull()) + } + + @Test + fun `when Logs option is disabled, capture manual log`() { + val (sut, mockClient) = getEnabledScopes { it.logs.isEnabled = false } + + sut.logger().warn("test message") + + verify(mockClient).captureLog(any(), anyOrNull()) } @Test @@ -3316,11 +3326,12 @@ class ScopesTest { } @Test - fun `when metrics is not enabled, do nothing`() { + fun `when Metrics option is disabled, capture manual metric`() { val (sut, mockClient) = getEnabledScopes { it.metrics.isEnabled = false } sut.metrics().count("metric name") - verify(mockClient, never()).captureMetric(any(), anyOrNull(), anyOrNull()) + + verify(mockClient).captureMetric(any(), anyOrNull(), anyOrNull()) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 61181ee96a6..d9ed1df0f2d 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -153,6 +153,17 @@ class SentryClientTest { assertTrue(sut.isEnabled) } + @Test + fun `when Logs and Metrics options are disabled, batch processors are still created`() { + val sut = fixture.getSut { + it.logs.isEnabled = false + it.metrics.isEnabled = false + } + + verify(fixture.loggerBatchProcessorFactory).create(fixture.sentryOptions, sut) + verify(fixture.metricsBatchProcessorFactory).create(fixture.sentryOptions, sut) + } + @Test fun `when dsn is an invalid string, client throws`() { fixture.sentryOptions.dsn = "invalid-dsn" diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 3d91c0f8ead..72400f3e339 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -22,6 +22,7 @@ import io.sentry.test.NonOverridableNoOpSentryExecutorService import io.sentry.test.createSentryClientMock import io.sentry.test.initForTest import io.sentry.test.injectForField +import io.sentry.transport.ITransport import io.sentry.util.PlatformTestManipulator import io.sentry.util.thread.IThreadChecker import io.sentry.util.thread.ThreadChecker @@ -52,6 +53,8 @@ import org.junit.rules.TemporaryFolder import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.check import org.mockito.kotlin.eq import org.mockito.kotlin.inOrder @@ -174,6 +177,30 @@ class SentryTest { assertFalse(Sentry.getGlobalScope().client.isEnabled) } + @Test + fun `manual Logs and Metrics reach transport when automatic collection is disabled`() { + val transport = mock() + val transportFactory = mock() + whenever(transportFactory.create(any(), any())).thenReturn(transport) + initForTest { + it.dsn = dsn + it.logs.isEnabled = false + it.metrics.isEnabled = false + it.setTransportFactory(transportFactory) + it.shutdownTimeoutMillis = 5_000 + } + + Sentry.logger().info("manual log") + Sentry.metrics().count("manual metric") + Sentry.flush(5_000) + + val envelopes = argumentCaptor() + verify(transport, atLeastOnce()).send(envelopes.capture()) + val itemTypes = + envelopes.allValues.flatMap { envelope -> envelope.items.map { it.header.type } } + assertThat(itemTypes).containsAtLeast(SentryItemType.Log, SentryItemType.TraceMetric) + } + @Test fun `close calls scopes close with isRestarting false`() { val scopes = mock() diff --git a/sentry/src/test/java/io/sentry/logger/LoggerBatchProcessorTest.kt b/sentry/src/test/java/io/sentry/logger/LoggerBatchProcessorTest.kt index 01b8afdb4a3..28637b4f5e8 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 @@ -19,13 +20,40 @@ 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 `flush without accepted items does not submit processor work`() { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.flush(0) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `close without accepted items does not submit processor work`() { + for (isRestarting in listOf(false, true)) { + val mockExecutor = mock() + val processor = LoggerBatchProcessor(SentryOptions(), mock(), mockExecutor) + + processor.close(isRestarting) + + verify(mockExecutor).close(any()) + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + } + @Test fun `schedules another flush after previous flush has run`() { val mockClient = mock() diff --git a/sentry/src/test/java/io/sentry/metrics/MetricsBatchProcessorTest.kt b/sentry/src/test/java/io/sentry/metrics/MetricsBatchProcessorTest.kt index d8320d9b1a6..fafefa5e9fb 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 @@ -18,13 +19,42 @@ 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 `flush without accepted items does not submit processor work`() { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock()) + processor.injectForField("executorService", mockExecutor) + + processor.flush(0) + + verifyNoInteractions(mockExecutor) + } + + @Test + fun `close without accepted items does not submit processor work`() { + for (isRestarting in listOf(false, true)) { + val mockExecutor = mock() + val processor = MetricsBatchProcessor(SentryOptions(), mock()) + processor.injectForField("executorService", mockExecutor) + + processor.close(isRestarting) + + verify(mockExecutor).close(any()) + verify(mockExecutor, never()).schedule(any(), any()) + verify(mockExecutor, never()).submit(any()) + } + } + @Test fun `schedules another flush after previous flush has run`() { val mockClient = mock()