From 170abc921699aa49a1e4c6380edcdeae393c0d5c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 13 Aug 2026 06:29:55 +0200 Subject: [PATCH 1/2] perf(core): Start Metrics batch worker on first use Avoid scheduling Metrics processor work for empty flush and close operations until the processor accepts its first item. Preserve existing batching, restart, and Android background behavior after first use. Co-Authored-By: Claude --- .../core/AndroidMetricsBatchProcessorTest.kt | 14 +++ sentry/api/sentry.api | 1 + .../sentry/metrics/MetricsBatchProcessor.java | 30 ++++-- .../metrics/MetricsBatchProcessorTest.kt | 91 +++++++++++++++++++ 4 files changed, 129 insertions(+), 7 deletions(-) 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/api/sentry.api b/sentry/api/sentry.api index 5041ecf13f8..d595cd603a0 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -5490,6 +5490,7 @@ public class io/sentry/metrics/MetricsBatchProcessor : io/sentry/metrics/IMetric public static final field MAX_QUEUE_SIZE I 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 public fun add (Lio/sentry/SentryMetricsEvent;)V public fun close (Z)V public fun flush (J)V diff --git a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java index 3c744dbe3c5..e2e71d1daa3 100644 --- a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java +++ b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java @@ -19,8 +19,10 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; @Open public class MetricsBatchProcessor implements IMetricsBatchProcessor { @@ -34,16 +36,26 @@ 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)); + } + + @ApiStatus.Internal + @TestOnly + public 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 +77,7 @@ public void add(final @NotNull SentryMetricsEvent metricsEvent) { } pendingCount.increment(); queue.offer(metricsEvent); + hasAcceptedItem = true; maybeSchedule(false); } @@ -72,14 +85,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 +119,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/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 From b87e3315f048e14126345fcbcf9610661f31e935 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 13 Aug 2026 12:20:36 +0200 Subject: [PATCH 2/2] fix(core): Keep Metrics test constructor internal Use package visibility for executor injection so the test-only constructor does not expand the published API. Co-Authored-By: Claude --- sentry/api/sentry.api | 1 - .../main/java/io/sentry/metrics/MetricsBatchProcessor.java | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index d595cd603a0..5041ecf13f8 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -5490,7 +5490,6 @@ public class io/sentry/metrics/MetricsBatchProcessor : io/sentry/metrics/IMetric public static final field MAX_QUEUE_SIZE I 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 public fun add (Lio/sentry/SentryMetricsEvent;)V public fun close (Z)V public fun flush (J)V diff --git a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java index e2e71d1daa3..8df9dc653f4 100644 --- a/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java +++ b/sentry/src/main/java/io/sentry/metrics/MetricsBatchProcessor.java @@ -19,10 +19,8 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.TestOnly; @Open public class MetricsBatchProcessor implements IMetricsBatchProcessor { @@ -46,9 +44,7 @@ public MetricsBatchProcessor( this(options, client, new SentryExecutorService(options)); } - @ApiStatus.Internal - @TestOnly - public MetricsBatchProcessor( + MetricsBatchProcessor( final @NotNull SentryOptions options, final @NotNull ISentryClient client, final @NotNull ISentryExecutorService executorService) {