From 07b758bad7d55989f7b72648983bc9c9e27b0096 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 17 Dec 2024 11:33:09 +0100 Subject: [PATCH 01/11] Get rid of the lock on touch events --- .../replay/capture/BaseCaptureStrategy.kt | 20 ++++++++----------- .../replay/capture/BufferCaptureStrategy.kt | 2 ++ .../android/replay/capture/CaptureStrategy.kt | 18 ++++++++--------- .../sentry/android/replay/util/Persistable.kt | 9 ++++++++- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index 4b37eafc786..6bd4f6eef0b 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -1,5 +1,6 @@ package io.sentry.android.replay.capture +import android.annotation.TargetApi import android.view.MotionEvent import io.sentry.Breadcrumb import io.sentry.DateUtils @@ -21,7 +22,6 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_WIDTH import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.Companion.createSegment -import io.sentry.android.replay.capture.CaptureStrategy.Companion.currentEventsLock import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.gestures.ReplayGestureConverter import io.sentry.android.replay.util.PersistableLinkedList @@ -32,7 +32,8 @@ import io.sentry.rrweb.RRWebEvent import io.sentry.transport.ICurrentDateProvider import java.io.File import java.util.Date -import java.util.LinkedList +import java.util.Deque +import java.util.concurrent.ConcurrentLinkedDeque import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ThreadFactory @@ -42,6 +43,7 @@ import java.util.concurrent.atomic.AtomicReference import kotlin.properties.ReadWriteProperty import kotlin.reflect.KProperty +@TargetApi(26) internal abstract class BaseCaptureStrategy( private val options: SentryOptions, private val hub: IHub?, @@ -81,12 +83,8 @@ internal abstract class BaseCaptureStrategy( override val replayCacheDir: File? get() = cache?.replayCacheDir override var replayType by persistableAtomic(propertyName = SEGMENT_KEY_REPLAY_TYPE) - protected val currentEvents: LinkedList = PersistableLinkedList( - propertyName = SEGMENT_KEY_REPLAY_RECORDING, - options, - persistingExecutor, - cacheProvider = { cache } - ) + + protected val currentEvents: Deque = ConcurrentLinkedDeque() protected val replayExecutor: ScheduledExecutorService by lazy { executor ?: Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) @@ -135,7 +133,7 @@ internal abstract class BaseCaptureStrategy( frameRate: Int = recorderConfig.frameRate, screenAtStart: String? = this.screenAtStart, breadcrumbs: List? = null, - events: LinkedList = this.currentEvents + events: Deque = this.currentEvents ): ReplaySegment = createSegment( hub, @@ -161,9 +159,7 @@ internal abstract class BaseCaptureStrategy( override fun onTouchEvent(event: MotionEvent) { val rrwebEvents = gestureConverter.convert(event, recorderConfig) if (rrwebEvents != null) { - synchronized(currentEventsLock) { - currentEvents += rrwebEvents - } + currentEvents += rrwebEvents } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index 2dbb2a17464..0d8ab2e6432 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -1,5 +1,6 @@ package io.sentry.android.replay.capture +import android.annotation.TargetApi import android.graphics.Bitmap import android.view.MotionEvent import io.sentry.DateUtils @@ -23,6 +24,7 @@ import java.io.File import java.util.Date import java.util.concurrent.ScheduledExecutorService +@TargetApi(26) internal class BufferCaptureStrategy( private val options: SentryOptions, private val hub: IHub?, 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 index 53f5c666831..b2a049e79ad 100644 --- 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 @@ -19,6 +19,7 @@ import io.sentry.rrweb.RRWebMetaEvent import io.sentry.rrweb.RRWebVideoEvent import java.io.File import java.util.Date +import java.util.Deque import java.util.LinkedList internal interface CaptureStrategy { @@ -57,7 +58,6 @@ internal interface CaptureStrategy { companion object { private const val BREADCRUMB_START_OFFSET = 100L - internal val currentEventsLock = Any() fun createSegment( hub: IHub?, @@ -73,7 +73,7 @@ internal interface CaptureStrategy { frameRate: Int, screenAtStart: String?, breadcrumbs: List?, - events: LinkedList + events: Deque ): ReplaySegment { val generatedVideo = cache?.createVideoOf( duration, @@ -127,7 +127,7 @@ internal interface CaptureStrategy { replayType: ReplayType, screenAtStart: String?, breadcrumbs: List, - events: LinkedList + events: Deque ): ReplaySegment { val endTimestamp = DateUtils.getDateTime(segmentTimestamp.time + videoDuration) val replay = SentryReplayEvent().apply { @@ -207,16 +207,16 @@ internal interface CaptureStrategy { } internal fun rotateEvents( - events: LinkedList, + events: Deque, until: Long, callback: ((RRWebEvent) -> Unit)? = null ) { - synchronized(currentEventsLock) { - var event = events.peek() - while (event != null && event.timestamp < until) { + val iter = events.iterator() + while (iter.hasNext()) { + val event = iter.next() + if (event.timestamp < until) { callback?.invoke(event) - events.remove() - event = events.peek() + iter.remove() } } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt index 553bae8dee8..0e224c53569 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt @@ -1,6 +1,9 @@ // ktlint-disable filename package io.sentry.android.replay.util +import android.annotation.TargetApi +import android.os.Build.VERSION_CODES +import androidx.annotation.RequiresApi import io.sentry.ReplayRecording import io.sentry.SentryOptions import io.sentry.android.replay.ReplayCache @@ -8,14 +11,18 @@ import io.sentry.rrweb.RRWebEvent import java.io.BufferedWriter import java.io.StringWriter import java.util.LinkedList +import java.util.concurrent.ConcurrentLinkedDeque import java.util.concurrent.ScheduledExecutorService +// TODO: enable this back after we are able to serialize individual touches to disk to not overload cpu +@Suppress("unused") +@TargetApi(26) internal class PersistableLinkedList( private val propertyName: String, private val options: SentryOptions, private val persistingExecutor: ScheduledExecutorService, private val cacheProvider: () -> ReplayCache? -) : LinkedList() { +) : ConcurrentLinkedDeque() { // only overriding methods that we use, to observe the collection override fun addAll(elements: Collection): Boolean { val result = super.addAll(elements) From e3df5393ccb3c14431b5c333943e78bc31dc4861 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 17 Dec 2024 11:35:30 +0100 Subject: [PATCH 02/11] pre-allocate some things for gesture converter --- .../sentry/android/replay/gestures/ReplayGestureConverter.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt index 59d6b30bce3..fa215960c43 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt @@ -56,7 +56,7 @@ class ReplayGestureConverter( val totalOffset = now - touchMoveBaseline return if (totalOffset > CAPTURE_MOVE_EVENT_THRESHOLD) { - val moveEvents = mutableListOf() + val moveEvents = ArrayList(currentPositions.size) for ((pointerId, positions) in currentPositions) { if (positions.isNotEmpty()) { moveEvents += RRWebInteractionMoveEvent().apply { @@ -88,7 +88,7 @@ class ReplayGestureConverter( } // new finger down - add a new pointer for tracking movement - currentPositions[pId] = ArrayList() + currentPositions[pId] = ArrayList(10) listOf( RRWebInteractionEvent().apply { timestamp = dateProvider.currentTimeMillis From fe152785d76f73ff9825c32494176d50755dccd8 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 18 Dec 2024 11:57:11 +0100 Subject: [PATCH 03/11] have one less thread switch for re]play --- .../android/replay/ReplayIntegration.kt | 26 ++++++++++++++----- .../android/replay/ScreenshotRecorder.kt | 10 +++---- .../sentry/android/replay/WindowRecorder.kt | 6 +++-- .../replay/capture/BaseCaptureStrategy.kt | 22 +--------------- .../replay/capture/BufferCaptureStrategy.kt | 4 +-- .../android/replay/capture/CaptureStrategy.kt | 2 -- .../replay/capture/SessionCaptureStrategy.kt | 2 +- .../sentry/android/replay/util/Executors.kt | 7 ++++- .../sentry/android/replay/util/Persistable.kt | 3 --- .../android/replay/ReplayIntegrationTest.kt | 1 - .../ReplayIntegrationWithRecorderTest.kt | 6 ++--- 11 files changed, 40 insertions(+), 49 deletions(-) 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 index 66bd17b7437..5b7e3ecae67 100644 --- 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 @@ -29,6 +29,7 @@ import io.sentry.android.replay.gestures.GestureRecorder import io.sentry.android.replay.gestures.TouchRecorderCallback import io.sentry.android.replay.util.MainLooperHandler import io.sentry.android.replay.util.appContext +import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.sample import io.sentry.android.replay.util.submitSafely import io.sentry.cache.PersistingScopeObserver @@ -46,8 +47,9 @@ import io.sentry.util.Random import java.io.Closeable import java.io.File import java.util.LinkedList +import java.util.concurrent.Executors +import java.util.concurrent.ThreadFactory import java.util.concurrent.atomic.AtomicBoolean -import kotlin.LazyThreadSafetyMode.NONE public class ReplayIntegration( private val context: Context, @@ -93,7 +95,10 @@ public class ReplayIntegration( private var recorder: Recorder? = null private var gestureRecorder: GestureRecorder? = null private val random by lazy { Random() } - internal val rootViewsSpy by lazy(NONE) { RootViewsSpy.install() } + internal val rootViewsSpy by lazy { RootViewsSpy.install() } + private val replayExecutor by lazy { + Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) + } // TODO: probably not everything has to be thread-safe here internal val isEnabled = AtomicBoolean(false) @@ -123,7 +128,7 @@ public class ReplayIntegration( } this.hub = hub - recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, mainLooperHandler) + recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, mainLooperHandler, replayExecutor) gestureRecorder = gestureRecorderProvider?.invoke() ?: GestureRecorder(options, this) isEnabled.set(true) @@ -166,9 +171,9 @@ public class ReplayIntegration( recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay) captureStrategy = replayCaptureStrategyProvider?.invoke(isFullSession) ?: if (isFullSession) { - SessionCaptureStrategy(options, hub, dateProvider, replayCacheProvider = replayCacheProvider) + SessionCaptureStrategy(options, hub, dateProvider, replayExecutor, replayCacheProvider) } else { - BufferCaptureStrategy(options, hub, dateProvider, random, replayCacheProvider = replayCacheProvider) + BufferCaptureStrategy(options, hub, dateProvider, random, replayExecutor, replayCacheProvider) } captureStrategy?.start(recorderConfig) @@ -229,7 +234,6 @@ public class ReplayIntegration( gestureRecorder?.stop() captureStrategy?.stop() isRecording.set(false) - captureStrategy?.close() captureStrategy = null } @@ -264,6 +268,7 @@ public class ReplayIntegration( recorder?.close() recorder = null rootViewsSpy.close() + replayExecutor.gracefullyShutdown(options) } override fun onConfigurationChanged(newConfig: Configuration) { @@ -405,4 +410,13 @@ public class ReplayIntegration( private class PreviousReplayHint : Backfillable { override fun shouldEnrich(): Boolean = false } + + private class ReplayExecutorServiceThreadFactory : ThreadFactory { + private var cnt = 0 + override fun newThread(r: Runnable): Thread { + val ret = Thread(r, "SentryReplayIntegration-" + cnt++) + ret.setDaemon(true) + return ret + } + } } 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 index 8ca9fed7fe8..6f588fe7791 100644 --- 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 @@ -25,7 +25,6 @@ import io.sentry.SentryReplayOptions import io.sentry.android.replay.util.MainLooperHandler import io.sentry.android.replay.util.addOnDrawListenerSafe import io.sentry.android.replay.util.getVisibleRects -import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.removeOnDrawListenerSafe import io.sentry.android.replay.util.submitSafely import io.sentry.android.replay.util.traverse @@ -34,7 +33,7 @@ import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.ImageViewHierarc import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.TextViewHierarchyNode import java.io.File import java.lang.ref.WeakReference -import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ThreadFactory import java.util.concurrent.atomic.AtomicBoolean import kotlin.LazyThreadSafetyMode.NONE @@ -44,13 +43,11 @@ import kotlin.math.roundToInt internal class ScreenshotRecorder( val config: ScreenshotRecorderConfig, val options: SentryOptions, - val mainLooperHandler: MainLooperHandler, + private val mainLooperHandler: MainLooperHandler, + private val recorder: ScheduledExecutorService, private val screenshotRecorderCallback: ScreenshotRecorderCallback? ) : ViewTreeObserver.OnDrawListener { - private val recorder by lazy { - Executors.newSingleThreadScheduledExecutor(RecorderExecutorServiceThreadFactory()) - } private var rootView: WeakReference? = null private val maskingPaint by lazy(NONE) { Paint() } private val singlePixelBitmap: Bitmap by lazy(NONE) { @@ -231,7 +228,6 @@ internal class ScreenshotRecorder( rootView?.clear() lastScreenshot?.recycle() isCapturing.set(false) - recorder.gracefullyShutdown(options) } private fun Bitmap.dominantColorForRect(rect: Rect): Int { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt index 35b5690a8d0..4237b2c9c8f 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt @@ -8,6 +8,7 @@ import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.scheduleAtFixedRateSafely import java.lang.ref.WeakReference import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ScheduledFuture import java.util.concurrent.ThreadFactory import java.util.concurrent.TimeUnit.MILLISECONDS @@ -17,7 +18,8 @@ import java.util.concurrent.atomic.AtomicBoolean internal class WindowRecorder( private val options: SentryOptions, private val screenshotRecorderCallback: ScreenshotRecorderCallback? = null, - private val mainLooperHandler: MainLooperHandler + private val mainLooperHandler: MainLooperHandler, + private val replayExecutor: ScheduledExecutorService ) : Recorder, OnRootViewsChangedListener { internal companion object { @@ -57,7 +59,7 @@ internal class WindowRecorder( return } - recorder = ScreenshotRecorder(recorderConfig, options, mainLooperHandler, screenshotRecorderCallback) + recorder = ScreenshotRecorder(recorderConfig, options, mainLooperHandler, replayExecutor, screenshotRecorderCallback) capturingTask = capturer.scheduleAtFixedRateSafely( options, "$TAG.capture", diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index 6bd4f6eef0b..df0b7575a37 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -15,7 +15,6 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FRAME_RATE import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_HEIGHT import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_ID import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_ID -import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_RECORDING import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_SCREEN_AT_START import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_TYPE import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP @@ -24,8 +23,6 @@ import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.Companion.createSegment import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.gestures.ReplayGestureConverter -import io.sentry.android.replay.util.PersistableLinkedList -import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.submitSafely import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebEvent @@ -48,7 +45,7 @@ internal abstract class BaseCaptureStrategy( private val options: SentryOptions, private val hub: IHub?, private val dateProvider: ICurrentDateProvider, - executor: ScheduledExecutorService? = null, + protected val replayExecutor: ScheduledExecutorService, private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null ) : CaptureStrategy { @@ -86,10 +83,6 @@ internal abstract class BaseCaptureStrategy( protected val currentEvents: Deque = ConcurrentLinkedDeque() - protected val replayExecutor: ScheduledExecutorService by lazy { - executor ?: Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) - } - override fun start( recorderConfig: ScreenshotRecorderConfig, segmentId: Int, @@ -163,19 +156,6 @@ internal abstract class BaseCaptureStrategy( } } - override fun close() { - replayExecutor.gracefullyShutdown(options) - } - - private class ReplayExecutorServiceThreadFactory : ThreadFactory { - private var cnt = 0 - override fun newThread(r: Runnable): Thread { - val ret = Thread(r, "SentryReplayIntegration-" + cnt++) - ret.setDaemon(true) - return ret - } - } - private class ReplayPersistingExecutorServiceThreadFactory : ThreadFactory { private var cnt = 0 override fun newThread(r: Runnable): Thread { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index 0d8ab2e6432..0418283dda1 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -30,9 +30,9 @@ internal class BufferCaptureStrategy( private val hub: IHub?, private val dateProvider: ICurrentDateProvider, private val random: Random, - executor: ScheduledExecutorService? = null, + executor: ScheduledExecutorService, replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null -) : BaseCaptureStrategy(options, hub, dateProvider, executor = executor, replayCacheProvider = replayCacheProvider) { +) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider = replayCacheProvider) { // TODO: capture envelopes for buffered segments instead, but don't send them until buffer is triggered private val bufferedSegments = mutableListOf() 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 index b2a049e79ad..f4e62157107 100644 --- 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 @@ -54,8 +54,6 @@ internal interface CaptureStrategy { fun convert(): CaptureStrategy - fun close() - companion object { private const val BREADCRUMB_START_OFFSET = 100L diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index 08aec7e9f7a..1a6dbc8c896 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -20,7 +20,7 @@ internal class SessionCaptureStrategy( private val options: SentryOptions, private val hub: IHub?, private val dateProvider: ICurrentDateProvider, - executor: ScheduledExecutorService? = null, + executor: ScheduledExecutorService, replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null ) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider) { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt index 453ff49df29..fefd3b78753 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt @@ -50,6 +50,11 @@ internal fun ExecutorService.submitSafely( taskName: String, task: Runnable ): Future<*>? { + if (Thread.currentThread().name.startsWith("SentryReplayIntegration")) { + // we're already on the worker thread, no need to submit + task.run() + return null + } return try { submit { try { @@ -73,7 +78,7 @@ internal fun ScheduledExecutorService.scheduleAtFixedRateSafely( task: Runnable ): ScheduledFuture<*>? { return try { - scheduleAtFixedRate({ + scheduleWithFixedDelay({ try { task.run() } catch (e: Throwable) { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt index 0e224c53569..2ae3bad1b6d 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Persistable.kt @@ -2,15 +2,12 @@ package io.sentry.android.replay.util import android.annotation.TargetApi -import android.os.Build.VERSION_CODES -import androidx.annotation.RequiresApi import io.sentry.ReplayRecording import io.sentry.SentryOptions import io.sentry.android.replay.ReplayCache import io.sentry.rrweb.RRWebEvent import java.io.BufferedWriter import java.io.StringWriter -import java.util.LinkedList import java.util.concurrent.ConcurrentLinkedDeque import java.util.concurrent.ScheduledExecutorService diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index ff396d04c10..95380deaa7f 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -411,7 +411,6 @@ class ReplayIntegrationTest { verify(recorder).stop() verify(recorder).close() verify(captureStrategy).stop() - verify(captureStrategy).close() assertFalse(replay.isRecording()) } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationWithRecorderTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationWithRecorderTest.kt index 6bab0f65498..ea28ce7e544 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationWithRecorderTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationWithRecorderTest.kt @@ -136,9 +136,6 @@ class ReplayIntegrationWithRecorderTest { replay.stop() assertEquals(STOPPED, recorder.state) - replay.close() - assertEquals(CLOSED, recorder.state) - // start again and capture some frames replay.start() @@ -176,6 +173,9 @@ class ReplayIntegrationWithRecorderTest { assertEquals(0, videoEvents?.first()?.segmentId) } ) + + replay.close() + assertEquals(CLOSED, recorder.state) } enum class LifecycleState { From c65f9b644d8d3644e99576f0cfa023fc89c4773c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 18 Dec 2024 13:23:29 +0100 Subject: [PATCH 04/11] update --- .../src/main/java/io/sentry/android/replay/WindowRecorder.kt | 2 ++ .../src/main/java/io/sentry/android/replay/util/Executors.kt | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt index 4237b2c9c8f..8f4b0526fcb 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt @@ -60,6 +60,8 @@ internal class WindowRecorder( } recorder = ScreenshotRecorder(recorderConfig, options, mainLooperHandler, replayExecutor, screenshotRecorderCallback) + // TODO: change this to use MainThreadHandler and just post on the main thread with delay + // to avoid thread context switch every time capturingTask = capturer.scheduleAtFixedRateSafely( options, "$TAG.capture", diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt index fefd3b78753..3c07ad7eaaa 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt @@ -78,7 +78,7 @@ internal fun ScheduledExecutorService.scheduleAtFixedRateSafely( task: Runnable ): ScheduledFuture<*>? { return try { - scheduleWithFixedDelay({ + scheduleAtFixedRate({ try { task.run() } catch (e: Throwable) { From bc2e9d6ddcd0d9a898213299d5c2eb71cd925c23 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 18 Dec 2024 13:36:47 +0100 Subject: [PATCH 05/11] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d1a18877fd..3dffe87b4c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Change TTFD timeout to 25 seconds ([#3984](https://github.com/getsentry/sentry-java/pull/3984)) - Session Replay: Fix memory leak when masking Compose screens ([#3985](https://github.com/getsentry/sentry-java/pull/3985)) +- Session Replay: Fix potential ANRs in `GestureRecorder` ([#4001](https://github.com/getsentry/sentry-java/pull/4001)) ## 7.19.0 From fb832b238e3c9c9fb39560dde6efae8791aee9ce Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 19 Dec 2024 17:38:09 +0100 Subject: [PATCH 06/11] Add option to disable orientation change tracking for session replay --- .../android/replay/ReplayIntegration.kt | 22 +++++++++++++------ sentry/api/sentry.api | 2 ++ .../java/io/sentry/SentryReplayOptions.java | 16 ++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) 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 index 5b7e3ecae67..700e661e868 100644 --- 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 @@ -134,10 +134,16 @@ public class ReplayIntegration( options.connectionStatusProvider.addConnectionStatusObserver(this) hub.rateLimiter?.addRateLimitObserver(this) - try { - context.registerComponentCallbacks(this) - } catch (e: Throwable) { - options.logger.log(INFO, "ComponentCallbacks is not available, orientation changes won't be handled by Session replay", e) + if (options.experimental.sessionReplay.isTrackOrientationChange) { + try { + context.registerComponentCallbacks(this) + } catch (e: Throwable) { + options.logger.log( + INFO, + "ComponentCallbacks is not available, orientation changes won't be handled by Session replay", + e + ) + } } addIntegrationToSdkVersion("Replay") @@ -260,9 +266,11 @@ public class ReplayIntegration( options.connectionStatusProvider.removeConnectionStatusObserver(this) hub?.rateLimiter?.removeRateLimitObserver(this) - try { - context.unregisterComponentCallbacks(this) - } catch (ignored: Throwable) { + if (options.experimental.sessionReplay.isTrackOrientationChange) { + try { + context.unregisterComponentCallbacks(this) + } catch (ignored: Throwable) { + } } stop() recorder?.close() diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index fc2f4ba51e4..ef922d4f593 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2741,12 +2741,14 @@ public final class io/sentry/SentryReplayOptions { public fun getUnmaskViewContainerClass ()Ljava/lang/String; public fun isSessionReplayEnabled ()Z public fun isSessionReplayForErrorsEnabled ()Z + public fun isTrackOrientationChange ()Z public fun setMaskAllImages (Z)V public fun setMaskAllText (Z)V public fun setMaskViewContainerClass (Ljava/lang/String;)V public fun setOnErrorSampleRate (Ljava/lang/Double;)V public fun setQuality (Lio/sentry/SentryReplayOptions$SentryReplayQuality;)V public fun setSessionSampleRate (Ljava/lang/Double;)V + public fun setTrackOrientationChange (Z)V public fun setUnmaskViewContainerClass (Ljava/lang/String;)V } diff --git a/sentry/src/main/java/io/sentry/SentryReplayOptions.java b/sentry/src/main/java/io/sentry/SentryReplayOptions.java index fd492213ac1..f9e82c8600e 100644 --- a/sentry/src/main/java/io/sentry/SentryReplayOptions.java +++ b/sentry/src/main/java/io/sentry/SentryReplayOptions.java @@ -108,6 +108,12 @@ public enum SentryReplayQuality { /** The maximum duration of a full session replay, defaults to 1h. */ private long sessionDuration = 60 * 60 * 1000L; + /** + * Whether to track orientation changes in session replay. Used in Flutter as it has its own + * callbacks to determine the orientation change. + */ + private boolean trackOrientationChange = true; + public SentryReplayOptions(final boolean empty) { if (!empty) { setMaskAllText(true); @@ -266,4 +272,14 @@ public void setUnmaskViewContainerClass(@NotNull String containerClass) { public @Nullable String getUnmaskViewContainerClass() { return unmaskViewContainerClass; } + + @ApiStatus.Internal + public boolean isTrackOrientationChange() { + return trackOrientationChange; + } + + @ApiStatus.Internal + public void setTrackOrientationChange(final boolean trackOrientationChange) { + this.trackOrientationChange = trackOrientationChange; + } } From 55dbeec461242e4aac83b14cd4e21cb8d8ceebba Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 20 Dec 2024 13:34:52 +0100 Subject: [PATCH 07/11] Make RecorderConfig lazier --- .../api/sentry-android-replay.api | 10 +++++----- .../java/io/sentry/android/replay/ReplayCache.kt | 16 ++++++++-------- .../sentry/android/replay/ReplayIntegration.kt | 11 +++++------ .../replay/capture/BaseCaptureStrategy.kt | 6 ++++-- .../replay/capture/BufferCaptureStrategy.kt | 2 +- .../android/replay/capture/CaptureStrategy.kt | 5 ++++- .../replay/capture/SessionCaptureStrategy.kt | 2 +- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index 7e2db5248f0..33043e69b66 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -43,12 +43,12 @@ public abstract interface class io/sentry/android/replay/Recorder : java/io/Clos public final class io/sentry/android/replay/ReplayCache : java/io/Closeable { public static final field $stable I public static final field Companion Lio/sentry/android/replay/ReplayCache$Companion; - public fun (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;Lio/sentry/android/replay/ScreenshotRecorderConfig;)V + public fun (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;)V public final fun addFrame (Ljava/io/File;JLjava/lang/String;)V public static synthetic fun addFrame$default (Lio/sentry/android/replay/ReplayCache;Ljava/io/File;JLjava/lang/String;ILjava/lang/Object;)V public fun close ()V - public final fun createVideoOf (JJIIILjava/io/File;)Lio/sentry/android/replay/GeneratedVideo; - public static synthetic fun createVideoOf$default (Lio/sentry/android/replay/ReplayCache;JJIIILjava/io/File;ILjava/lang/Object;)Lio/sentry/android/replay/GeneratedVideo; + public final fun createVideoOf (JJIIIIILjava/io/File;)Lio/sentry/android/replay/GeneratedVideo; + public static synthetic fun createVideoOf$default (Lio/sentry/android/replay/ReplayCache;JJIIIIILjava/io/File;ILjava/lang/Object;)Lio/sentry/android/replay/GeneratedVideo; public final fun persistSegmentValues (Ljava/lang/String;Ljava/lang/String;)V public final fun rotate (J)Ljava/lang/String; } @@ -60,8 +60,8 @@ public final class io/sentry/android/replay/ReplayCache$Companion { 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 (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;)V - public fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)V - public synthetic fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V + public synthetic fun (Landroid/content/Context;Lio/sentry/transport/ICurrentDateProvider;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun captureReplay (Ljava/lang/Boolean;)V public fun close ()V public fun getBreadcrumbConverter ()Lio/sentry/ReplayBreadcrumbConverter; diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt index 3db92ea5d80..a757c4b4554 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt @@ -38,8 +38,7 @@ import java.util.concurrent.atomic.AtomicBoolean */ public class ReplayCache( private val options: SentryOptions, - private val replayId: SentryId, - private val recorderConfig: ScreenshotRecorderConfig + private val replayId: SentryId ) : Closeable { private val isClosed = AtomicBoolean(false) @@ -133,6 +132,8 @@ public class ReplayCache( segmentId: Int, height: Int, width: Int, + frameRate: Int, + bitRate: Int, videoFile: File = File(replayCacheDir, "$segmentId.mp4") ): GeneratedVideo? { if (videoFile.exists() && videoFile.length() > 0) { @@ -146,7 +147,6 @@ public class ReplayCache( return null } - // TODO: reuse instance of encoder and just change file path to create a different muxer encoder = synchronized(encoderLock) { SimpleVideoEncoder( options, @@ -154,13 +154,13 @@ public class ReplayCache( file = videoFile, recordingHeight = height, recordingWidth = width, - frameRate = recorderConfig.frameRate, - bitRate = recorderConfig.bitRate + frameRate = frameRate, + bitRate = bitRate ) ).also { it.start() } } - val step = 1000 / recorderConfig.frameRate.toLong() + val step = 1000 / frameRate.toLong() var frameCount = 0 var lastFrame: ReplayFrame = frames.first() for (timestamp in from until (from + (duration)) step step) { @@ -306,7 +306,7 @@ public class ReplayCache( } } - internal fun fromDisk(options: SentryOptions, replayId: SentryId, replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null): LastSegmentData? { + internal fun fromDisk(options: SentryOptions, replayId: SentryId, replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null): LastSegmentData? { val replayCacheDir = makeReplayCacheDir(options, replayId) val lastSegmentFile = File(replayCacheDir, ONGOING_SEGMENT) if (!lastSegmentFile.exists()) { @@ -360,7 +360,7 @@ public class ReplayCache( scaleFactorY = 1.0f ) - val cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig) + val cache = replayCacheProvider?.invoke(replayId) ?: ReplayCache(options, replayId) cache.replayCacheDir?.listFiles { dir, name -> if (name.endsWith(".jpg")) { val file = File(dir, name) 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 index 700e661e868..4148fbb26cd 100644 --- 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 @@ -56,7 +56,7 @@ public class ReplayIntegration( private val dateProvider: ICurrentDateProvider, private val recorderProvider: (() -> Recorder)? = null, private val recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)? = null, - private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null + private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null ) : Integration, Closeable, ScreenshotRecorderCallback, @@ -80,7 +80,7 @@ public class ReplayIntegration( dateProvider: ICurrentDateProvider, recorderProvider: (() -> Recorder)?, recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)?, - replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)?, + replayCacheProvider: ((replayId: SentryId) -> ReplayCache)?, replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null, mainLooperHandler: MainLooperHandler? = null, gestureRecorderProvider: (() -> GestureRecorder)? = null @@ -110,8 +110,6 @@ public class ReplayIntegration( private var mainLooperHandler: MainLooperHandler = MainLooperHandler() private var gestureRecorderProvider: (() -> GestureRecorder)? = null - private lateinit var recorderConfig: ScreenshotRecorderConfig - override fun register(hub: IHub, options: SentryOptions) { this.options = options @@ -175,7 +173,7 @@ public class ReplayIntegration( return } - recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay) + val recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay) captureStrategy = replayCaptureStrategyProvider?.invoke(isFullSession) ?: if (isFullSession) { SessionCaptureStrategy(options, hub, dateProvider, replayExecutor, replayCacheProvider) } else { @@ -287,7 +285,7 @@ public class ReplayIntegration( recorder?.stop() // refresh config based on new device configuration - recorderConfig = recorderConfigProvider?.invoke(true) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay) + val recorderConfig = recorderConfigProvider?.invoke(true) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay) captureStrategy?.onConfigurationChanged(recorderConfig) recorder?.start(recorderConfig) @@ -400,6 +398,7 @@ public class ReplayIntegration( height = lastSegment.recorderConfig.recordingHeight, width = lastSegment.recorderConfig.recordingWidth, frameRate = lastSegment.recorderConfig.frameRate, + bitRate = lastSegment.recorderConfig.bitRate, cache = lastSegment.cache, replayType = lastSegment.replayType, screenAtStart = lastSegment.screenAtStart, diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index df0b7575a37..fbc80565b1b 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -46,7 +46,7 @@ internal abstract class BaseCaptureStrategy( private val hub: IHub?, private val dateProvider: ICurrentDateProvider, protected val replayExecutor: ScheduledExecutorService, - private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null + private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null ) : CaptureStrategy { internal companion object { @@ -89,7 +89,7 @@ internal abstract class BaseCaptureStrategy( replayId: SentryId, replayType: ReplayType? ) { - cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig) + cache = replayCacheProvider?.invoke(replayId) ?: ReplayCache(options, replayId) this.currentReplayId = replayId this.currentSegment = segmentId @@ -124,6 +124,7 @@ internal abstract class BaseCaptureStrategy( replayType: ReplayType = this.replayType, cache: ReplayCache? = this.cache, frameRate: Int = recorderConfig.frameRate, + bitRate: Int = recorderConfig.bitRate, screenAtStart: String? = this.screenAtStart, breadcrumbs: List? = null, events: Deque = this.currentEvents @@ -140,6 +141,7 @@ internal abstract class BaseCaptureStrategy( replayType, cache, frameRate, + bitRate, screenAtStart, breadcrumbs, events diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index 0418283dda1..8ef346f8a3a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -31,7 +31,7 @@ internal class BufferCaptureStrategy( private val dateProvider: ICurrentDateProvider, private val random: Random, executor: ScheduledExecutorService, - replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null + replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null ) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider = replayCacheProvider) { // TODO: capture envelopes for buffered segments instead, but don't send them until buffer is triggered 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 index f4e62157107..2f7a5bef148 100644 --- 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 @@ -69,6 +69,7 @@ internal interface CaptureStrategy { replayType: ReplayType, cache: ReplayCache?, frameRate: Int, + bitRate: Int, screenAtStart: String?, breadcrumbs: List?, events: Deque @@ -78,7 +79,9 @@ internal interface CaptureStrategy { currentSegmentTimestamp.time, segmentId, height, - width + width, + frameRate, + bitRate ) ?: return ReplaySegment.Failed val (video, frameCount, videoDuration) = generatedVideo diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index 1a6dbc8c896..a8c8f1387cc 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -21,7 +21,7 @@ internal class SessionCaptureStrategy( private val hub: IHub?, private val dateProvider: ICurrentDateProvider, executor: ScheduledExecutorService, - replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null + replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null ) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider) { internal companion object { From 01b5a88fec2c54586b4f2691d0330670ac20d179 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 20 Dec 2024 13:42:34 +0100 Subject: [PATCH 08/11] Fix tests --- .../sentry/android/replay/ReplayCacheTest.kt | 72 +++++++------------ .../android/replay/ReplayIntegrationTest.kt | 4 +- .../capture/BufferCaptureStrategyTest.kt | 4 +- .../capture/SessionCaptureStrategyTest.kt | 4 +- 4 files changed, 33 insertions(+), 51 deletions(-) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt index 91a17f51929..c7529ac1f60 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt @@ -47,14 +47,12 @@ class ReplayCacheTest { val options = SentryOptions() fun getSut( dir: TemporaryFolder?, - replayId: SentryId = SentryId(), - frameRate: Int + replayId: SentryId = SentryId() ): ReplayCache { - val recorderConfig = ScreenshotRecorderConfig(100, 200, 1f, 1f, frameRate = frameRate, bitRate = 20_000) options.run { cacheDirPath = dir?.newFolder()?.absolutePath } - return ReplayCache(options, replayId, recorderConfig) + return ReplayCache(options, replayId) } } @@ -70,8 +68,7 @@ class ReplayCacheTest { val replayId = SentryId() val replayCache = fixture.getSut( null, - replayId, - frameRate = 1 + replayId ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) @@ -85,8 +82,7 @@ class ReplayCacheTest { val replayId = SentryId() val replayCache = fixture.getSut( tmpDir, - replayId, - frameRate = 1 + replayId ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) @@ -101,11 +97,10 @@ class ReplayCacheTest { @Test fun `when no frames are provided, returns nothing`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) - val video = replayCache.createVideoOf(5000L, 0, 0, 100, 200) + val video = replayCache.createVideoOf(5000L, 0, 0, 100, 200, 1, 20_000) assertNull(video) } @@ -114,8 +109,7 @@ class ReplayCacheTest { fun `deletes frames after creating a video`() { ReplayShadowMediaCodec.framesToEncode = 3 val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) @@ -123,7 +117,7 @@ class ReplayCacheTest { replayCache.addFrame(bitmap, 1001) replayCache.addFrame(bitmap, 2001) - val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200) + val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200, 1, 20_000) assertEquals(3, segment0!!.frameCount) assertEquals(3000, segment0.duration) assertTrue { segment0.video.exists() && segment0.video.length() > 0 } @@ -136,14 +130,13 @@ class ReplayCacheTest { @Test fun `repeats last known frame for the segment duration`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) replayCache.addFrame(bitmap, 1) - val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200) + val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200, 1, 20_000) assertEquals(5, segment0!!.frameCount) assertEquals(5000, segment0.duration) assertTrue { segment0.video.exists() && segment0.video.length() > 0 } @@ -153,15 +146,14 @@ class ReplayCacheTest { @Test fun `repeats last known frame for the segment duration for each timespan`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) replayCache.addFrame(bitmap, 1) replayCache.addFrame(bitmap, 3001) - val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200) + val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200, 1, 20_000) assertEquals(5, segment0!!.frameCount) assertEquals(5000, segment0.duration) assertTrue { segment0.video.exists() && segment0.video.length() > 0 } @@ -171,20 +163,19 @@ class ReplayCacheTest { @Test fun `repeats last known frame for each segment`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) replayCache.addFrame(bitmap, 1) replayCache.addFrame(bitmap, 5001) - val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200) + val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200, 1, 20_000) assertEquals(5, segment0!!.frameCount) assertEquals(5000, segment0.duration) assertEquals(File(replayCache.replayCacheDir, "0.mp4"), segment0.video) - val segment1 = replayCache.createVideoOf(5000L, 5000L, 1, 100, 200) + val segment1 = replayCache.createVideoOf(5000L, 5000L, 1, 100, 200, 1, 20_000) assertEquals(5, segment1!!.frameCount) assertEquals(5000, segment1.duration) assertTrue { segment0.video.exists() && segment0.video.length() > 0 } @@ -196,8 +187,7 @@ class ReplayCacheTest { ReplayShadowMediaCodec.framesToEncode = 6 val replayCache = fixture.getSut( - tmpDir, - frameRate = 2 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) @@ -205,7 +195,7 @@ class ReplayCacheTest { replayCache.addFrame(bitmap, 1001) replayCache.addFrame(bitmap, 1501) - val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200) + val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200, 2, 20_000) assertEquals(6, segment0!!.frameCount) assertEquals(3000, segment0.duration) assertTrue { segment0.video.exists() && segment0.video.length() > 0 } @@ -215,8 +205,7 @@ class ReplayCacheTest { @Test fun `does not add frame when bitmap is recycled`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888).also { it.recycle() } @@ -228,8 +217,7 @@ class ReplayCacheTest { @Test fun `addFrame with File path works`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val flutterCacheDir = @@ -240,7 +228,7 @@ class ReplayCacheTest { val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888).also { it.recycle() } replayCache.addFrame(screenshot, frameTimestamp = 1) - val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200, videoFile = video) + val segment0 = replayCache.createVideoOf(5000L, 0, 0, 100, 200, 1, 20_000, videoFile = video) assertEquals(5, segment0!!.frameCount) assertEquals(5000, segment0.duration) @@ -251,8 +239,7 @@ class ReplayCacheTest { @Test fun `rotates frames`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) @@ -269,8 +256,7 @@ class ReplayCacheTest { @Test fun `rotate returns first screen in buffer`() { val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) @@ -288,8 +274,7 @@ class ReplayCacheTest { val replayId = SentryId() val replayCache = fixture.getSut( tmpDir, - replayId, - frameRate = 1 + replayId ) replayCache.close() @@ -303,8 +288,7 @@ class ReplayCacheTest { val replayId = SentryId() val replayCache = fixture.getSut( tmpDir, - replayId, - frameRate = 1 + replayId ) replayCache.persistSegmentValues("key1", "value1") @@ -320,8 +304,7 @@ class ReplayCacheTest { val replayId = SentryId() val replayCache = fixture.getSut( tmpDir, - replayId, - frameRate = 1 + replayId ) replayCache.persistSegmentValues("key1", "value1") @@ -467,8 +450,7 @@ class ReplayCacheTest { ReplayShadowMediaCodec.framesToEncode = 3 val replayCache = fixture.getSut( - tmpDir, - frameRate = 1 + tmpDir ) val oldVideoFile = File(replayCache.replayCacheDir, "0.mp4").also { @@ -480,7 +462,7 @@ class ReplayCacheTest { replayCache.addFrame(bitmap, 1001) replayCache.addFrame(bitmap, 2001) - val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200, oldVideoFile) + val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200, 1, 20_000, oldVideoFile) assertEquals(3, segment0!!.frameCount) assertEquals(3000, segment0.duration) assertTrue { segment0.video.exists() && segment0.video.length() > 0 } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index 95380deaa7f..f3751361493 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -96,7 +96,7 @@ class ReplayIntegrationTest { val replayCache = mock { on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997))) - on { createVideoOf(anyLong(), anyLong(), anyInt(), anyInt(), anyInt(), any()) } + on { createVideoOf(anyLong(), anyLong(), anyInt(), anyInt(), anyInt(), anyInt(), anyInt(), any()) } .thenReturn(GeneratedVideo(File("0.mp4"), 5, VIDEO_DURATION)) } @@ -127,7 +127,7 @@ class ReplayIntegrationTest { dateProvider, recorderProvider, recorderConfigProvider = recorderConfigProvider, - replayCacheProvider = { _, _ -> replayCache }, + replayCacheProvider = { _ -> replayCache }, replayCaptureStrategyProvider = replayCaptureStrategyProvider, gestureRecorderProvider = gestureRecorderProvider ) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt index 625306cb8e4..1fdb41386a7 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt @@ -70,7 +70,7 @@ class BufferCaptureStrategyTest { on { persistSegmentValues(any(), anyOrNull()) }.then { persistedSegment.put(it.arguments[0].toString(), it.arguments[1]?.toString()) } - on { createVideoOf(anyLong(), anyLong(), anyInt(), anyInt(), anyInt(), any()) } + on { createVideoOf(anyLong(), anyLong(), anyInt(), anyInt(), anyInt(), anyInt(), anyInt(), any()) } .thenReturn(GeneratedVideo(File("0.mp4"), 5, VIDEO_DURATION)) } val recorderConfig = ScreenshotRecorderConfig( @@ -104,7 +104,7 @@ class BufferCaptureStrategyTest { null }.whenever(it).submit(any()) } - ) { _, _ -> replayCache } + ) { _ -> replayCache } } fun mockedMotionEvent(action: Int): MotionEvent = mock { diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index 12eb10c3f4b..50adeae3a11 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -76,7 +76,7 @@ class SessionCaptureStrategyTest { on { persistSegmentValues(any(), anyOrNull()) }.then { persistedSegment.put(it.arguments[0].toString(), it.arguments[1]?.toString()) } - on { createVideoOf(anyLong(), anyLong(), anyInt(), anyInt(), anyInt(), any()) } + on { createVideoOf(anyLong(), anyLong(), anyInt(), anyInt(), anyInt(), anyInt(), anyInt(), any()) } .thenReturn(GeneratedVideo(File("0.mp4"), 5, VIDEO_DURATION)) } val recorderConfig = ScreenshotRecorderConfig( @@ -105,7 +105,7 @@ class SessionCaptureStrategyTest { null }.whenever(it).submit(any()) } - ) { _, _ -> replayCache } + ) { _ -> replayCache } } } From e96370789a0dcd78599b6001a83739a25b889c26 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 20 Dec 2024 14:02:54 +0100 Subject: [PATCH 09/11] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dffe87b4c4..8649137bfb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - Session Replay: Fix memory leak when masking Compose screens ([#3985](https://github.com/getsentry/sentry-java/pull/3985)) - Session Replay: Fix potential ANRs in `GestureRecorder` ([#4001](https://github.com/getsentry/sentry-java/pull/4001)) +### Internal + +- Session Replay: Flutter improvements ([#4007](https://github.com/getsentry/sentry-java/pull/4007)) + ## 7.19.0 ### Fixes From c48094f4e415d46f293033cf267fe8d46f5653f5 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 30 Dec 2024 14:43:51 +0100 Subject: [PATCH 10/11] Allow overriding SdkVersion for replay events only --- sentry/api/sentry.api | 8 +++--- .../java/io/sentry/ExperimentalOptions.java | 6 +++-- .../java/io/sentry/MainEventProcessor.java | 7 +++++ .../src/main/java/io/sentry/SentryClient.java | 3 ++- .../main/java/io/sentry/SentryOptions.java | 12 +++++++-- .../java/io/sentry/SentryReplayOptions.java | 27 ++++++++++++++++--- .../java/io/sentry/MainEventProcessorTest.kt | 14 +++++++++- .../java/io/sentry/SentryReplayOptionsTest.kt | 6 ++--- 8 files changed, 68 insertions(+), 15 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index ef922d4f593..92a8e909769 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -315,7 +315,7 @@ public abstract interface class io/sentry/EventProcessor { } public final class io/sentry/ExperimentalOptions { - public fun (Z)V + public fun (ZLio/sentry/protocol/SdkVersion;)V public fun getSessionReplay ()Lio/sentry/SentryReplayOptions; public fun setSessionReplay (Lio/sentry/SentryReplayOptions;)V } @@ -2724,8 +2724,8 @@ public final class io/sentry/SentryReplayOptions { public static final field TEXT_VIEW_CLASS_NAME Ljava/lang/String; public static final field VIDEO_VIEW_CLASS_NAME Ljava/lang/String; public static final field WEB_VIEW_CLASS_NAME Ljava/lang/String; - public fun (Ljava/lang/Double;Ljava/lang/Double;)V - public fun (Z)V + public fun (Ljava/lang/Double;Ljava/lang/Double;Lio/sentry/protocol/SdkVersion;)V + public fun (ZLio/sentry/protocol/SdkVersion;)V public fun addMaskViewClass (Ljava/lang/String;)V public fun addUnmaskViewClass (Ljava/lang/String;)V public fun getErrorReplayDuration ()J @@ -2734,6 +2734,7 @@ public final class io/sentry/SentryReplayOptions { public fun getMaskViewContainerClass ()Ljava/lang/String; public fun getOnErrorSampleRate ()Ljava/lang/Double; public fun getQuality ()Lio/sentry/SentryReplayOptions$SentryReplayQuality; + public fun getSdkVersion ()Lio/sentry/protocol/SdkVersion; public fun getSessionDuration ()J public fun getSessionSampleRate ()Ljava/lang/Double; public fun getSessionSegmentDuration ()J @@ -2747,6 +2748,7 @@ public final class io/sentry/SentryReplayOptions { public fun setMaskViewContainerClass (Ljava/lang/String;)V public fun setOnErrorSampleRate (Ljava/lang/Double;)V public fun setQuality (Lio/sentry/SentryReplayOptions$SentryReplayQuality;)V + public fun setSdkVersion (Lio/sentry/protocol/SdkVersion;)V public fun setSessionSampleRate (Ljava/lang/Double;)V public fun setTrackOrientationChange (Z)V public fun setUnmaskViewContainerClass (Ljava/lang/String;)V diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index 4a0e7de78d1..f1bf9a8bc71 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -1,6 +1,8 @@ package io.sentry; +import io.sentry.protocol.SdkVersion; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * Experimental options for new features, these options are going to be promoted to SentryOptions @@ -11,8 +13,8 @@ public final class ExperimentalOptions { private @NotNull SentryReplayOptions sessionReplay; - public ExperimentalOptions(final boolean empty) { - this.sessionReplay = new SentryReplayOptions(empty); + public ExperimentalOptions(final boolean empty, final @Nullable SdkVersion sdkVersion) { + this.sessionReplay = new SentryReplayOptions(empty, sdkVersion); } @NotNull diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index d6445e3a56d..30f95b8b8fb 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -4,6 +4,7 @@ import io.sentry.hints.Cached; import io.sentry.protocol.DebugImage; import io.sentry.protocol.DebugMeta; +import io.sentry.protocol.SdkVersion; import io.sentry.protocol.SentryException; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -159,6 +160,12 @@ private void processNonCachedEvent(final @NotNull SentryBaseEvent event) { if (shouldApplyScopeData(event, hint)) { processNonCachedEvent(event); + final @Nullable SdkVersion replaySdkVersion = + options.getExperimental().getSessionReplay().getSdkVersion(); + if (replaySdkVersion != null) { + // we override the SdkVersion only for replay events as those may come from Hybrid SDKs + event.setSdk(replaySdkVersion); + } } return event; } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 31a5d6f780b..fdbfaf46222 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -641,7 +641,8 @@ public void captureUserFeedback(final @NotNull UserFeedback userFeedback) { final SentryId sentryId = event.getEventId(); final SentryEnvelopeHeader envelopeHeader = - new SentryEnvelopeHeader(sentryId, options.getSdkVersion(), traceContext); + new SentryEnvelopeHeader( + sentryId, options.getExperimental().getSessionReplay().getSdkVersion(), traceContext); return new SentryEnvelope(envelopeHeader, envelopeItems); } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 22738b4ba17..23f195ba8ea 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1421,6 +1421,13 @@ public void setSslSocketFactory(final @Nullable SSLSocketFactory sslSocketFactor */ @ApiStatus.Internal public void setSdkVersion(final @Nullable SdkVersion sdkVersion) { + final @Nullable SdkVersion replaySdkVersion = experimental.getSessionReplay().getSdkVersion(); + if (this.sdkVersion != null + && replaySdkVersion != null + && this.sdkVersion.equals(replaySdkVersion)) { + // if sdkVersion = sessionReplay.sdkVersion we override it, as it means no one else set it + experimental.getSessionReplay().setSdkVersion(sdkVersion); + } this.sdkVersion = sdkVersion; } @@ -2626,7 +2633,8 @@ public SentryOptions() { * @param empty if options should be empty. */ private SentryOptions(final boolean empty) { - experimental = new ExperimentalOptions(empty); + final @NotNull SdkVersion sdkVersion = createSdkVersion(); + experimental = new ExperimentalOptions(empty, sdkVersion); if (!empty) { // SentryExecutorService should be initialized before any // SendCachedEventFireAndForgetIntegration @@ -2647,7 +2655,7 @@ private SentryOptions(final boolean empty) { } setSentryClientName(BuildConfig.SENTRY_JAVA_SDK_NAME + "/" + BuildConfig.VERSION_NAME); - setSdkVersion(createSdkVersion()); + setSdkVersion(sdkVersion); addPackageInfo(); } } diff --git a/sentry/src/main/java/io/sentry/SentryReplayOptions.java b/sentry/src/main/java/io/sentry/SentryReplayOptions.java index f9e82c8600e..73eb7a33e98 100644 --- a/sentry/src/main/java/io/sentry/SentryReplayOptions.java +++ b/sentry/src/main/java/io/sentry/SentryReplayOptions.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.SdkVersion; import io.sentry.util.SampleRateUtils; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; @@ -114,7 +115,13 @@ public enum SentryReplayQuality { */ private boolean trackOrientationChange = true; - public SentryReplayOptions(final boolean empty) { + /** + * SdkVersion object that contains the Sentry Client Name and its version. This object is only + * applied to {@link SentryReplayEvent}s. + */ + private @Nullable SdkVersion sdkVersion; + + public SentryReplayOptions(final boolean empty, final @Nullable SdkVersion sdkVersion) { if (!empty) { setMaskAllText(true); setMaskAllImages(true); @@ -123,14 +130,18 @@ public SentryReplayOptions(final boolean empty) { maskViewClasses.add(ANDROIDX_MEDIA_VIEW_CLASS_NAME); maskViewClasses.add(EXOPLAYER_CLASS_NAME); maskViewClasses.add(EXOPLAYER_STYLED_CLASS_NAME); + this.sdkVersion = sdkVersion; } } public SentryReplayOptions( - final @Nullable Double sessionSampleRate, final @Nullable Double onErrorSampleRate) { - this(false); + final @Nullable Double sessionSampleRate, + final @Nullable Double onErrorSampleRate, + final @Nullable SdkVersion sdkVersion) { + this(false, sdkVersion); this.sessionSampleRate = sessionSampleRate; this.onErrorSampleRate = onErrorSampleRate; + this.sdkVersion = sdkVersion; } @Nullable @@ -282,4 +293,14 @@ public boolean isTrackOrientationChange() { public void setTrackOrientationChange(final boolean trackOrientationChange) { this.trackOrientationChange = trackOrientationChange; } + + @ApiStatus.Internal + public @Nullable SdkVersion getSdkVersion() { + return sdkVersion; + } + + @ApiStatus.Internal + public void setSdkVersion(final @Nullable SdkVersion sdkVersion) { + this.sdkVersion = sdkVersion; + } } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 00214e92c51..e82b184c277 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -27,7 +27,7 @@ import kotlin.test.assertTrue class MainEventProcessorTest { class Fixture { - private val sentryOptions: SentryOptions = SentryOptions().apply { + val sentryOptions: SentryOptions = SentryOptions().apply { dsn = dsnString release = "release" dist = "dist" @@ -619,6 +619,18 @@ class MainEventProcessorTest { assertEquals("value1", replayEvent.tags!!["tag1"]) } + @Test + fun `uses SdkVersion from replay options for replay events`() { + val sut = fixture.getSut(tags = mapOf("tag1" to "value1")) + + fixture.sentryOptions.experimental.sessionReplay.sdkVersion = SdkVersion("dart", "3.2.1") + var replayEvent = SentryReplayEvent() + replayEvent = sut.process(replayEvent, Hint()) + + assertEquals("3.2.1", replayEvent.sdk!!.version) + assertEquals("dart", replayEvent.sdk!!.name) + } + private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = SentryEvent().apply { val mockThrowable = mock() diff --git a/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt index 794a3dac092..48d9d71ac45 100644 --- a/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryReplayOptionsTest.kt @@ -7,7 +7,7 @@ class SentryReplayOptionsTest { @Test fun `uses medium quality as default`() { - val replayOptions = SentryReplayOptions(true) + val replayOptions = SentryReplayOptions(true, null) assertEquals(SentryReplayOptions.SentryReplayQuality.MEDIUM, replayOptions.quality) assertEquals(75_000, replayOptions.quality.bitRate) @@ -16,7 +16,7 @@ class SentryReplayOptionsTest { @Test fun `low quality`() { - val replayOptions = SentryReplayOptions(true).apply { quality = SentryReplayOptions.SentryReplayQuality.LOW } + val replayOptions = SentryReplayOptions(true, null).apply { quality = SentryReplayOptions.SentryReplayQuality.LOW } assertEquals(50_000, replayOptions.quality.bitRate) assertEquals(0.8f, replayOptions.quality.sizeScale) @@ -24,7 +24,7 @@ class SentryReplayOptionsTest { @Test fun `high quality`() { - val replayOptions = SentryReplayOptions(true).apply { quality = SentryReplayOptions.SentryReplayQuality.HIGH } + val replayOptions = SentryReplayOptions(true, null).apply { quality = SentryReplayOptions.SentryReplayQuality.HIGH } assertEquals(100_000, replayOptions.quality.bitRate) assertEquals(1.0f, replayOptions.quality.sizeScale) From 870fdd8f41c463ab799ebfae0359f05370f777e7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 30 Dec 2024 17:56:16 +0100 Subject: [PATCH 11/11] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c1b0807c3e..4336a3aa821 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Fix warm start detection ([#3937](https://github.com/getsentry/sentry-java/pull/3937)) +### Internal + +- Session Replay: Allow overriding `SdkVersion` for replay events ([#4014](https://github.com/getsentry/sentry-java/pull/4014)) + ## 7.19.1 ### Fixes