Merge pull request #2372 from bugsnag/PLAT-15150/stack-sampling

AppHang: Optional Stack sampling

Author: lemnik

bugsnag-plugin-android-apphang/api/bugsnag-plugin-android-apphang.api

@@ -1,10 +1,14 @@
 public final class com/bugsnag/android/AppHangConfiguration {
 	public fun <init> ()V
-	public fun <init> (JLandroid/os/Looper;)V
-	public synthetic fun <init> (JLandroid/os/Looper;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (JLandroid/os/Looper;Ljava/lang/Long;J)V
+	public synthetic fun <init> (JLandroid/os/Looper;Ljava/lang/Long;JILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun getAppHangThresholdMillis ()J
+	public final fun getStackSamplingIntervalMillis ()J
+	public final fun getStackSamplingThresholdMillis ()Ljava/lang/Long;
 	public final fun getWatchedLooper ()Landroid/os/Looper;
 	public final fun setAppHangThresholdMillis (J)V
+	public final fun setStackSamplingIntervalMillis (J)V
+	public final fun setStackSamplingThresholdMillis (Ljava/lang/Long;)V
 	public final fun setWatchedLooper (Landroid/os/Looper;)V
 }
 

bugsnag-plugin-android-apphang/src/main/java/com/bugsnag/android/AppHangConfiguration.kt

@@ -19,10 +19,41 @@ class AppHangConfiguration(
      * Defaults to [Looper.getMainLooper]
      */
     var watchedLooper: Looper = Looper.getMainLooper(),
+    /**
+     * How long after a heartbeat before the monitored thread should start being stack sampled.
+     * Setting this to a value between 1 and [appHangThresholdMillis] enables stack sampling,
+     * which produces a significantly higher-quality error at the expense of some runtime slowdown
+     * and memory.
+     *
+     * When enabled, potential AppHangs are identified early and the monitored thread's stack is
+     * sampled repeatedly until either the thread recovers or an AppHang error is raised. If a
+     * full AppHang error is reported, a secondary stack trace of the most frequently seen stack
+     * path will be attached to the report. These error reports tend to group better than typical
+     * AppHangs and ANRs, and provide more actionable insights.
+     *
+     * A reasonable starting value is 1 second (`1000`) which is a noticeable pause for a user,
+     * but not long enough to trigger many false-positives.
+     *
+     * Set to `null` to disable stack sampling (default).
+     *
+     * @see [stackSamplingIntervalMillis]
+     */
+    var stackSamplingThresholdMillis: Long? = null,
+    /**
+     * How many milliseconds to wait between stack samples. This is a best-effort value and the
+     * real sampling rate may be different.
+     *
+     * This property only takes effect when [stackSamplingThresholdMillis] is set to a non-null
+     * value to enable stack sampling.
+     *
+     * Defaults to 50
+     */
+    var stackSamplingIntervalMillis: Long = DEFAULT_SAMPLING_INTERVAL,
 ) {
     constructor() : this(DEFAULT_APP_HANG_THRESHOLD)
 
     internal companion object {
         internal const val DEFAULT_APP_HANG_THRESHOLD: Long = 3000L
+        internal const val DEFAULT_SAMPLING_INTERVAL: Long = 50L
     }
 }

bugsnag-plugin-android-apphang/src/main/java/com/bugsnag/android/BugsnagAppHangPlugin.kt

@@ -2,6 +2,7 @@ package com.bugsnag.android
 
 import androidx.annotation.VisibleForTesting
 import com.bugsnag.android.internal.LooperMonitorThread
+import com.bugsnag.android.internal.ThreadSampler
 
 /**
  * An alternative to Application Not Responding (ANR) reporting with configurable timeouts.
@@ -10,6 +11,8 @@ class BugsnagAppHangPlugin @JvmOverloads constructor(
     configuration: AppHangConfiguration = AppHangConfiguration()
 ) : Plugin {
     private val appHangThresholdMillis = configuration.appHangThresholdMillis
+    private val samplingThresholdMillis = configuration.stackSamplingThresholdMillis ?: 0
+    private val samplingRateMillis = configuration.stackSamplingIntervalMillis
     private val watchedLooper = configuration.watchedLooper
 
     private var client: Client? = null
@@ -37,7 +40,7 @@ class BugsnagAppHangPlugin @JvmOverloads constructor(
         client = null
     }
 
-    private fun reportAppHang(timeSinceLastHeartbeat: Long) {
+    private fun reportAppHang(timeSinceLastHeartbeat: Long, sampler: ThreadSampler?) {
         val watchedThread = watchedLooper.thread
         val stackTrace = watchedThread.stackTrace
         val threadName = watchedThread.name
@@ -49,6 +52,7 @@ class BugsnagAppHangPlugin @JvmOverloads constructor(
             )
         ) { event ->
             event.errors.firstOrNull()?.errorClass = "AppHang"
+            sampler?.createError(event)
 
             @Suppress("DEPRECATION")
             event.setErrorReportingThread(watchedThread.id)
@@ -65,6 +69,8 @@ class BugsnagAppHangPlugin @JvmOverloads constructor(
         monitorThread = LooperMonitorThread(
             watchedLooper,
             appHangThresholdMillis,
+            if (samplingThresholdMillis in 1..appHangThresholdMillis) samplingThresholdMillis else 0,
+            if (samplingRateMillis in 1..appHangThresholdMillis) samplingRateMillis else 0,
             this::reportAppHang
         )
 

bugsnag-plugin-android-apphang/src/main/java/com/bugsnag/android/internal/LooperMonitorThread.kt

@@ -6,14 +6,27 @@ import android.os.SystemClock
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.locks.LockSupport
+import kotlin.compareTo
+import kotlin.text.compareTo
+import kotlin.text.get
+import kotlin.text.set
 
 internal class LooperMonitorThread(
     watchedLooper: Looper,
     private val appHangThresholdMillis: Long,
-    private val onAppHangDetected: (timeSinceLastHeartbeat: Long) -> Unit
+    private val samplingThresholdMillis: Long,
+    private val samplingRateMillis: Long,
+    private val onAppHangDetected: (timeSinceLastHeartbeat: Long, ThreadSampler?) -> Unit
 ) : Thread("Bugsnag AppHang Monitor: ${watchedLooper.thread.name}") {
     private val handler: Handler = Handler(watchedLooper)
 
+    private val threadSampler: ThreadSampler? =
+        if (samplingThresholdMillis > 0) ThreadSampler(watchedLooper.thread)
+        else null
+
+    @Volatile
+    private var lastStackSampleTimestamp = 0L
+
     @Volatile
     private var lastHeartbeatTimestamp = 0L
 
@@ -23,9 +36,6 @@ internal class LooperMonitorThread(
 
     private val heartbeat: Runnable = Heartbeat()
 
-    private fun calculateTimeToAppHang(now: Long): Long =
-        (lastHeartbeatTimestamp + appHangThresholdMillis) - now
-
     fun startMonitoring() {
         if (isRunning.compareAndSet(false, true)) {
             start()
@@ -49,40 +59,87 @@ internal class LooperMonitorThread(
         }
 
         isAppHangDetected = true
-        onAppHangDetected(timeSinceLastHeartbeat)
+        onAppHangDetected(timeSinceLastHeartbeat, threadSampler)
     }
 
     override fun run() {
         handler.post(heartbeat)
 
         while (isRunning.get()) {
-            val waitThreshold =
-                if (lastHeartbeatTimestamp <= 0L) appHangThresholdMillis
-                else calculateTimeToAppHang(SystemClock.uptimeMillis())
+            val now = SystemClock.uptimeMillis()
+            val timeSinceHeartbeat = now - lastHeartbeatTimestamp
 
-            val waitThresholdNanos = TimeUnit.MILLISECONDS.toNanos(waitThreshold)
-            LockSupport.parkNanos(waitThresholdNanos)
+            // Wait until next sample time or hang detection time, whichever comes first
+            val waitMillis = calculateNextWaitTime(now, timeSinceHeartbeat)
+            LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(waitMillis))
 
             if (!isRunning.get()) break
 
-            val timeSinceLastHeartbeat = SystemClock.uptimeMillis() - lastHeartbeatTimestamp
+            val currentTime = SystemClock.uptimeMillis()
+            val currentTimeSinceHeartbeat = currentTime - lastHeartbeatTimestamp
+
+            if (shouldTakeSample(currentTime, currentTimeSinceHeartbeat)) {
+                threadSampler?.captureSample()
+                lastStackSampleTimestamp = currentTime
+            }
 
-            if (timeSinceLastHeartbeat >= appHangThresholdMillis) {
-                reportAppHang(timeSinceLastHeartbeat)
+            if (currentTimeSinceHeartbeat >= appHangThresholdMillis) {
+                reportAppHang(currentTimeSinceHeartbeat)
             }
 
             if (!handler.post(heartbeat)) {
-                // handler.post returning false means the Looper has likely quit
                 isRunning.set(false)
             }
         }
     }
 
+    private fun calculateNextWaitTime(now: Long, timeSinceHeartbeat: Long): Long {
+        if (lastHeartbeatTimestamp <= 0L) return appHangThresholdMillis
+        if (timeSinceHeartbeat >= appHangThresholdMillis) return Long.MAX_VALUE
+
+        val timeToHang = appHangThresholdMillis - timeSinceHeartbeat
+        if (threadSampler == null) return timeToHang
+
+        return calculateTimeToNextStackSample(now, timeToHang, timeSinceHeartbeat)
+    }
+
+    private fun calculateTimeToNextStackSample(
+        now: Long,
+        timeToHang: Long,
+        timeSinceHeartbeat: Long
+    ): Long {
+        return if (lastStackSampleTimestamp > 0L) {
+            // Already sampling - wait for next sample
+            val timeToNextSample = samplingRateMillis - (now - lastStackSampleTimestamp)
+            minOf(timeToNextSample, timeToHang)
+        } else {
+            val timeToSamplingStart = samplingThresholdMillis - timeSinceHeartbeat
+            minOf(timeToSamplingStart, timeToHang)
+        }
+    }
+
+    private fun shouldTakeSample(currentTime: Long, timeSinceHeartbeat: Long): Boolean {
+        if (threadSampler == null) return false
+        if (timeSinceHeartbeat < samplingThresholdMillis) return false
+
+        val timeSinceLastSample = if (lastStackSampleTimestamp <= 0L) {
+            Long.MAX_VALUE
+        } else {
+            currentTime - lastStackSampleTimestamp
+        }
+
+        return timeSinceLastSample >= samplingRateMillis
+    }
+
     private inner class Heartbeat : Runnable {
         override fun run() {
             lastHeartbeatTimestamp = SystemClock.uptimeMillis()
             isAppHangDetected = false
 
+            // Reset sampler when thread recovers
+            threadSampler?.resetSampling()
+            lastStackSampleTimestamp = 0L
+
             resetHeartbeatTimer()
         }