[SR] Disable replay in session mode when rate limit is active (#3854)

* Do not capture screenshots in session mode when rate limit is active * Changelog * WIP * Format code * Change approach to rate-limit and offline * Clean up * Tests * Api dump * Fix tests * Address PR feedback * Fix tests
getsentry · Nov 15, 2024 · adbc51d · adbc51d
1 parent dab52e2
commit adbc51d
Show file tree

Hide file tree

Showing 14 changed files with 369 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 - Ensure android initialization process continues even if options configuration block throws an exception ([#3887](https://github.com/getsentry/sentry-java/pull/3887))
 - Do not report parsing ANR error when there are no threads ([#3888](https://github.com/getsentry/sentry-java/pull/3888))
   - This should significantly reduce the number of events with message "Sentry Android SDK failed to parse system thread dump..." reported
+- Session Replay: Disable replay in session mode when rate limit is active ([#3854](https://github.com/getsentry/sentry-java/pull/3854))
 
 ### Dependencies
 

diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api
@@ -57,7 +57,7 @@ public final class io/sentry/android/replay/ReplayCache$Companion {
 	public final fun makeReplayCacheDir (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;)Ljava/io/File;
 }
 
-public final class io/sentry/android/replay/ReplayIntegration : android/content/ComponentCallbacks, io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, io/sentry/android/replay/gestures/TouchRecorderCallback, java/io/Closeable {
+public final class io/sentry/android/replay/ReplayIntegration : android/content/ComponentCallbacks, io/sentry/IConnectionStatusProvider$IConnectionStatusObserver, io/sentry/Integration, io/sentry/ReplayController, io/sentry/android/replay/ScreenshotRecorderCallback, io/sentry/android/replay/gestures/TouchRecorderCallback, io/sentry/transport/RateLimiter$IRateLimitObserver, java/io/Closeable {
 	public static final field $stable I
 	public fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;)V
 	public fun <init> (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)V
@@ -69,7 +69,9 @@ public final class io/sentry/android/replay/ReplayIntegration : android/content/
 	public fun getReplayId ()Lio/sentry/protocol/SentryId;
 	public fun isRecording ()Z
 	public fun onConfigurationChanged (Landroid/content/res/Configuration;)V
+	public fun onConnectionStatusChanged (Lio/sentry/IConnectionStatusProvider$ConnectionStatus;)V
 	public fun onLowMemory ()V
+	public fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V
 	public fun onScreenshotRecorded (Landroid/graphics/Bitmap;)V
 	public fun onScreenshotRecorded (Ljava/io/File;J)V
 	public fun onTouchEvent (Landroid/view/MotionEvent;)V

diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt
@@ -7,6 +7,11 @@ import android.graphics.Bitmap
 import android.os.Build
 import android.view.MotionEvent
 import io.sentry.Breadcrumb
+import io.sentry.DataCategory.All
+import io.sentry.DataCategory.Replay
+import io.sentry.IConnectionStatusProvider.ConnectionStatus
+import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED
+import io.sentry.IConnectionStatusProvider.IConnectionStatusObserver
 import io.sentry.IHub
 import io.sentry.Integration
 import io.sentry.NoOpReplayBreadcrumbConverter
@@ -32,6 +37,8 @@ import io.sentry.cache.PersistingScopeObserver.REPLAY_FILENAME
 import io.sentry.hints.Backfillable
 import io.sentry.protocol.SentryId
 import io.sentry.transport.ICurrentDateProvider
+import io.sentry.transport.RateLimiter
+import io.sentry.transport.RateLimiter.IRateLimitObserver
 import io.sentry.util.FileUtils
 import io.sentry.util.HintUtils
 import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
@@ -48,7 +55,14 @@ public class ReplayIntegration(
     private val recorderProvider: (() -> Recorder)? = null,
     private val recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)? = null,
     private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
-) : Integration, Closeable, ScreenshotRecorderCallback, TouchRecorderCallback, ReplayController, ComponentCallbacks {
+) : Integration,
+    Closeable,
+    ScreenshotRecorderCallback,
+    TouchRecorderCallback,
+    ReplayController,
+    ComponentCallbacks,
+    IConnectionStatusObserver,
+    IRateLimitObserver {
 
     // needed for the Java's call site
     constructor(context: Context, dateProvider: ICurrentDateProvider) : this(
@@ -113,6 +127,8 @@ public class ReplayIntegration(
         gestureRecorder = gestureRecorderProvider?.invoke() ?: GestureRecorder(options, this)
         isEnabled.set(true)
 
+        options.connectionStatusProvider.addConnectionStatusObserver(this)
+        hub.rateLimiter?.addRateLimitObserver(this)
         try {
             context.registerComponentCallbacks(this)
         } catch (e: Throwable) {
@@ -222,12 +238,14 @@ public class ReplayIntegration(
         hub?.configureScope { screen = it.screen?.substringAfterLast('.') }
         captureStrategy?.onScreenshotRecorded(bitmap) { frameTimeStamp ->
             addFrame(bitmap, frameTimeStamp, screen)
+            checkCanRecord()
         }
     }
 
     override fun onScreenshotRecorded(screenshot: File, frameTimestamp: Long) {
         captureStrategy?.onScreenshotRecorded { _ ->
             addFrame(screenshot, frameTimestamp)
+            checkCanRecord()
         }
     }
 
@@ -236,6 +254,8 @@ public class ReplayIntegration(
             return
         }
 
+        options.connectionStatusProvider.removeConnectionStatusObserver(this)
+        hub?.rateLimiter?.removeRateLimitObserver(this)
         try {
             context.unregisterComponentCallbacks(this)
         } catch (ignored: Throwable) {
@@ -259,12 +279,55 @@ public class ReplayIntegration(
         recorder?.start(recorderConfig)
     }
 
+    override fun onConnectionStatusChanged(status: ConnectionStatus) {
+        if (captureStrategy !is SessionCaptureStrategy) {
+            // we only want to stop recording when offline for session mode
+            return
+        }
+
+        if (status == DISCONNECTED) {
+            pause()
+        } else {
+            // being positive for other states, even if it's NO_PERMISSION
+            resume()
+        }
+    }
+
+    override fun onRateLimitChanged(rateLimiter: RateLimiter) {
+        if (captureStrategy !is SessionCaptureStrategy) {
+            // we only want to stop recording when rate-limited for session mode
+            return
+        }
+
+        if (rateLimiter.isActiveForCategory(All) || rateLimiter.isActiveForCategory(Replay)) {
+            pause()
+        } else {
+            resume()
+        }
+    }
+
     override fun onLowMemory() = Unit
 
     override fun onTouchEvent(event: MotionEvent) {
         captureStrategy?.onTouchEvent(event)
     }
 
+    /**
+     * Check if we're offline or rate-limited and pause for session mode to not overflow the
+     * envelope cache.
+     */
+    private fun checkCanRecord() {
+        if (captureStrategy is SessionCaptureStrategy &&
+            (
+                options.connectionStatusProvider.connectionStatus == DISCONNECTED ||
+                    hub?.rateLimiter?.isActiveForCategory(All) == true ||
+                    hub?.rateLimiter?.isActiveForCategory(Replay) == true
+                )
+        ) {
+            pause()
+        }
+    }
+
     private fun registerRootViewListeners() {
         if (recorder is OnRootViewsChangedListener) {
             rootViewsSpy.listeners += (recorder as OnRootViewsChangedListener)

diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt
@@ -35,7 +35,6 @@ import java.lang.ref.WeakReference
 import java.util.concurrent.Executors
 import java.util.concurrent.ThreadFactory
 import java.util.concurrent.atomic.AtomicBoolean
-import java.util.concurrent.atomic.AtomicReference
 import kotlin.LazyThreadSafetyMode.NONE
 import kotlin.math.roundToInt
 
@@ -51,7 +50,6 @@ internal class ScreenshotRecorder(
         Executors.newSingleThreadScheduledExecutor(RecorderExecutorServiceThreadFactory())
     }
     private var rootView: WeakReference<View>? = null
-    private val pendingViewHierarchy = AtomicReference<ViewHierarchyNode>()
     private val maskingPaint by lazy(NONE) { Paint() }
     private val singlePixelBitmap: Bitmap by lazy(NONE) {
         Bitmap.createBitmap(
@@ -230,7 +228,6 @@ internal class ScreenshotRecorder(
         unbind(rootView?.get())
         rootView?.clear()
         lastScreenshot?.recycle()
-        pendingViewHierarchy.set(null)
         isCapturing.set(false)
         recorder.gracefullyShutdown(options)
     }

diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt
@@ -56,6 +56,7 @@ internal interface CaptureStrategy {
     fun close()
 
     companion object {
+        private const val BREADCRUMB_START_OFFSET = 100L
         internal val currentEventsLock = Any()
 
         fun createSegment(
@@ -161,7 +162,10 @@ internal interface CaptureStrategy {
 
             val urls = LinkedList<String>()
             breadcrumbs.forEach { breadcrumb ->
-                if (breadcrumb.timestamp.time >= segmentTimestamp.time &&
+                // we add some fixed breadcrumb offset to make sure we don't miss any
+                // breadcrumbs that might be relevant for the current segment, but just happened
+                // earlier than the current segment (e.g. network connectivity changed)
+                if ((breadcrumb.timestamp.time + BREADCRUMB_START_OFFSET) >= segmentTimestamp.time &&
                     breadcrumb.timestamp.time < endTimestamp.time
                 ) {
                     val rrwebEvent = options

diff --git a/...y-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/...y-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt
@@ -1,7 +1,6 @@
 package io.sentry.android.replay.capture
 
 import android.graphics.Bitmap
-import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED
 import io.sentry.IHub
 import io.sentry.SentryLevel.DEBUG
 import io.sentry.SentryLevel.INFO
@@ -73,11 +72,6 @@ internal class SessionCaptureStrategy(
     }
 
     override fun onScreenshotRecorded(bitmap: Bitmap?, store: ReplayCache.(frameTimestamp: Long) -> Unit) {
-        if (options.connectionStatusProvider.connectionStatus == DISCONNECTED) {
-            options.logger.log(DEBUG, "Skipping screenshot recording, no internet connection")
-            bitmap?.recycle()
-            return
-        }
         // have to do it before submitting, otherwise if the queue is busy, the timestamp won't be
         // reflecting the exact time of when it was captured
         val frameTimestamp = dateProvider.currentTimeMillis