diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b65f0b74f..d09834301cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ - Previously only the body was cached which could lead to problems in the FilterChain as Request parameters were not available - We now hold a strong reference to the underlying OpenTelemetry span when it is created through Sentry API ([#3997](https://github.com/getsentry/sentry-java/pull/3997)) - This keeps it from being garbage collected too early +- Close backpressure monitor on SDK shutdown ([#3998](https://github.com/getsentry/sentry-java/pull/3998)) + - Due to the backpressure monitor rescheduling a task to run every 10s, it very likely caused shutdown to wait the full `shutdownTimeoutMillis` (defaulting to 2s) instead of being able to terminate immediately ## 8.0.0-rc.2 diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index ef446fd35b7..ea40e22e554 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3790,17 +3790,20 @@ public final class io/sentry/UserFeedback$JsonKeys { public final class io/sentry/backpressure/BackpressureMonitor : io/sentry/backpressure/IBackpressureMonitor, java/lang/Runnable { public fun (Lio/sentry/SentryOptions;Lio/sentry/IScopes;)V + public fun close ()V public fun getDownsampleFactor ()I public fun run ()V public fun start ()V } public abstract interface class io/sentry/backpressure/IBackpressureMonitor { + public abstract fun close ()V public abstract fun getDownsampleFactor ()I public abstract fun start ()V } public final class io/sentry/backpressure/NoOpBackpressureMonitor : io/sentry/backpressure/IBackpressureMonitor { + public fun close ()V public fun getDownsampleFactor ()I public static fun getInstance ()Lio/sentry/backpressure/NoOpBackpressureMonitor; public fun start ()V diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index 67b38f48bc3..2b0d5103686 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -403,6 +403,7 @@ public void close(final boolean isRestarting) { configureScope(scope -> scope.clear()); configureScope(ScopeType.ISOLATION, scope -> scope.clear()); + getOptions().getBackpressureMonitor().close(); getOptions().getTransactionProfiler().close(); getOptions().getTransactionPerformanceCollector().close(); final @NotNull ISentryExecutorService executorService = getOptions().getExecutorService(); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index b0dc1b112d6..dbbf2a0b318 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.backpressure.BackpressureMonitor; +import io.sentry.backpressure.NoOpBackpressureMonitor; import io.sentry.cache.EnvelopeCache; import io.sentry.cache.IEnvelopeCache; import io.sentry.config.PropertiesProviderFactory; @@ -599,7 +600,10 @@ private static void initConfigurations(final @NotNull SentryOptions options) { } if (options.isEnableBackpressureHandling() && Platform.isJvm()) { - options.setBackpressureMonitor(new BackpressureMonitor(options, ScopesAdapter.getInstance())); + if (options.getBackpressureMonitor() instanceof NoOpBackpressureMonitor) { + options.setBackpressureMonitor( + new BackpressureMonitor(options, ScopesAdapter.getInstance())); + } options.getBackpressureMonitor().start(); } } diff --git a/sentry/src/main/java/io/sentry/backpressure/BackpressureMonitor.java b/sentry/src/main/java/io/sentry/backpressure/BackpressureMonitor.java index 6541a7586a5..92fa3ce0a5c 100644 --- a/sentry/src/main/java/io/sentry/backpressure/BackpressureMonitor.java +++ b/sentry/src/main/java/io/sentry/backpressure/BackpressureMonitor.java @@ -2,9 +2,13 @@ import io.sentry.IScopes; import io.sentry.ISentryExecutorService; +import io.sentry.ISentryLifecycleToken; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.util.AutoClosableReentrantLock; +import java.util.concurrent.Future; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public final class BackpressureMonitor implements IBackpressureMonitor, Runnable { static final int MAX_DOWNSAMPLE_FACTOR = 10; @@ -14,6 +18,8 @@ public final class BackpressureMonitor implements IBackpressureMonitor, Runnable private final @NotNull SentryOptions sentryOptions; private final @NotNull IScopes scopes; private int downsampleFactor = 0; + private volatile @Nullable Future latestScheduledRun = null; + private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); public BackpressureMonitor( final @NotNull SentryOptions sentryOptions, final @NotNull IScopes scopes) { @@ -37,6 +43,16 @@ public int getDownsampleFactor() { return downsampleFactor; } + @Override + public void close() { + final @Nullable Future currentRun = latestScheduledRun; + if (currentRun != null) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + currentRun.cancel(true); + } + } + } + void checkHealth() { if (isHealthy()) { if (downsampleFactor > 0) { @@ -62,7 +78,9 @@ void checkHealth() { private void reschedule(final int delay) { final @NotNull ISentryExecutorService executorService = sentryOptions.getExecutorService(); if (!executorService.isClosed()) { - executorService.schedule(this, delay); + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + latestScheduledRun = executorService.schedule(this, delay); + } } } diff --git a/sentry/src/main/java/io/sentry/backpressure/IBackpressureMonitor.java b/sentry/src/main/java/io/sentry/backpressure/IBackpressureMonitor.java index 05cf681950c..5577d67e3a5 100644 --- a/sentry/src/main/java/io/sentry/backpressure/IBackpressureMonitor.java +++ b/sentry/src/main/java/io/sentry/backpressure/IBackpressureMonitor.java @@ -7,4 +7,6 @@ public interface IBackpressureMonitor { void start(); int getDownsampleFactor(); + + void close(); } diff --git a/sentry/src/main/java/io/sentry/backpressure/NoOpBackpressureMonitor.java b/sentry/src/main/java/io/sentry/backpressure/NoOpBackpressureMonitor.java index edbf660e24e..14a0db322c2 100644 --- a/sentry/src/main/java/io/sentry/backpressure/NoOpBackpressureMonitor.java +++ b/sentry/src/main/java/io/sentry/backpressure/NoOpBackpressureMonitor.java @@ -19,4 +19,9 @@ public void start() { public int getDownsampleFactor() { return 0; } + + @Override + public void close() { + // do nothing + } } diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index 06386b39dde..91ad296e060 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -1760,15 +1760,18 @@ class ScopesTest { val executor = mock() val profiler = mock() val performanceCollector = mock() + val backpressureMonitorMock = mock() val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" cacheDirPath = file.absolutePath executorService = executor setTransactionProfiler(profiler) transactionPerformanceCollector = performanceCollector + backpressureMonitor = backpressureMonitorMock } val sut = createScopes(options) sut.close() + verify(backpressureMonitorMock).close() verify(executor).close(any()) verify(profiler).close() verify(performanceCollector).close() diff --git a/sentry/src/test/java/io/sentry/backpressure/BackpressureMonitorTest.kt b/sentry/src/test/java/io/sentry/backpressure/BackpressureMonitorTest.kt index cf234574bf3..690ba54c147 100644 --- a/sentry/src/test/java/io/sentry/backpressure/BackpressureMonitorTest.kt +++ b/sentry/src/test/java/io/sentry/backpressure/BackpressureMonitorTest.kt @@ -4,8 +4,10 @@ import io.sentry.IScopes import io.sentry.ISentryExecutorService import io.sentry.SentryOptions import io.sentry.backpressure.BackpressureMonitor.MAX_DOWNSAMPLE_FACTOR +import org.mockito.ArgumentMatchers.eq 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 java.util.concurrent.Future @@ -19,10 +21,12 @@ class BackpressureMonitorTest { val options = SentryOptions() val scopes = mock() val executor = mock() + val returnedFuture = mock>() fun getSut(): BackpressureMonitor { options.executorService = executor whenever(executor.isClosed).thenReturn(false) - whenever(executor.schedule(any(), any())).thenReturn(mock>()) + whenever(executor.schedule(any(), any())).thenReturn(returnedFuture) + whenever(returnedFuture.cancel(any())).thenReturn(true) return BackpressureMonitor(options, scopes) } } @@ -80,4 +84,17 @@ class BackpressureMonitorTest { verify(fixture.executor).schedule(any(), any()) } + + @Test + fun `close cancels latest job`() { + val sut = fixture.getSut() + sut.run() + + verify(fixture.executor).schedule(any(), any()) + verify(fixture.returnedFuture, never()).cancel(any()) + + sut.close() + + verify(fixture.returnedFuture).cancel(eq(true)) + } }