Merge 7dcc745 into 283d83e

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - Add current activity name to app context ([#2999](https://github.com/getsentry/sentry-java/pull/2999))
 
+### Fixes
+
+- Reduce main thread work on init ([#3036](https://github.com/getsentry/sentry-java/pull/3036))
+
 ## 6.33.1
 
 ### Fixes

sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java

@@ -37,28 +37,42 @@ 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.");
+        options.getExecutorService().submit(() -> startOutboxSender(hub, options, path));
       } catch (Throwable e) {
-        // it could throw eg NoSuchFileException or NullPointerException
-        options
-            .getLogger()
-            .log(SentryLevel.ERROR, "Failed to initialize EnvelopeFileObserverIntegration.", e);
+        logger.log(
+            SentryLevel.DEBUG,
+            "Failed to start EnvelopeFileObserverIntegration on executor thread. Starting on the calling thread.",
+            e);
+        startOutboxSender(hub, options, path);
       }
     }
   }
 
+  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() {
     if (observer != null) {

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) {

sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt

@@ -2,10 +2,17 @@ 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.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 +22,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<ILogger>()
+
+        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 +77,32 @@ class EnvelopeFileObserverIntegrationTest {
         hub.close()
         verify(integrationMock).close()
     }
+
+    @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."))
+    }
 }

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,14 +86,23 @@ 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)
 
         await.untilFalse(fixture.flag)
         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)

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) {

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) {

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

sentry/src/test/java/io/sentry/SentryTest.kt

@@ -270,6 +270,40 @@ 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 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()