From e6ffd7b0865959ede648a24bb9735c99a97fd54f Mon Sep 17 00:00:00 2001 From: Stefano Date: Mon, 27 Nov 2023 19:31:52 +0100 Subject: [PATCH] Reduce main thread work on init (#3036) * delete old profiles in the background on SDK init * moved some Integration.register() to the background using executorService * Move integrations registration to background during init (#3043) * removed integrations registration falling back to calling thread --- CHANGELOG.md | 7 ++ .../sentry/android/core/AnrIntegration.java | 67 +++++++++++++------ .../core/EnvelopeFileObserverIntegration.java | 59 +++++++++++----- .../PhoneStateBreadcrumbsIntegration.java | 60 ++++++++++++----- .../core/SendCachedEnvelopeIntegration.java | 17 ++--- .../SystemEventsBreadcrumbsIntegration.java | 55 +++++++++++---- .../TempSensorBreadcrumbsIntegration.java | 63 +++++++++++------ .../core/ActivityLifecycleIntegrationTest.kt | 38 +++-------- .../sentry/android/core/AnrIntegrationTest.kt | 28 +++++++- .../EnvelopeFileObserverIntegrationTest.kt | 67 +++++++++++++++++++ .../PhoneStateBreadcrumbsIntegrationTest.kt | 52 +++++++++----- .../core/SendCachedEnvelopeIntegrationTest.kt | 20 +++++- .../sentry/android/core/SentryAndroidTest.kt | 3 +- .../SystemEventsBreadcrumbsIntegrationTest.kt | 26 ++++++- .../TempSensorBreadcrumbsIntegrationTest.kt | 63 ++++++++++++----- .../okhttp/SentryOkHttpEventListenerTest.kt | 15 ++--- .../android/okhttp/SentryOkHttpEventTest.kt | 11 +-- .../api/sentry-test-support.api | 12 ++++ .../src/main/kotlin/io/sentry/test/Mocks.kt | 29 +++++++- .../main/kotlin/io/sentry/test/Reflection.kt | 7 ++ ...achedEnvelopeFireAndForgetIntegration.java | 13 ++-- sentry/src/main/java/io/sentry/Sentry.java | 7 +- ...aultTransactionPerformanceCollectorTest.kt | 21 ++---- ...hedEnvelopeFireAndForgetIntegrationTest.kt | 21 ++++++ sentry/src/test/java/io/sentry/SentryTest.kt | 36 ++++++++++ 25 files changed, 588 insertions(+), 209 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2f8f969fb..e460c97ef7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +### Fixes + +- Reduce main thread work on init ([#3036](https://github.com/getsentry/sentry-java/pull/3036)) +- Move Integrations registration to background on init ([#3043](https://github.com/getsentry/sentry-java/pull/3043)) + ## 6.34.0 ### Features diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java index 4c6ac6373a..0eff83e83e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java @@ -27,6 +27,8 @@ public final class AnrIntegration implements Integration, Closeable { private final @NotNull Context context; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public AnrIntegration(final @NotNull Context context) { this.context = context; @@ -55,27 +57,47 @@ private void register(final @NotNull IHub hub, final @NotNull SentryAndroidOptio .log(SentryLevel.DEBUG, "AnrIntegration enabled: %s", options.isAnrEnabled()); if (options.isAnrEnabled()) { - synchronized (watchDogLock) { - if (anrWatchDog == null) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "ANR timeout in milliseconds: %d", - options.getAnrTimeoutIntervalMillis()); - - anrWatchDog = - new ANRWatchDog( - options.getAnrTimeoutIntervalMillis(), - options.isAnrReportInDebug(), - error -> reportANR(hub, options, error), - options.getLogger(), - context); - anrWatchDog.start(); - - options.getLogger().log(SentryLevel.DEBUG, "AnrIntegration installed."); - addIntegrationToSdkVersion(); - } + addIntegrationToSdkVersion(); + try { + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startAnrWatchdog(hub, options); + } + } + }); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.DEBUG, "Failed to start AnrIntegration on executor thread.", e); + } + } + } + + private void startAnrWatchdog( + final @NotNull IHub hub, final @NotNull SentryAndroidOptions options) { + synchronized (watchDogLock) { + if (anrWatchDog == null) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "ANR timeout in milliseconds: %d", + options.getAnrTimeoutIntervalMillis()); + + anrWatchDog = + new ANRWatchDog( + options.getAnrTimeoutIntervalMillis(), + options.isAnrReportInDebug(), + error -> reportANR(hub, options, error), + options.getLogger(), + context); + anrWatchDog.start(); + + options.getLogger().log(SentryLevel.DEBUG, "AnrIntegration installed."); } } } @@ -126,6 +148,9 @@ ANRWatchDog getANRWatchDog() { @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } synchronized (watchDogLock) { if (anrWatchDog != null) { anrWatchDog.interrupt(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java index 140b944982..fa8ba7ff16 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java @@ -16,6 +16,8 @@ public abstract class EnvelopeFileObserverIntegration implements Integration, Closeable { private @Nullable EnvelopeFileObserver observer; private @Nullable ILogger logger; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public static @NotNull EnvelopeFileObserverIntegration getOutboxFileObserver() { return new OutboxEnvelopeFileObserverIntegration(); @@ -37,30 +39,55 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions logger.log( SentryLevel.DEBUG, "Registering EnvelopeFileObserverIntegration for path: %s", path); - final OutboxSender outboxSender = - new OutboxSender( - hub, - options.getEnvelopeReader(), - options.getSerializer(), - logger, - options.getFlushTimeoutMillis()); - - observer = - new EnvelopeFileObserver(path, outboxSender, logger, options.getFlushTimeoutMillis()); try { - observer.startWatching(); - logger.log(SentryLevel.DEBUG, "EnvelopeFileObserverIntegration installed."); - } catch (Throwable e) { - // it could throw eg NoSuchFileException or NullPointerException options - .getLogger() - .log(SentryLevel.ERROR, "Failed to initialize EnvelopeFileObserverIntegration.", e); + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startOutboxSender(hub, options, path); + } + } + }); + } catch (Throwable e) { + logger.log( + SentryLevel.DEBUG, + "Failed to start EnvelopeFileObserverIntegration on executor thread.", + e); } } } + private void startOutboxSender( + final @NotNull IHub hub, final @NotNull SentryOptions options, final @NotNull String path) { + final OutboxSender outboxSender = + new OutboxSender( + hub, + options.getEnvelopeReader(), + options.getSerializer(), + options.getLogger(), + options.getFlushTimeoutMillis()); + + observer = + new EnvelopeFileObserver( + path, outboxSender, options.getLogger(), options.getFlushTimeoutMillis()); + try { + observer.startWatching(); + options.getLogger().log(SentryLevel.DEBUG, "EnvelopeFileObserverIntegration installed."); + } catch (Throwable e) { + // it could throw eg NoSuchFileException or NullPointerException + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to initialize EnvelopeFileObserverIntegration.", e); + } + } + @Override public void close() { + synchronized (startLock) { + isClosed = true; + } if (observer != null) { observer.stopWatching(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java index be23c668c7..cef16d7f27 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java @@ -23,12 +23,13 @@ public final class PhoneStateBreadcrumbsIntegration implements Integration, Clos private @Nullable SentryAndroidOptions options; @TestOnly @Nullable PhoneStateChangeListener listener; private @Nullable TelephonyManager telephonyManager; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public PhoneStateBreadcrumbsIntegration(final @NotNull Context context) { this.context = Objects.requireNonNull(context, "Context is required"); } - @SuppressWarnings("deprecation") @Override public void register(final @NotNull IHub hub, final @NotNull SentryOptions options) { Objects.requireNonNull(hub, "Hub is required"); @@ -46,28 +47,55 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio if (this.options.isEnableSystemEventBreadcrumbs() && Permissions.hasPermission(context, READ_PHONE_STATE)) { - telephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE); - if (telephonyManager != null) { - try { - listener = new PhoneStateChangeListener(hub); - telephonyManager.listen(listener, android.telephony.PhoneStateListener.LISTEN_CALL_STATE); - - options.getLogger().log(SentryLevel.DEBUG, "PhoneStateBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion(); - } catch (Throwable e) { - this.options - .getLogger() - .log(SentryLevel.INFO, e, "TelephonyManager is not available or ready to use."); - } - } else { - this.options.getLogger().log(SentryLevel.INFO, "TelephonyManager is not available"); + try { + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startTelephonyListener(hub, options); + } + } + }); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Failed to start PhoneStateBreadcrumbsIntegration on executor thread.", + e); + } + } + } + + @SuppressWarnings("deprecation") + private void startTelephonyListener( + final @NotNull IHub hub, final @NotNull SentryOptions options) { + telephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE); + if (telephonyManager != null) { + try { + listener = new PhoneStateChangeListener(hub); + telephonyManager.listen(listener, android.telephony.PhoneStateListener.LISTEN_CALL_STATE); + + options.getLogger().log(SentryLevel.DEBUG, "PhoneStateBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion(); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.INFO, e, "TelephonyManager is not available or ready to use."); } + } else { + options.getLogger().log(SentryLevel.INFO, "TelephonyManager is not available"); } } @SuppressWarnings("deprecation") @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } if (telephonyManager != null && listener != null) { telephonyManager.listen(listener, android.telephony.PhoneStateListener.LISTEN_NONE); listener = null; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java index e0d08325b4..e201d8535e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -40,20 +40,21 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { return; } - final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender = - factory.create(hub, androidOptions); - - if (sender == null) { - androidOptions.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); - return; - } - try { Future future = androidOptions .getExecutorService() .submit( () -> { + final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender = + factory.create(hub, androidOptions); + + if (sender == null) { + androidOptions + .getLogger() + .log(SentryLevel.ERROR, "SendFireAndForget factory is null."); + return; + } try { sender.send(); } catch (Throwable e) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 9f7343fc8f..77440dcf59 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -65,6 +65,8 @@ public final class SystemEventsBreadcrumbsIntegration implements Integration, Cl private @Nullable SentryAndroidOptions options; private final @NotNull List actions; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) { this(context, getDefaultActions()); @@ -92,27 +94,49 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio this.options.isEnableSystemEventBreadcrumbs()); if (this.options.isEnableSystemEventBreadcrumbs()) { - receiver = new SystemEventsBroadcastReceiver(hub, this.options.getLogger()); - final IntentFilter filter = new IntentFilter(); - for (String item : actions) { - filter.addAction(item); - } + try { - // registerReceiver can throw SecurityException but it's not documented in the official docs - ContextUtils.registerReceiver(context, options, receiver, filter); - this.options - .getLogger() - .log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion(); + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startSystemEventsReceiver(hub, (SentryAndroidOptions) options); + } + } + }); } catch (Throwable e) { - this.options.setEnableSystemEventBreadcrumbs(false); - this.options + options .getLogger() - .log(SentryLevel.ERROR, "Failed to initialize SystemEventsBreadcrumbsIntegration.", e); + .log( + SentryLevel.DEBUG, + "Failed to start SystemEventsBreadcrumbsIntegration on executor thread.", + e); } } } + private void startSystemEventsReceiver( + final @NotNull IHub hub, final @NotNull SentryAndroidOptions options) { + receiver = new SystemEventsBroadcastReceiver(hub, options.getLogger()); + final IntentFilter filter = new IntentFilter(); + for (String item : actions) { + filter.addAction(item); + } + try { + // registerReceiver can throw SecurityException but it's not documented in the official docs + ContextUtils.registerReceiver(context, options, receiver, filter); + options.getLogger().log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion(); + } catch (Throwable e) { + options.setEnableSystemEventBreadcrumbs(false); + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to initialize SystemEventsBreadcrumbsIntegration.", e); + } + } + @SuppressWarnings("deprecation") private static @NotNull List getDefaultActions() { final List actions = new ArrayList<>(); @@ -164,6 +188,9 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } if (receiver != null) { context.unregisterReceiver(receiver); receiver = null; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java index e4577b43ec..22df7b27e3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java @@ -29,12 +29,13 @@ public final class TempSensorBreadcrumbsIntegration private @Nullable SentryAndroidOptions options; @TestOnly @Nullable SensorManager sensorManager; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public TempSensorBreadcrumbsIntegration(final @NotNull Context context) { this.context = Objects.requireNonNull(context, "Context is required"); } - @SuppressWarnings("deprecation") @Override public void register(final @NotNull IHub hub, final @NotNull SentryOptions options) { this.hub = Objects.requireNonNull(hub, "Hub is required"); @@ -51,34 +52,56 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio this.options.isEnableSystemEventBreadcrumbs()); if (this.options.isEnableSystemEventBreadcrumbs()) { + try { - sensorManager = (SensorManager) context.getSystemService(SENSOR_SERVICE); - if (sensorManager != null) { - final Sensor defaultSensor = - sensorManager.getDefaultSensor(Sensor.TYPE_AMBIENT_TEMPERATURE); - if (defaultSensor != null) { - sensorManager.registerListener(this, defaultSensor, SensorManager.SENSOR_DELAY_NORMAL); - - options - .getLogger() - .log(SentryLevel.DEBUG, "TempSensorBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion(); - } else { - this.options - .getLogger() - .log(SentryLevel.INFO, "TYPE_AMBIENT_TEMPERATURE is not available."); - } + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startSensorListener(options); + } + } + }); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Failed to start TempSensorBreadcrumbsIntegration on executor thread.", + e); + } + } + } + + private void startSensorListener(final @NotNull SentryOptions options) { + try { + sensorManager = (SensorManager) context.getSystemService(SENSOR_SERVICE); + if (sensorManager != null) { + final Sensor defaultSensor = + sensorManager.getDefaultSensor(Sensor.TYPE_AMBIENT_TEMPERATURE); + if (defaultSensor != null) { + sensorManager.registerListener(this, defaultSensor, SensorManager.SENSOR_DELAY_NORMAL); + + options.getLogger().log(SentryLevel.DEBUG, "TempSensorBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion(); } else { - this.options.getLogger().log(SentryLevel.INFO, "SENSOR_SERVICE is not available."); + options.getLogger().log(SentryLevel.INFO, "TYPE_AMBIENT_TEMPERATURE is not available."); } - } catch (Throwable e) { - options.getLogger().log(SentryLevel.ERROR, e, "Failed to init. the SENSOR_SERVICE."); + } else { + options.getLogger().log(SentryLevel.INFO, "SENSOR_SERVICE is not available."); } + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, e, "Failed to init. the SENSOR_SERVICE."); } } @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } if (sensorManager != null) { sensorManager.unregisterListener(this); sensorManager = null; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 26bd42e5ab..8cb1a9db50 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -14,7 +14,6 @@ import io.sentry.Breadcrumb import io.sentry.DateUtils import io.sentry.FullyDisplayedReporter import io.sentry.Hub -import io.sentry.ISentryExecutorService import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.Sentry @@ -32,6 +31,7 @@ import io.sentry.TransactionFinishedCallback import io.sentry.TransactionOptions import io.sentry.protocol.MeasurementValue import io.sentry.protocol.TransactionNameSource +import io.sentry.test.DeferredExecutorService import io.sentry.test.getProperty import org.junit.runner.RunWith import org.mockito.ArgumentCaptor @@ -48,9 +48,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import java.util.Date -import java.util.concurrent.Callable import java.util.concurrent.Future -import java.util.concurrent.FutureTask import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -1096,20 +1094,10 @@ class ActivityLifecycleIntegrationTest { @Test fun `When isEnableTimeToFullDisplayTracing is true and reportFullyDrawn is not called, ttfd span is finished automatically with timeout`() { val sut = fixture.getSut() - var lastScheduledRunnable: Runnable? = null - val mockExecutorService = object : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = mock() - override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - lastScheduledRunnable = runnable - return FutureTask {} - } - override fun close(timeoutMillis: Long) {} - override fun isClosed() = false - } + val deferredExecutorService = DeferredExecutorService() fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true - fixture.options.executorService = mockExecutorService + fixture.options.executorService = deferredExecutorService sut.register(fixture.hub, fixture.options) val activity = mock() sut.onActivityCreated(activity, fixture.bundle) @@ -1118,10 +1106,10 @@ class ActivityLifecycleIntegrationTest { // Assert the ttfd span is running and a timeout autoCancel task has been scheduled assertNotNull(ttfdSpan) assertFalse(ttfdSpan.isFinished) - assertNotNull(lastScheduledRunnable) + assertTrue(deferredExecutorService.scheduledRunnables.isNotEmpty()) // Run the autoClose task and assert the ttfd span is finished with deadlineExceeded - lastScheduledRunnable!!.run() + deferredExecutorService.runAll() assertTrue(ttfdSpan.isFinished) assertEquals(SpanStatus.DEADLINE_EXCEEDED, ttfdSpan.status) @@ -1323,20 +1311,10 @@ class ActivityLifecycleIntegrationTest { val sut = fixture.getSut() val activity = mock() val view = fixture.createView() - var lastScheduledRunnable: Runnable? = null - val mockExecutorService = object : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = mock() - override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - lastScheduledRunnable = runnable - return FutureTask {} - } - override fun close(timeoutMillis: Long) {} - override fun isClosed() = false - } + val deferredExecutorService = DeferredExecutorService() fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true - fixture.options.executorService = mockExecutorService + fixture.options.executorService = deferredExecutorService sut.register(fixture.hub, fixture.options) sut.onActivityCreated(activity, fixture.bundle) sut.onActivityResumed(activity) @@ -1355,7 +1333,7 @@ class ActivityLifecycleIntegrationTest { // Run the autoClose task 1 ms after finishing the ttid span and assert the ttfd span is finished Thread.sleep(1) - lastScheduledRunnable!!.run() + deferredExecutorService.runAll() assertTrue(ttfdSpan.isFinished) // the ttfd span should be trimmed to be equal to the ttid span, and the description should end with "-exceeded" diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt index 55386b1c7a..cceabc9774 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt @@ -6,6 +6,8 @@ import io.sentry.IHub import io.sentry.SentryLevel import io.sentry.android.core.AnrIntegration.AnrHint import io.sentry.exception.ExceptionMechanismException +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService import io.sentry.util.HintUtils import org.mockito.kotlin.any import org.mockito.kotlin.check @@ -23,7 +25,7 @@ class AnrIntegrationTest { private class Fixture { val context = mock() val hub = mock() - val options = SentryAndroidOptions().apply { + var options: SentryAndroidOptions = SentryAndroidOptions().apply { setLogger(mock()) } @@ -44,6 +46,7 @@ class AnrIntegrationTest { @Test fun `When ANR is enabled, ANR watch dog should be started`() { + fixture.options.executorService = ImmediateExecutorService() val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) @@ -52,6 +55,16 @@ class AnrIntegrationTest { assertTrue((sut.anrWatchDog as ANRWatchDog).isAlive) } + @Test + fun `ANR watch dog should be started in the executorService`() { + fixture.options.executorService = mock() + val sut = fixture.getSut() + + sut.register(fixture.hub, fixture.options) + + assertNull(sut.anrWatchDog) + } + @Test fun `When ANR is disabled, ANR should not be started`() { val sut = fixture.getSut() @@ -82,6 +95,7 @@ class AnrIntegrationTest { @Test fun `When ANR integration is closed, watch dog should stop`() { val sut = fixture.getSut() + fixture.options.executorService = ImmediateExecutorService() sut.register(fixture.hub, fixture.options) @@ -92,6 +106,18 @@ class AnrIntegrationTest { assertNull(sut.anrWatchDog) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut() + fixture.options.executorService = deferredExecutorService + sut.register(fixture.hub, fixture.options) + assertNull(sut.anrWatchDog) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.anrWatchDog) + } + @Test fun `When ANR watch dog is triggered, constructs exception with proper mechanism and snapshot flag`() { val sut = fixture.getSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt index 2fcc6415e4..699fa2d2f2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt @@ -2,10 +2,18 @@ package io.sentry.android.core import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Hub +import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.SentryLevel import io.sentry.SentryOptions +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService import org.junit.runner.RunWith +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import java.io.File import java.nio.file.Files import kotlin.test.AfterTest @@ -15,6 +23,25 @@ import kotlin.test.assertEquals @RunWith(AndroidJUnit4::class) class EnvelopeFileObserverIntegrationTest { + inner class Fixture { + val hub: IHub = mock() + private lateinit var options: SentryAndroidOptions + val logger = mock() + + fun getSut(optionConfiguration: (SentryAndroidOptions) -> Unit = {}): EnvelopeFileObserverIntegration { + options = SentryAndroidOptions() + options.setLogger(logger) + options.isDebug = true + optionConfiguration(options) + whenever(hub.options).thenReturn(options) + + return object : EnvelopeFileObserverIntegration() { + override fun getPath(options: SentryOptions): String? = file.absolutePath + } + } + } + + private val fixture = Fixture() private lateinit var file: File @@ -51,4 +78,44 @@ class EnvelopeFileObserverIntegrationTest { hub.close() verify(integrationMock).close() } + + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val integration = fixture.getSut { + it.executorService = deferredExecutorService + } + integration.register(fixture.hub, fixture.hub.options) + integration.close() + deferredExecutorService.runAll() + verify(fixture.logger, never()).log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed.")) + } + + @Test + fun `register with fake executor service does not install integration`() { + val integration = fixture.getSut { + it.executorService = mock() + } + integration.register(fixture.hub, fixture.hub.options) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("Registering EnvelopeFileObserverIntegration for path: %s"), + eq(file.absolutePath) + ) + verify(fixture.logger, never()).log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed.")) + } + + @Test + fun `register integration on the background via executor service`() { + val integration = fixture.getSut { + it.executorService = ImmediateExecutorService() + } + integration.register(fixture.hub, fixture.hub.options) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("Registering EnvelopeFileObserverIntegration for path: %s"), + eq(file.absolutePath) + ) + verify(fixture.logger).log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed.")) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt index 00bddf7a6a..2b6ca801da 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt @@ -5,7 +5,10 @@ import android.telephony.PhoneStateListener import android.telephony.TelephonyManager import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.check import org.mockito.kotlin.eq @@ -23,8 +26,10 @@ class PhoneStateBreadcrumbsIntegrationTest { private class Fixture { val context = mock() val manager = mock() + val options = SentryAndroidOptions() - fun getSut(): PhoneStateBreadcrumbsIntegration { + fun getSut(executorService: ISentryExecutorService = ImmediateExecutorService()): PhoneStateBreadcrumbsIntegration { + options.executorService = executorService whenever(context.getSystemService(eq(Context.TELEPHONY_SERVICE))).thenReturn(manager) return PhoneStateBreadcrumbsIntegration(context) @@ -36,21 +41,31 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When system events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) verify(fixture.manager).listen(any(), eq(PhoneStateListener.LISTEN_CALL_STATE)) assertNotNull(sut.listener) } + @Test + fun `Phone state callback is registered in the executorService`() { + val sut = fixture.getSut(mock()) + val hub = mock() + sut.register(hub, fixture.options) + + assertNull(sut.listener) + } + @Test fun `When system events breadcrumb is disabled, it doesn't register callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions().apply { - isEnableSystemEventBreadcrumbs = false - } val hub = mock() - sut.register(hub, options) + sut.register( + hub, + fixture.options.apply { + isEnableSystemEventBreadcrumbs = false + } + ) verify(fixture.manager, never()).listen(any(), any()) assertNull(sut.listener) } @@ -58,20 +73,29 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When ActivityBreadcrumbsIntegration is closed, it should unregister the callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.close() verify(fixture.manager).listen(any(), eq(PhoneStateListener.LISTEN_NONE)) assertNull(sut.listener) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut(executorService = deferredExecutorService) + sut.register(mock(), fixture.options) + assertNull(sut.listener) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.listener) + } + @Test fun `When on call state received, added breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.listener!!.onCallStateChanged(TelephonyManager.CALL_STATE_RINGING, null) verify(hub).addBreadcrumb( @@ -87,9 +111,8 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When on idle state received, added breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.listener!!.onCallStateChanged(TelephonyManager.CALL_STATE_IDLE, null) verify(hub, never()).addBreadcrumb(any()) } @@ -97,9 +120,8 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When on offhook state received, added breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.listener!!.onCallStateChanged(TelephonyManager.CALL_STATE_OFFHOOK, null) verify(hub, never()).addBreadcrumb(any()) } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt index 36094f78eb..1fa08986b4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt @@ -2,9 +2,12 @@ package io.sentry.android.core import io.sentry.IHub import io.sentry.ILogger +import io.sentry.ISentryExecutorService import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory import io.sentry.SentryLevel.DEBUG +import io.sentry.SentryOptions +import io.sentry.test.ImmediateExecutorService import io.sentry.util.LazyEvaluator import org.awaitility.kotlin.await import org.mockito.kotlin.any @@ -32,11 +35,13 @@ class SendCachedEnvelopeIntegrationTest { hasStartupCrashMarker: Boolean = false, hasSender: Boolean = true, delaySend: Long = 0L, - taskFails: Boolean = false + taskFails: Boolean = false, + mockExecutorService: ISentryExecutorService? = null ): SendCachedEnvelopeIntegration { options.cacheDirPath = cacheDirPath options.setLogger(logger) options.isDebug = true + options.executorService = mockExecutorService ?: SentryOptions().executorService whenever(sender.send()).then { Thread.sleep(delaySend) @@ -71,7 +76,7 @@ class SendCachedEnvelopeIntegrationTest { @Test fun `when factory returns null, does nothing`() { - val sut = fixture.getSut(hasSender = false) + val sut = fixture.getSut(hasSender = false, mockExecutorService = ImmediateExecutorService()) sut.register(fixture.hub, fixture.options) @@ -81,7 +86,7 @@ class SendCachedEnvelopeIntegrationTest { @Test fun `when has factory and cacheDirPath set, submits task into queue`() { - val sut = fixture.getSut() + val sut = fixture.getSut(mockExecutorService = ImmediateExecutorService()) sut.register(fixture.hub, fixture.options) @@ -89,6 +94,15 @@ class SendCachedEnvelopeIntegrationTest { verify(fixture.sender).send() } + @Test + fun `when executorService is fake, does nothing`() { + val sut = fixture.getSut(mockExecutorService = mock()) + sut.register(fixture.hub, fixture.options) + + verify(fixture.factory, never()).create(any(), any()) + verify(fixture.sender, never()).send() + } + @Test fun `when has startup crash marker, awaits the task on the calling thread`() { val sut = fixture.getSut(hasStartupCrashMarker = true) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index b441c7789a..0918b943b2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -363,7 +363,6 @@ class SentryAndroidTest { // this is necessary to delay the AnrV2Integration processing to execute the configure // scope block below (otherwise it won't be possible as hub is no-op before .init) it.executorService.submit { - Thread.sleep(2000L) Sentry.configureScope { scope -> // make sure the scope values changed to test that we're still using previously // persisted values for the old ANR events @@ -380,7 +379,7 @@ class SentryAndroidTest { .untilTrue(asserted) // assert that persisted values have changed - options.executorService.close(1000L) // finalizes all enqueued persisting tasks + options.executorService.close(5000L) // finalizes all enqueued persisting tasks assertEquals( "TestActivity", PersistingScopeObserver.read(options, TRANSACTION_FILENAME, String::class.java) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index c0f7ce5abf..f8293f9b87 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -4,7 +4,10 @@ import android.content.Context import android.content.Intent import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check @@ -25,9 +28,10 @@ class SystemEventsBreadcrumbsIntegrationTest { var options = SentryAndroidOptions() val hub = mock() - fun getSut(enableSystemEventBreadcrumbs: Boolean = true): SystemEventsBreadcrumbsIntegration { + fun getSut(enableSystemEventBreadcrumbs: Boolean = true, executorService: ISentryExecutorService = ImmediateExecutorService()): SystemEventsBreadcrumbsIntegration { options = SentryAndroidOptions().apply { isEnableSystemEventBreadcrumbs = enableSystemEventBreadcrumbs + this.executorService = executorService } return SystemEventsBreadcrumbsIntegration(context) } @@ -45,6 +49,15 @@ class SystemEventsBreadcrumbsIntegrationTest { assertNotNull(sut.receiver) } + @Test + fun `system events callback is registered in the executorService`() { + val sut = fixture.getSut(executorService = mock()) + val hub = mock() + sut.register(hub, fixture.options) + + assertNull(sut.receiver) + } + @Test fun `When system events breadcrumb is disabled, it doesn't register callback`() { val sut = fixture.getSut(enableSystemEventBreadcrumbs = false) @@ -66,6 +79,17 @@ class SystemEventsBreadcrumbsIntegrationTest { assertNull(sut.receiver) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut(executorService = deferredExecutorService) + sut.register(fixture.hub, fixture.options) + assertNull(sut.receiver) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.receiver) + } + @Test fun `When broadcast received, added breadcrumb with type and category`() { val sut = fixture.getSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt index 29cebed6bd..d443b1e345 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt @@ -6,8 +6,15 @@ import android.hardware.SensorEvent import android.hardware.SensorEventListener import android.hardware.SensorManager import io.sentry.Breadcrumb +import io.sentry.Hint import io.sentry.IHub +import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.TypeCheckHint +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService +import io.sentry.test.getDeclaredCtor +import io.sentry.test.injectForField import org.mockito.kotlin.any import org.mockito.kotlin.check import org.mockito.kotlin.eq @@ -15,7 +22,6 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import kotlin.test.Ignore import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -26,8 +32,10 @@ class TempSensorBreadcrumbsIntegrationTest { val context = mock() val manager = mock() val sensor = mock() + val options = SentryAndroidOptions() - fun getSut(): TempSensorBreadcrumbsIntegration { + fun getSut(executorService: ISentryExecutorService = ImmediateExecutorService()): TempSensorBreadcrumbsIntegration { + options.executorService = executorService whenever(context.getSystemService(Context.SENSOR_SERVICE)).thenReturn(manager) whenever(manager.getDefaultSensor(Sensor.TYPE_AMBIENT_TEMPERATURE)).thenReturn(sensor) return TempSensorBreadcrumbsIntegration(context) @@ -39,21 +47,31 @@ class TempSensorBreadcrumbsIntegrationTest { @Test fun `When system events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) verify(fixture.manager).registerListener(any(), any(), eq(SensorManager.SENSOR_DELAY_NORMAL)) assertNotNull(sut.sensorManager) } + @Test + fun `temp sensor listener is registered in the executorService`() { + val sut = fixture.getSut(executorService = mock()) + val hub = mock() + sut.register(hub, fixture.options) + + assertNull(sut.sensorManager) + } + @Test fun `When system events breadcrumb is disabled, it should not register a callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions().apply { - isEnableSystemEventBreadcrumbs = false - } val hub = mock() - sut.register(hub, options) + sut.register( + hub, + fixture.options.apply { + isEnableSystemEventBreadcrumbs = false + } + ) verify(fixture.manager, never()).registerListener(any(), any(), any()) assertNull(sut.sensorManager) } @@ -61,28 +79,42 @@ class TempSensorBreadcrumbsIntegrationTest { @Test fun `When TempSensorBreadcrumbsIntegration is closed, it should unregister the callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.close() verify(fixture.manager).unregisterListener(any()) assertNull(sut.sensorManager) } - @Ignore("SensorEvent.values is always null, even when mocking it") + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut(executorService = deferredExecutorService) + sut.register(mock(), fixture.options) + assertNull(sut.sensorManager) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.sensorManager) + } + @Test fun `When onSensorChanged received, add a breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) - sut.onSensorChanged(mock()) + sut.register(hub, fixture.options) + val sensorCtor = "android.hardware.SensorEvent".getDeclaredCtor(emptyArray()) + val sensorEvent: SensorEvent = sensorCtor.newInstance() as SensorEvent + sensorEvent.injectForField("values", FloatArray(2) { 1F }) + sut.onSensorChanged(sensorEvent) verify(hub).addBreadcrumb( check { assertEquals("device.event", it.category) assertEquals("system", it.type) assertEquals(SentryLevel.INFO, it.level) + }, + check { + assertEquals(sensorEvent, it.get(TypeCheckHint.ANDROID_SENSOR_EVENT)) } ) } @@ -90,9 +122,8 @@ class TempSensorBreadcrumbsIntegrationTest { @Test fun `When onSensorChanged received and null values, do not add a breadcrumb`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) val event = mock() assertNull(event.values) sut.onSensorChanged(event) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index a1cea5e3d7..8d66cc7dd9 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -2,13 +2,13 @@ package io.sentry.android.okhttp import io.sentry.BaggageHeader import io.sentry.IHub -import io.sentry.ISentryExecutorService import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanDataConvention import io.sentry.SpanStatus import io.sentry.TransactionContext +import io.sentry.test.ImmediateExecutorService import okhttp3.Call import okhttp3.EventListener import okhttp3.OkHttpClient @@ -21,14 +21,12 @@ import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull -import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import java.util.concurrent.Future import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -334,13 +332,10 @@ class SentryOkHttpEventListenerTest { @Test fun `when response is not closed, root call is trimmed to responseHeadersEnd`() { - val mockExecutor = mock() - val captor = argumentCaptor() - whenever(mockExecutor.schedule(captor.capture(), any())).then { - captor.lastValue.run() - mock>() - } - val sut = fixture.getSut(httpStatusCode = 500, configureOptions = { it.executorService = mockExecutor }) + val sut = fixture.getSut( + httpStatusCode = 500, + configureOptions = { it.executorService = ImmediateExecutorService() } + ) val request = getRequest() val call = sut.newCall(request) val response = spy(call.execute()) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt index 9fab3b475c..51cc493d89 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -23,6 +23,7 @@ import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BOD import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT import io.sentry.exception.SentryHttpClientException +import io.sentry.test.ImmediateExecutorService import io.sentry.test.getProperty import okhttp3.Protocol import okhttp3.Request @@ -31,7 +32,6 @@ import okhttp3.mockwebserver.MockWebServer import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat -import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -39,7 +39,6 @@ import org.mockito.kotlin.never import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import java.util.concurrent.Future import java.util.concurrent.RejectedExecutionException import kotlin.RuntimeException import kotlin.test.Test @@ -534,13 +533,7 @@ class SentryOkHttpEventTest { @Test fun `scheduleFinish schedules finishEvent`() { - val mockExecutor = mock() - val captor = argumentCaptor() - whenever(mockExecutor.schedule(captor.capture(), any())).then { - captor.lastValue.run() - mock>() - } - fixture.hub.options.executorService = mockExecutor + fixture.hub.options.executorService = ImmediateExecutorService() val sut = spy(fixture.getSut()) val timestamp = mock() sut.scheduleFinish(timestamp) diff --git a/sentry-test-support/api/sentry-test-support.api b/sentry-test-support/api/sentry-test-support.api index 5de79d4a85..d7e83ad5ac 100644 --- a/sentry-test-support/api/sentry-test-support.api +++ b/sentry-test-support/api/sentry-test-support.api @@ -11,6 +11,17 @@ public final class io/sentry/SkipError : java/lang/Error { public fun (Ljava/lang/String;)V } +public final class io/sentry/test/DeferredExecutorService : io/sentry/ISentryExecutorService { + public fun ()V + public fun close (J)V + public final fun getScheduledRunnables ()Ljava/util/ArrayList; + public fun isClosed ()Z + public final fun runAll ()V + public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; + public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; + public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; +} + public final class io/sentry/test/ImmediateExecutorService : io/sentry/ISentryExecutorService { public fun ()V public fun close (J)V @@ -24,5 +35,6 @@ public final class io/sentry/test/ReflectionKt { public static final fun containsMethod (Ljava/lang/Class;Ljava/lang/String;Ljava/lang/Class;)Z public static final fun containsMethod (Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/Class;)Z public static final fun getCtor (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Constructor; + public static final fun getDeclaredCtor (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Constructor; } diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt index 3f48e530ce..8d0466cfaf 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt @@ -13,7 +13,34 @@ class ImmediateExecutorService : ISentryExecutorService { } override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> = mock() + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { + runnable.run() + return mock>() + } + override fun close(timeoutMillis: Long) {} + override fun isClosed(): Boolean = false +} + +class DeferredExecutorService : ISentryExecutorService { + + private val runnables = ArrayList() + val scheduledRunnables = ArrayList() + + fun runAll() { + runnables.forEach { it.run() } + scheduledRunnables.forEach { it.run() } + } + + override fun submit(runnable: Runnable): Future<*> { + runnables.add(runnable) + return mock() + } + + override fun submit(callable: Callable): Future = mock() + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { + scheduledRunnables.add(runnable) + return mock() + } override fun close(timeoutMillis: Long) {} override fun isClosed(): Boolean = false } diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt index f76d86e817..690dcc8725 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt @@ -52,3 +52,10 @@ fun String.getCtor(ctorTypes: Array>): Constructor<*> { val clazz = Class.forName(this) return clazz.getConstructor(*ctorTypes) } + +fun String.getDeclaredCtor(ctorTypes: Array>): Constructor<*> { + val clazz = Class.forName(this) + val constructor = clazz.getDeclaredConstructor(*ctorTypes) + constructor.isAccessible = true + return constructor +} diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 01f9b96018..16826b2c50 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -64,18 +64,17 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions return; } - final SendFireAndForget sender = factory.create(hub, options); - - if (sender == null) { - options.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); - return; - } - try { options .getExecutorService() .submit( () -> { + final SendFireAndForget sender = factory.create(hub, options); + + if (sender == null) { + options.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); + return; + } try { sender.send(); } catch (Throwable e) { diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 71b588d55b..fbb9997b94 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -335,18 +335,21 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) final File profilingTracesDir = new File(profilingTracesDirPath); profilingTracesDir.mkdirs(); - final File[] oldTracesDirContent = profilingTracesDir.listFiles(); + final long timestamp = System.currentTimeMillis(); try { options .getExecutorService() .submit( () -> { + final File[] oldTracesDirContent = profilingTracesDir.listFiles(); if (oldTracesDirContent == null) return; // Method trace files are normally deleted at the end of traces, but if that fails // for some reason we try to clear any old files here. for (File f : oldTracesDirContent) { - FileUtils.deleteRecursively(f); + if (f.lastModified() < timestamp) { + FileUtils.deleteRecursively(f); + } } }); } catch (RejectedExecutionException e) { diff --git a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt index e132839da1..bc55f6f079 100644 --- a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.test.DeferredExecutorService import io.sentry.test.getCtor import io.sentry.test.getProperty import io.sentry.test.injectForField @@ -13,9 +14,6 @@ import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.util.Timer -import java.util.concurrent.Callable -import java.util.concurrent.Future -import java.util.concurrent.FutureTask import java.util.concurrent.RejectedExecutionException import kotlin.test.Test import kotlin.test.assertEquals @@ -38,18 +36,7 @@ class DefaultTransactionPerformanceCollectorTest { val hub: IHub = mock() val options = SentryOptions() var mockTimer: Timer? = null - var lastScheduledRunnable: Runnable? = null - - val mockExecutorService = object : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = mock() - override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - lastScheduledRunnable = runnable - return FutureTask {} - } - override fun close(timeoutMillis: Long) {} - override fun isClosed() = false - } + val deferredExecutorService = DeferredExecutorService() val mockCpuCollector: ICollector = object : ICollector { override fun setup() {} @@ -62,7 +49,7 @@ class DefaultTransactionPerformanceCollectorTest { whenever(hub.options).thenReturn(options) } - fun getSut(memoryCollector: ICollector? = JavaMemoryCollector(), cpuCollector: ICollector? = mockCpuCollector, executorService: ISentryExecutorService = mockExecutorService): TransactionPerformanceCollector { + fun getSut(memoryCollector: ICollector? = JavaMemoryCollector(), cpuCollector: ICollector? = mockCpuCollector, executorService: ISentryExecutorService = deferredExecutorService): TransactionPerformanceCollector { options.dsn = "https://key@sentry.io/proj" options.executorService = executorService if (cpuCollector != null) { @@ -173,7 +160,7 @@ class DefaultTransactionPerformanceCollectorTest { verify(fixture.mockTimer, never())!!.cancel() // Let the timeout job stop the collector - fixture.lastScheduledRunnable?.run() + fixture.deferredExecutorService.runAll() verify(fixture.mockTimer)!!.cancel() // Data is deleted after the collector times out diff --git a/sentry/src/test/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegrationTest.kt b/sentry/src/test/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegrationTest.kt index a1c06d31db..12d78d46dd 100644 --- a/sentry/src/test/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegrationTest.kt +++ b/sentry/src/test/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegrationTest.kt @@ -1,9 +1,11 @@ package io.sentry import io.sentry.protocol.SdkVersion +import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -65,6 +67,7 @@ class SendCachedEnvelopeFireAndForgetIntegrationTest { fun `when Factory returns null, register logs and exit`() { val sut = SendCachedEnvelopeFireAndForgetIntegration(CustomFactory()) fixture.options.cacheDirPath = "abc" + fixture.options.executorService = ImmediateExecutorService() sut.register(fixture.hub, fixture.options) verify(fixture.logger).log(eq(SentryLevel.ERROR), eq("SendFireAndForget factory is null.")) verifyNoMoreInteractions(fixture.hub) @@ -92,6 +95,24 @@ class SendCachedEnvelopeFireAndForgetIntegrationTest { verify(fixture.logger).log(eq(SentryLevel.ERROR), eq("Failed to call the executor. Cached events will not be sent. Did you call Sentry.close()?"), any()) } + @Test + fun `register runs on executor service`() { + fixture.options.executorService = ImmediateExecutorService() + fixture.options.cacheDirPath = "cache" + val sut = fixture.getSut() + sut.register(fixture.hub, fixture.options) + verify(fixture.callback).create(eq(fixture.hub), eq(fixture.options)) + } + + @Test + fun `does not register on fake executor service`() { + fixture.options.executorService = mock() + fixture.options.cacheDirPath = "cache" + val sut = fixture.getSut() + sut.register(fixture.hub, fixture.options) + verify(fixture.callback, never()).create(any(), any()) + } + private class CustomFactory : SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory { override fun create(hub: IHub, options: SentryOptions): SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget? { return null diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index e9342c0004..a9a96936f5 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -270,6 +270,42 @@ class SentryTest { assertTrue(File(sentryOptions?.profilingTracesDirPath!!).list()!!.isEmpty()) } + @Test + fun `only old profiles in profilingTracesDirPath should be cleared when profiling is enabled`() { + val tempPath = getTempPath() + val options = SentryOptions().also { + it.dsn = dsn + it.cacheDirPath = tempPath + } + val dir = File(options.profilingTracesDirPath!!) + val oldProfile = File(dir, "oldProfile") + val newProfile = File(dir, "newProfile") + + // Create all files + dir.mkdirs() + oldProfile.createNewFile() + newProfile.createNewFile() + // Make the old profile look like it's created earlier + oldProfile.setLastModified(System.currentTimeMillis() - 10000) + // Make the new profile look like it's created later + newProfile.setLastModified(System.currentTimeMillis() + 10000) + + // Assert both file exist + assertTrue(oldProfile.exists()) + assertTrue(newProfile.exists()) + + Sentry.init { + it.dsn = dsn + it.profilesSampleRate = 1.0 + it.cacheDirPath = tempPath + it.executorService = ImmediateExecutorService() + } + + // Assert only the new profile exists + assertFalse(oldProfile.exists()) + assertTrue(newProfile.exists()) + } + @Test fun `profilingTracesDirPath should not be created and cleared when profiling is disabled`() { val tempPath = getTempPath()