fix: if attach stack traces are enabled, it should attach current thread and its stack traces (#957)

Author: marandaneto

sentry/src/main/java/io/sentry/MainEventProcessor.java

@@ -80,11 +80,11 @@ private void processNonCachedEvent(SentryEvent event) {
       event.setSdk(options.getSdkVersion());
     }
 
-    if (event.getThreads() == null && options.isAttachThreads()) {
+    if (event.getThreads() == null) {
+      // collecting threadIds that came from the exception mechanism, so we can mark threads as
+      // crashed properly
       List<Long> mechanismThreadIds = null;
       if (event.getExceptions() != null) {
-        // collecting threadIds that came from the exception mechanism, so we can mark threads as
-        // crashed properly
         for (SentryException item : event.getExceptions()) {
           if (item.getMechanism() != null && item.getThreadId() != null) {
             if (mechanismThreadIds == null) {
@@ -95,7 +95,12 @@ private void processNonCachedEvent(SentryEvent event) {
         }
       }
 
-      event.setThreads(sentryThreadFactory.getCurrentThreads(mechanismThreadIds));
+      if (options.isAttachThreads()) {
+        event.setThreads(sentryThreadFactory.getCurrentThreads(mechanismThreadIds));
+      } else if (options.isAttachStacktrace()) {
+        // when attachStacktrace is enabled, we attach only the current thread and its stack traces
+        event.setThreads(sentryThreadFactory.getCurrentThread(mechanismThreadIds));
+      }
     }
   }
 }

sentry/src/main/java/io/sentry/SentryOptions.java

@@ -159,12 +159,13 @@ public class SentryOptions {
   /** Sets the distribution. Think about it together with release and environment */
   private @Nullable String dist;
 
-  /** When enabled, threads are automatically attached to all logged events. */
+  /** When enabled, all the threads are automatically attached to all logged events. */
   private boolean attachThreads;
 
   /**
    * When enabled, stack traces are automatically attached to all threads logged. Stack traces are
-   * always attached to exceptions but when this is set stack traces are also sent with threads
+   * always attached to exceptions but when this is set stack traces are also sent with threads. If
+   * no threads are logged, we log the current thread automatically.
    */
   private boolean attachStacktrace = true;
 

sentry/src/main/java/io/sentry/SentryStackTraceFactory.java

@@ -36,10 +36,17 @@ List<SentryStackFrame> getStackFrames(@Nullable final StackTraceElement[] elemen
       sentryStackFrames = new ArrayList<>();
       for (StackTraceElement item : elements) {
         if (item != null) {
+
+          // we don't want to add our own frames
+          final String className = item.getClassName();
+          if (className.startsWith("io.sentry.")) {
+            continue;
+          }
+
           final SentryStackFrame sentryStackFrame = new SentryStackFrame();
           // https://docs.sentry.io/development/sdk-dev/features/#in-app-frames
-          sentryStackFrame.setInApp(isInApp(item.getClassName()));
-          sentryStackFrame.setModule(item.getClassName());
+          sentryStackFrame.setInApp(isInApp(className));
+          sentryStackFrame.setModule(className);
           sentryStackFrame.setFunction(item.getMethodName());
           sentryStackFrame.setFilename(item.getFileName());
           // Protocol doesn't accept negative line numbers.

sentry/src/main/java/io/sentry/SentryThreadFactory.java

@@ -5,6 +5,7 @@
 import io.sentry.protocol.SentryThread;
 import io.sentry.util.Objects;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import org.jetbrains.annotations.NotNull;
@@ -34,6 +35,22 @@ public SentryThreadFactory(
     this.options = Objects.requireNonNull(options, "The SentryOptions is required");
   }
 
+  /**
+   * Converts the current thread to a SentryThread, it assumes its being called from the captured
+   * thread.
+   *
+   * @param mechanismThreadIds list of threadIds that came from exception mechanism
+   * @return a list of SentryThread with a single item or null
+   */
+  @Nullable
+  List<SentryThread> getCurrentThread(final @Nullable List<Long> mechanismThreadIds) {
+    final Map<Thread, StackTraceElement[]> threads = new HashMap<>();
+    final Thread currentThread = Thread.currentThread();
+    threads.put(currentThread, currentThread.getStackTrace());
+
+    return getCurrentThreads(threads, mechanismThreadIds);
+  }
+
   /**
    * Converts a list of all current threads to a list of SentryThread Assumes its being called from
    * the crashed thread.

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

@@ -25,8 +25,9 @@ class MainEventProcessorTest {
                 version = "1.2.3"
             }
         }
-        fun getSut(attachThreads: Boolean = true): MainEventProcessor {
+        fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true): MainEventProcessor {
             sentryOptions.isAttachThreads = attachThreads
+            sentryOptions.isAttachStacktrace = attachStackTrace
             return MainEventProcessor(sentryOptions)
         }
     }
@@ -132,7 +133,7 @@ class MainEventProcessorTest {
 
     @Test
     fun `when processing an event and attach threads is disabled, threads should not be set`() {
-        val sut = fixture.getSut(false)
+        val sut = fixture.getSut(attachThreads = false, attachStackTrace = false)
 
         var event = SentryEvent()
         event = sut.process(event, null)
@@ -150,6 +151,16 @@ class MainEventProcessorTest {
         assertNotNull(event.threads)
     }
 
+    @Test
+    fun `when processing an event and attach threads is disabled, but attach stacktrace is enabled, current thread should be set`() {
+        val sut = fixture.getSut(attachThreads = false, attachStackTrace = true)
+
+        var event = SentryEvent()
+        event = sut.process(event, null)
+
+        assertEquals(1, event.threads.count())
+    }
+
     @Test
     fun `sets sdkVersion in the event`() {
         val sut = fixture.getSut()

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

@@ -12,7 +12,8 @@ class SentryStackTraceFactoryTest {
     @Test
     fun `when getStackFrames is called passing a valid Array, not empty result`() {
         val stacktrace = Thread.currentThread().stackTrace
-        val count = stacktrace.size
+        // count the stack traces but ignores the test class which is io.sentry package
+        val count = stacktrace.size - 1
         assertEquals(count, sut.getStackFrames(stacktrace)!!.count())
     }
 
@@ -55,9 +56,9 @@ class SentryStackTraceFactoryTest {
     //region inAppExcludes
     @Test
     fun `when getStackFrames is called passing a valid inAppExcludes, inApp should be false if prefix matches it`() {
-        val element = generateStackTrace("io.sentry.MyActivity")
+        val element = generateStackTrace("io.mysentry.MyActivity")
         val elements = arrayOf(element)
-        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.sentry"), null)
+        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.mysentry"), null)
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
 
         assertFalse(sentryElements!!.first().isInApp)
@@ -67,15 +68,15 @@ class SentryStackTraceFactoryTest {
     fun `when getStackFrames is called passing a valid inAppExcludes, inApp should be false if prefix doesnt matches it`() {
         val element = generateStackTrace("io.myapp.MyActivity")
         val elements = arrayOf(element)
-        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.sentry"), null)
+        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.mysentry"), null)
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
 
         assertFalse(sentryElements!!.first().isInApp)
     }
 
     @Test
     fun `when getStackFrames is called passing an invalid inAppExcludes, inApp should be false`() {
-        val element = generateStackTrace("io.sentry.MyActivity")
+        val element = generateStackTrace("io.mysentry.MyActivity")
         val elements = arrayOf(element)
         val sentryStackTraceFactory = SentryStackTraceFactory(null, null)
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
@@ -87,9 +88,9 @@ class SentryStackTraceFactoryTest {
     //region inAppIncludes
     @Test
     fun `when getStackFrames is called passing a valid inAppIncludes, inApp should be true if prefix matches it`() {
-        val element = generateStackTrace("io.sentry.MyActivity")
+        val element = generateStackTrace("io.mysentry.MyActivity")
         val elements = arrayOf(element)
-        val sentryStackTraceFactory = SentryStackTraceFactory(null, listOf("io.sentry"))
+        val sentryStackTraceFactory = SentryStackTraceFactory(null, listOf("io.mysentry"))
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
 
         assertTrue(sentryElements!!.first().isInApp)
@@ -99,15 +100,15 @@ class SentryStackTraceFactoryTest {
     fun `when getStackFrames is called passing a valid inAppIncludes, inApp should be false if prefix doesnt matches it`() {
         val element = generateStackTrace("io.myapp.MyActivity")
         val elements = arrayOf(element)
-        val sentryStackTraceFactory = SentryStackTraceFactory(null, listOf("io.sentry"))
+        val sentryStackTraceFactory = SentryStackTraceFactory(null, listOf("io.mysentry"))
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
 
         assertFalse(sentryElements!!.first().isInApp)
     }
 
     @Test
     fun `when getStackFrames is called passing an invalid inAppIncludes, inApp should be false`() {
-        val element = generateStackTrace("io.sentry.MyActivity")
+        val element = generateStackTrace("io.mysentry.MyActivity")
         val elements = arrayOf(element)
         val sentryStackTraceFactory = SentryStackTraceFactory(null, null)
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
@@ -118,29 +119,40 @@ class SentryStackTraceFactoryTest {
 
     @Test
     fun `when getStackFrames is called passing a valid inAppIncludes and inAppExcludes, inApp should take precedence`() {
-        val element = generateStackTrace("io.sentry.MyActivity")
+        val element = generateStackTrace("io.mysentry.MyActivity")
         val elements = arrayOf(element)
-        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.sentry"), listOf("io.sentry"))
+        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.mysentry"), listOf("io.mysentry"))
         val sentryElements = sentryStackTraceFactory.getStackFrames(elements)
 
         assertTrue(sentryElements!!.first().isInApp)
     }
 
     @Test
     fun `when class is defined in the app, inApp is true`() {
-        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.sentry.not"), listOf("io.sentry.inApp"))
-        assertTrue(sentryStackTraceFactory.isInApp("io.sentry.inApp.ClassName"))
-        assertTrue(sentryStackTraceFactory.isInApp("io.sentry.inApp.somePackage.ClassName"))
-        assertFalse(sentryStackTraceFactory.isInApp("io.sentry.not.ClassName"))
-        assertFalse(sentryStackTraceFactory.isInApp("io.sentry.not.somePackage.ClassName"))
+        val sentryStackTraceFactory = SentryStackTraceFactory(listOf("io.mysentry.not"), listOf("io.mysentry.inApp"))
+        assertTrue(sentryStackTraceFactory.isInApp("io.mysentry.inApp.ClassName"))
+        assertTrue(sentryStackTraceFactory.isInApp("io.mysentry.inApp.somePackage.ClassName"))
+        assertFalse(sentryStackTraceFactory.isInApp("io.mysentry.not.ClassName"))
+        assertFalse(sentryStackTraceFactory.isInApp("io.mysentry.not.somePackage.ClassName"))
     }
 
     @Test
     fun `when class is not in the list, is not inApp`() {
-        val sentryStackTraceFactory = SentryStackTraceFactory(listOf(), listOf("io.sentry"))
+        val sentryStackTraceFactory = SentryStackTraceFactory(listOf(), listOf("io.mysentry"))
         assertFalse(sentryStackTraceFactory.isInApp("com.getsentry"))
     }
 
+    @Test
+    fun `when getStackFrames is called, remove sentry classes`() {
+        val stacktrace = Thread.currentThread().stackTrace
+        val sentryElement = StackTraceElement("io.sentry.element", "test", "test.java", 1)
+        stacktrace.plusElement(sentryElement)
+
+        assertNull(sut.getStackFrames(stacktrace)!!.find {
+            it.module.startsWith("io.sentry")
+        })
+    }
+
     private fun generateStackTrace(className: String?) =
         StackTraceElement(className, "method", "fileName", 10)
 }

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

@@ -3,8 +3,8 @@ package io.sentry
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNotEquals
 import kotlin.test.assertNotNull
-import kotlin.test.assertNotSame
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
@@ -23,7 +23,7 @@ class SentryThreadFactoryTest {
     fun `when getCurrentThreads is called, not empty result`() {
         val sut = fixture.getSut()
         val threads = sut.getCurrentThreads(null)
-        assertNotSame(0, threads!!.count())
+        assertNotEquals(0, threads!!.count())
     }
 
     @Test
@@ -82,4 +82,11 @@ class SentryThreadFactoryTest {
 
         assertNotNull(threads!!.firstOrNull { it.isCrashed })
     }
+
+    @Test
+    fun `when getCurrentThread is called, returns current thread`() {
+        val sut = fixture.getSut()
+        val threads = sut.getCurrentThread(null)
+        assertEquals(1, threads!!.count())
+    }
 }