diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d1497bffe0..7df988a3684 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Continuous Profiling - stop when app goes in background ([#4311](https://github.com/getsentry/sentry-java/pull/4311)) - Continuous Profiling - Add delayed stop ([#4293](https://github.com/getsentry/sentry-java/pull/4293)) - Continuous Profiling - Out of Experimental ([#4310](https://github.com/getsentry/sentry-java/pull/4310)) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 3f31b51e646..c2b313b8e83 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -42,7 +42,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler, io/sentry/transport/RateLimiter$IRateLimitObserver { public fun (Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ILogger;Ljava/lang/String;ILio/sentry/ISentryExecutorService;)V - public fun close ()V + public fun close (Z)V public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun getRootSpanCounter ()I public fun isRunning ()Z diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java index 94be555104d..f3e8088760d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java @@ -322,12 +322,15 @@ public void reevaluateSampling() { shouldSample = true; } - public void close() { + @Override + public void close(final boolean isTerminating) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { rootSpanCounter = 0; shouldStop = true; - stop(false); - isClosed.set(true); + if (isTerminating) { + stop(false); + isClosed.set(true); + } } } 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 8230157a753..657e6d369ae 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 @@ -268,7 +268,7 @@ private static void setupProfiler( // This is a safeguard, but it should never happen, as the app start profiler should be the // continuous one. if (appStartContinuousProfiler != null) { - appStartContinuousProfiler.close(); + appStartContinuousProfiler.close(true); } if (appStartTransactionProfiler != null) { options.setTransactionProfiler(appStartTransactionProfiler); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index d83ecdd6752..89d78193207 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -122,6 +122,7 @@ public void run() { scopes.endSession(); } scopes.getOptions().getReplayController().stop(); + scopes.getOptions().getContinuousProfiler().close(false); } }; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 4b569cba6a8..3c162aab1ad 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -95,7 +95,7 @@ public void shutdown() { final @Nullable IContinuousProfiler appStartContinuousProfiler = AppStartMetrics.getInstance().getAppStartContinuousProfiler(); if (appStartContinuousProfiler != null) { - appStartContinuousProfiler.close(); + appStartContinuousProfiler.close(true); } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 0cc7cdca8e2..562c8949914 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -225,7 +225,7 @@ public void clear() { } appStartProfiler = null; if (appStartContinuousProfiler != null) { - appStartContinuousProfiler.close(); + appStartContinuousProfiler.close(true); } appStartContinuousProfiler = null; appStartSamplingDecision = null; @@ -333,7 +333,7 @@ private void checkCreateTimeOnMain() { appStartProfiler = null; } if (appStartContinuousProfiler != null && appStartContinuousProfiler.isRunning()) { - appStartContinuousProfiler.close(); + appStartContinuousProfiler.close(true); appStartContinuousProfiler = null; } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt index 4e1b45ebb02..db0964f15f4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt @@ -389,7 +389,7 @@ class AndroidContinuousProfilerTest { profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) - profiler.close() + profiler.close(true) assertFalse(profiler.isRunning) // The timeout scheduled job should be cleared @@ -470,6 +470,23 @@ class AndroidContinuousProfilerTest { verify(fixture.scopes).captureProfileChunk(any()) } + @Test + fun `close without terminating stops all profiles after chunk is finished`() { + val profiler = fixture.getSut() + profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + profiler.startProfiler(ProfileLifecycle.TRACE, fixture.mockTracesSampler) + assertTrue(profiler.isRunning) + // We are scheduling the profiler to stop at the end of the chunk, so it should still be running + profiler.close(false) + assertTrue(profiler.isRunning) + // However, close() already resets the rootSpanCounter + assertEquals(0, profiler.rootSpanCounter) + + // We run the executor service to trigger the chunk finish, and the profiler shouldn't restart + fixture.executor.runAll() + assertFalse(profiler.isRunning) + } + @Test fun `profiler does not send chunks after close`() { val profiler = fixture.getSut() @@ -477,7 +494,7 @@ class AndroidContinuousProfilerTest { assertTrue(profiler.isRunning) // We close the profiler, which should prevent sending additional chunks - profiler.close() + profiler.close(true) // The executor used to send the chunk doesn't do anything fixture.executor.runAll() 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 dfc88fa0e78..e5a2395feb3 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 @@ -446,7 +446,7 @@ class AndroidOptionsInitializerTest { assertEquals(fixture.sentryOptions.continuousProfiler, NoOpContinuousProfiler.getInstance()) // app start profiler is closed, because it will never be used - verify(appStartContinuousProfiler).close() + verify(appStartContinuousProfiler).close(eq(true)) // AppStartMetrics should be cleared assertNull(AppStartMetrics.getInstance().appStartProfiler) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt index 5f088221b9d..c1862b41f7b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt @@ -3,6 +3,7 @@ package io.sentry.android.core import androidx.lifecycle.LifecycleOwner import io.sentry.Breadcrumb import io.sentry.DateUtils +import io.sentry.IContinuousProfiler import io.sentry.IScope import io.sentry.IScopes import io.sentry.ReplayController @@ -15,6 +16,7 @@ import io.sentry.transport.ICurrentDateProvider import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.timeout @@ -38,6 +40,7 @@ class LifecycleWatcherTest { val dateProvider = mock() val options = SentryOptions() val replayController = mock() + val continuousProfiler = mock() fun getSUT( sessionIntervalMillis: Long = 0L, @@ -52,6 +55,7 @@ class LifecycleWatcherTest { argumentCaptor.value.run(scope) } options.setReplayController(replayController) + options.setContinuousProfiler(continuousProfiler) whenever(scopes.options).thenReturn(options) return LifecycleWatcher( @@ -106,6 +110,7 @@ class LifecycleWatcherTest { watcher.onStop(fixture.ownerMock) verify(fixture.scopes, timeout(10000)).endSession() verify(fixture.replayController, timeout(10000)).stop() + verify(fixture.continuousProfiler, timeout(10000)).close(eq(false)) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index f8734a26f00..114ea06d2d2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -16,6 +16,7 @@ import io.sentry.android.core.SentryAndroidOptions import io.sentry.android.core.SentryShadowProcess import org.junit.Before import org.junit.runner.RunWith +import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -273,7 +274,7 @@ class AppStartMetricsTest { // Job on main thread checks if activity was launched Shadows.shadowOf(Looper.getMainLooper()).idle() - verify(profiler).close() + verify(profiler).close(eq(true)) } @Test @@ -301,7 +302,7 @@ class AppStartMetricsTest { // Job on main thread checks if activity was launched Shadows.shadowOf(Looper.getMainLooper()).idle() - verify(profiler, never()).close() + verify(profiler, never()).close(any()) } @Test diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 15fb6dadffd..067fa327ed4 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -747,7 +747,7 @@ public abstract interface class io/sentry/IConnectionStatusProvider$IConnectionS } public abstract interface class io/sentry/IContinuousProfiler { - public abstract fun close ()V + public abstract fun close (Z)V public abstract fun getProfilerId ()Lio/sentry/protocol/SentryId; public abstract fun isRunning ()Z public abstract fun reevaluateSampling ()V @@ -1439,7 +1439,7 @@ public final class io/sentry/NoOpConnectionStatusProvider : io/sentry/IConnectio } public final class io/sentry/NoOpContinuousProfiler : io/sentry/IContinuousProfiler { - public fun close ()V + public fun close (Z)V public static fun getInstance ()Lio/sentry/NoOpContinuousProfiler; public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun isRunning ()Z diff --git a/sentry/src/main/java/io/sentry/IContinuousProfiler.java b/sentry/src/main/java/io/sentry/IContinuousProfiler.java index f423401c5d6..3abca9822aa 100644 --- a/sentry/src/main/java/io/sentry/IContinuousProfiler.java +++ b/sentry/src/main/java/io/sentry/IContinuousProfiler.java @@ -14,8 +14,12 @@ void startProfiler( void stopProfiler(final @NotNull ProfileLifecycle profileLifecycle); - /** Cancel the profiler and stops it. Used on SDK close. */ - void close(); + /** + * Cancel the profiler and stops it. + * + * @param isTerminating whether the profiler is terminating and won't be restarted or not. + */ + void close(final boolean isTerminating); void reevaluateSampling(); diff --git a/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java b/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java index 35bb0db5f05..893eb914ad9 100644 --- a/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java @@ -27,7 +27,7 @@ public void startProfiler( final @NotNull TracesSampler tracesSampler) {} @Override - public void close() {} + public void close(final boolean isTerminating) {} @Override public void reevaluateSampling() {} diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index b2c1bb1aaec..002043c3dfc 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -405,7 +405,7 @@ public void close(final boolean isRestarting) { configureScope(ScopeType.ISOLATION, scope -> scope.clear()); getOptions().getBackpressureMonitor().close(); getOptions().getTransactionProfiler().close(); - getOptions().getContinuousProfiler().close(); + getOptions().getContinuousProfiler().close(true); getOptions().getCompositePerformanceCollector().close(); final @NotNull ISentryExecutorService executorService = getOptions().getExecutorService(); if (isRestarting) { diff --git a/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt index de2dc7e4c48..559190004b8 100644 --- a/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt @@ -24,7 +24,7 @@ class NoOpContinuousProfilerTest { @Test fun `close does not throw`() = - profiler.close() + profiler.close(true) @Test fun `getProfilerId returns Empty SentryId`() { diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index d61ae59f60c..ebb84162240 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -1826,7 +1826,7 @@ class ScopesTest { verify(backpressureMonitorMock).close() verify(executor).close(any()) verify(profiler).close() - verify(continuousProfiler).close() + verify(continuousProfiler).close(eq(true)) verify(performanceCollector).close() }