From 7027bd7ff015b873e3295c8f66287fc284ec1261 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 5 Aug 2024 11:05:44 +0200 Subject: [PATCH 01/21] Persist buffer replay type when switching to session --- .../android/replay/capture/BaseCaptureStrategy.kt | 14 +++++++------- .../replay/capture/BufferCaptureStrategy.kt | 10 ++++++---- .../android/replay/capture/CaptureStrategy.kt | 4 +++- .../replay/capture/SessionCaptureStrategy.kt | 6 ++++-- .../sentry/android/replay/ReplayIntegrationTest.kt | 12 ++++++++---- .../replay/capture/BufferCaptureStrategyTest.kt | 12 ++++++++++++ 6 files changed, 40 insertions(+), 18 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 62bba00cd0..ce45ba9e81 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 @@ -87,7 +87,7 @@ internal abstract class BaseCaptureStrategy( override var currentSegment: Int by persistableAtomic(initialValue = -1, propertyName = SEGMENT_KEY_ID) override val replayCacheDir: File? get() = cache?.replayCacheDir - private var replayType by persistableAtomic(propertyName = SEGMENT_KEY_REPLAY_TYPE) + override var replayType by persistableAtomic(propertyName = SEGMENT_KEY_REPLAY_TYPE) protected val currentEvents: LinkedList = PersistableLinkedList( propertyName = SEGMENT_KEY_REPLAY_RECORDING, options, @@ -105,15 +105,15 @@ internal abstract class BaseCaptureStrategy( override fun start( recorderConfig: ScreenshotRecorderConfig, segmentId: Int, - replayId: SentryId + replayId: SentryId, + replayType: ReplayType? ) { cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig) - // TODO: this should be persisted even after conversion - replayType = if (this is SessionCaptureStrategy) SESSION else BUFFER + this.replayType = replayType ?: (if (this is SessionCaptureStrategy) SESSION else BUFFER) this.recorderConfig = recorderConfig - currentSegment = segmentId - currentReplayId = replayId + this.currentSegment = segmentId + this.currentReplayId = replayId segmentTimestamp = DateUtils.getCurrentDateTime() replayStartTimestamp.set(dateProvider.currentTimeMillis) @@ -140,7 +140,7 @@ internal abstract class BaseCaptureStrategy( segmentId: Int, height: Int, width: Int, - replayType: ReplayType = SESSION, + replayType: ReplayType = this.replayType, cache: ReplayCache? = this.cache, frameRate: Int = recorderConfig.frameRate, screenAtStart: String? = this.screenAtStart, 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 a49c7bf789..a74e6f1603 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 @@ -8,6 +8,7 @@ import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.ERROR import io.sentry.SentryLevel.INFO import io.sentry.SentryOptions +import io.sentry.SentryReplayEvent.ReplayType import io.sentry.SentryReplayEvent.ReplayType.BUFFER import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ScreenshotRecorderConfig @@ -46,9 +47,10 @@ internal class BufferCaptureStrategy( override fun start( recorderConfig: ScreenshotRecorderConfig, segmentId: Int, - replayId: SentryId + replayId: SentryId, + replayType: ReplayType? ) { - super.start(recorderConfig, segmentId, replayId) + super.start(recorderConfig, segmentId, replayId, replayType) hub?.configureScope { val screen = it.screen?.substringAfterLast('.') @@ -159,7 +161,7 @@ internal class BufferCaptureStrategy( } // we hand over replayExecutor to the new strategy to preserve order of execution val captureStrategy = SessionCaptureStrategy(options, hub, dateProvider, replayExecutor) - captureStrategy.start(recorderConfig, segmentId = currentSegment, replayId = currentReplayId) + captureStrategy.start(recorderConfig, segmentId = currentSegment, replayId = currentReplayId, replayType = BUFFER) return captureStrategy } @@ -250,7 +252,7 @@ internal class BufferCaptureStrategy( replayExecutor.submitSafely(options, "$TAG.$taskName") { val segment = - createSegmentInternal(duration, currentSegmentTimestamp, replayId, segmentId, height, width, BUFFER) + createSegmentInternal(duration, currentSegmentTimestamp, replayId, segmentId, height, width) onSegmentCreated(segment) } } 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 c3be520b84..e95ad8ce04 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 @@ -25,11 +25,13 @@ internal interface CaptureStrategy { var currentSegment: Int var currentReplayId: SentryId val replayCacheDir: File? + var replayType: ReplayType fun start( recorderConfig: ScreenshotRecorderConfig, segmentId: Int = 0, - replayId: SentryId = SentryId() + replayId: SentryId = SentryId(), + replayType: ReplayType? = null ) fun stop() 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 d0fd2ce1e1..a8517b263d 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 @@ -7,6 +7,7 @@ import io.sentry.IHub import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.INFO import io.sentry.SentryOptions +import io.sentry.SentryReplayEvent.ReplayType import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment @@ -31,9 +32,10 @@ internal class SessionCaptureStrategy( override fun start( recorderConfig: ScreenshotRecorderConfig, segmentId: Int, - replayId: SentryId + replayId: SentryId, + replayType: ReplayType? ) { - super.start(recorderConfig, segmentId, replayId) + super.start(recorderConfig, segmentId, replayId, replayType) // only set replayId on the scope if it's a full session, otherwise all events will be // tagged with the replay that might never be sent when we're recording in buffer mode hub?.configureScope { 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 a98e344277..03016e349a 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 @@ -41,6 +41,7 @@ import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyLong import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer @@ -160,7 +161,7 @@ class ReplayIntegrationTest { replay.start() - verify(captureStrategy, never()).start(any(), any(), any()) + verify(captureStrategy, never()).start(any(), any(), any(), anyOrNull()) } @Test @@ -186,7 +187,8 @@ class ReplayIntegrationTest { verify(captureStrategy, times(1)).start( any(), eq(0), - argThat { this != SentryId.EMPTY_ID } + argThat { this != SentryId.EMPTY_ID }, + anyOrNull() ) } @@ -201,7 +203,8 @@ class ReplayIntegrationTest { verify(captureStrategy, never()).start( any(), eq(0), - argThat { this != SentryId.EMPTY_ID } + argThat { this != SentryId.EMPTY_ID }, + anyOrNull() ) } @@ -216,7 +219,8 @@ class ReplayIntegrationTest { verify(captureStrategy, times(1)).start( any(), eq(0), - argThat { this != SentryId.EMPTY_ID } + argThat { this != SentryId.EMPTY_ID }, + anyOrNull() ) } 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 5e5130aae8..54f0cd1483 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 @@ -241,6 +241,18 @@ class BufferCaptureStrategyTest { assertEquals(strategy.currentReplayId, fixture.scope.replayId) } + @Test + fun `convert persists buffer replayType when converting to session strategy`() { + val strategy = fixture.getSut() + strategy.start(fixture.recorderConfig) + + val converted = strategy.convert() + assertEquals( + ReplayType.BUFFER, + converted.replayType + ) + } + @Test fun `captureReplay does not replayId to scope when not sampled`() { val strategy = fixture.getSut(errorSampleRate = 0.0) From dd8f9f38b2b6d0c4c02da6653f443330dea605b3 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 5 Aug 2024 12:13:37 +0200 Subject: [PATCH 02/21] Ensure no gaps in segment timestamps when converting strategies --- .../io/sentry/android/replay/ReplayIntegration.kt | 3 ++- .../android/replay/capture/BaseCaptureStrategy.kt | 2 +- .../replay/capture/BufferCaptureStrategy.kt | 6 +++--- .../android/replay/capture/CaptureStrategy.kt | 5 ++--- .../replay/capture/SessionCaptureStrategy.kt | 8 ++++---- .../replay/capture/BufferCaptureStrategyTest.kt | 14 ++++++++++++++ 6 files changed, 26 insertions(+), 12 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 e99aec2c90..ce0f70e10b 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 @@ -176,8 +176,9 @@ public class ReplayIntegration( return } - captureStrategy?.captureReplay(isTerminating == true, onSegmentSent = { + captureStrategy?.captureReplay(isTerminating == true, onSegmentSent = { newTimestamp -> captureStrategy?.currentSegment = captureStrategy?.currentSegment!! + 1 + captureStrategy?.segmentTimestamp = newTimestamp }) captureStrategy = captureStrategy?.convert() } 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 ce45ba9e81..d888c2e33d 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 @@ -78,7 +78,7 @@ internal abstract class BaseCaptureStrategy( cache?.persistSegmentValues(SEGMENT_KEY_FRAME_RATE, newValue.frameRate.toString()) cache?.persistSegmentValues(SEGMENT_KEY_BIT_RATE, newValue.bitRate.toString()) } - protected var segmentTimestamp by persistableAtomicNullable(propertyName = SEGMENT_KEY_TIMESTAMP) { _, _, newValue -> + override var segmentTimestamp by persistableAtomicNullable(propertyName = SEGMENT_KEY_TIMESTAMP) { _, _, newValue -> cache?.persistSegmentValues(SEGMENT_KEY_TIMESTAMP, if (newValue == null) null else DateUtils.getTimestamp(newValue)) } protected val replayStartTimestamp = AtomicLong() 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 a74e6f1603..a70c35a5f6 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 @@ -21,6 +21,7 @@ import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils import java.io.File import java.security.SecureRandom +import java.util.Date import java.util.concurrent.ScheduledExecutorService internal class BufferCaptureStrategy( @@ -92,7 +93,7 @@ internal class BufferCaptureStrategy( override fun captureReplay( isTerminating: Boolean, - onSegmentSent: () -> Unit + onSegmentSent: (Date) -> Unit ) { val sampled = random.sample(options.experimental.sessionReplay.errorSampleRate) @@ -123,8 +124,7 @@ internal class BufferCaptureStrategy( // we only want to increment segment_id in the case of success, but currentSegment // might be irrelevant since we changed strategies, so in the callback we increment // it on the new strategy already - // TODO: also pass new segmentTimestamp to the new strategy - onSegmentSent() + onSegmentSent(segment.replay.timestamp) } } } 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 e95ad8ce04..7e9168df22 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 @@ -26,6 +26,7 @@ internal interface CaptureStrategy { var currentReplayId: SentryId val replayCacheDir: File? var replayType: ReplayType + var segmentTimestamp: Date? fun start( recorderConfig: ScreenshotRecorderConfig, @@ -40,7 +41,7 @@ internal interface CaptureStrategy { fun resume() - fun captureReplay(isTerminating: Boolean, onSegmentSent: () -> Unit) + fun captureReplay(isTerminating: Boolean, onSegmentSent: (Date) -> Unit) fun onScreenshotRecorded(bitmap: Bitmap? = null, store: ReplayCache.(frameTimestamp: Long) -> Unit) @@ -196,7 +197,6 @@ internal interface CaptureStrategy { replay.urls = urls return ReplaySegment.Created( - videoDuration = videoDuration, replay = replay, recording = recording ) @@ -221,7 +221,6 @@ internal interface CaptureStrategy { sealed class ReplaySegment { object Failed : ReplaySegment() data class Created( - val videoDuration: Long, val replay: SentryReplayEvent, val recording: ReplayRecording ) : ReplaySegment() { 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 a8517b263d..b50ed98929 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 @@ -1,7 +1,6 @@ package io.sentry.android.replay.capture import android.graphics.Bitmap -import io.sentry.DateUtils import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED import io.sentry.IHub import io.sentry.SentryLevel.DEBUG @@ -15,6 +14,7 @@ import io.sentry.android.replay.util.submitSafely import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils +import java.util.Date import java.util.concurrent.ScheduledExecutorService internal class SessionCaptureStrategy( @@ -67,7 +67,7 @@ internal class SessionCaptureStrategy( super.stop() } - override fun captureReplay(isTerminating: Boolean, onSegmentSent: () -> Unit) { + override fun captureReplay(isTerminating: Boolean, onSegmentSent: (Date) -> Unit) { options.logger.log(DEBUG, "Replay is already running in 'session' mode, not capturing for event") this.isTerminating.set(isTerminating) } @@ -112,7 +112,7 @@ internal class SessionCaptureStrategy( segment.capture(hub) currentSegment++ // set next segment timestamp as close to the previous one as possible to avoid gaps - segmentTimestamp = DateUtils.getDateTime(currentSegmentTimestamp.time + segment.videoDuration) + segmentTimestamp = segment.replay.timestamp } } @@ -131,7 +131,7 @@ internal class SessionCaptureStrategy( currentSegment++ // set next segment timestamp as close to the previous one as possible to avoid gaps - segmentTimestamp = DateUtils.getDateTime(currentSegmentTimestamp.time + segment.videoDuration) + segmentTimestamp = segment.replay.timestamp } } 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 54f0cd1483..bbad02444d 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 @@ -16,6 +16,7 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_TYPE import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP import io.sentry.android.replay.ReplayFrame import io.sentry.android.replay.ScreenshotRecorderConfig +import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Companion.VIDEO_DURATION import io.sentry.protocol.SentryId import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider @@ -279,4 +280,17 @@ class BufferCaptureStrategyTest { assertEquals(strategy.currentReplayId, fixture.scope.replayId) assertTrue(called) } + + @Test + fun `captureReplay sets new segment timestamp to new strategy after successful creation`() { + val strategy = fixture.getSut() + strategy.start(fixture.recorderConfig) + val oldTimestamp = strategy.segmentTimestamp + + strategy.captureReplay(false) { newTimestamp -> + assertEquals(oldTimestamp!!.time + VIDEO_DURATION, newTimestamp.time) + } + + verify(fixture.hub).captureReplay(any(), any()) + } } From 0729e6d0afe6c55006237e8207f02d8b8f935876 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 5 Aug 2024 19:48:36 +0200 Subject: [PATCH 03/21] Properly store screen name at start for buffer mode --- .../api/sentry-android-replay.api | 5 +- .../io/sentry/android/replay/ReplayCache.kt | 18 ++++--- .../android/replay/ReplayIntegration.kt | 12 ++--- .../replay/capture/BufferCaptureStrategy.kt | 51 +------------------ .../replay/capture/SessionCaptureStrategy.kt | 2 +- .../sentry/android/replay/ReplayCacheTest.kt | 17 +++++++ .../android/replay/ReplayIntegrationTest.kt | 28 +++++++++- 7 files changed, 63 insertions(+), 70 deletions(-) diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index 2a81b45b82..caf0a3e37c 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -36,12 +36,13 @@ 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 Companion Lio/sentry/android/replay/ReplayCache$Companion; public fun (Lio/sentry/SentryOptions;Lio/sentry/protocol/SentryId;Lio/sentry/android/replay/ScreenshotRecorderConfig;)V - public final fun addFrame (Ljava/io/File;J)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 persistSegmentValues (Ljava/lang/String;Ljava/lang/String;)V - public final fun rotate (J)V + public final fun rotate (J)Ljava/lang/String; } public final class io/sentry/android/replay/ReplayCache$Companion { 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 21dbd6ec21..c1bfeb1e52 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 @@ -76,7 +76,7 @@ public class ReplayCache( * @param bitmap the frame screenshot * @param frameTimestamp the timestamp when the frame screenshot was taken */ - internal fun addFrame(bitmap: Bitmap, frameTimestamp: Long) { + internal fun addFrame(bitmap: Bitmap, frameTimestamp: Long, screen: String? = null) { if (replayCacheDir == null || bitmap.isRecycled) { return } @@ -89,7 +89,7 @@ public class ReplayCache( it.flush() } - addFrame(screenshot, frameTimestamp) + addFrame(screenshot, frameTimestamp, screen) } /** @@ -101,8 +101,8 @@ public class ReplayCache( * @param screenshot file containing the frame screenshot * @param frameTimestamp the timestamp when the frame screenshot was taken */ - public fun addFrame(screenshot: File, frameTimestamp: Long) { - val frame = ReplayFrame(screenshot, frameTimestamp) + public fun addFrame(screenshot: File, frameTimestamp: Long, screen: String? = null) { + val frame = ReplayFrame(screenshot, frameTimestamp, screen) frames += frame } @@ -233,15 +233,20 @@ public class ReplayCache( * Removes frames from the in-memory and disk cache from start to [until]. * * @param until value until whose the frames should be removed, represented as unix timestamp + * @return the first screen in the rotated buffer, if any */ - fun rotate(until: Long) { + fun rotate(until: Long): String? { + var screen: String? = null frames.removeAll { if (it.timestamp < until) { deleteFile(it.screenshot) return@removeAll true + } else if (screen == null) { + screen = it.screen } return@removeAll false } + return screen } override fun close() { @@ -426,7 +431,8 @@ internal data class LastSegmentData( internal data class ReplayFrame( val screenshot: File, - val timestamp: Long + val timestamp: Long, + val screen: String? = null ) public data class GeneratedVideo( 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 ce0f70e10b..6e6d9ea052 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 @@ -12,7 +12,6 @@ import io.sentry.Integration import io.sentry.NoOpReplayBreadcrumbConverter import io.sentry.ReplayBreadcrumbConverter import io.sentry.ReplayController -import io.sentry.ScopeObserverAdapter import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.INFO @@ -28,7 +27,6 @@ import io.sentry.cache.PersistingScopeObserver import io.sentry.cache.PersistingScopeObserver.BREADCRUMBS_FILENAME import io.sentry.cache.PersistingScopeObserver.REPLAY_FILENAME import io.sentry.hints.Backfillable -import io.sentry.protocol.Contexts import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils @@ -102,12 +100,6 @@ public class ReplayIntegration( } this.hub = hub - this.options.addScopeObserver(object : ScopeObserverAdapter() { - override fun setContexts(contexts: Contexts) { - // scope screen has fully-qualified name - captureStrategy?.onScreenChanged(contexts.app?.viewNames?.lastOrNull()?.substringAfterLast('.')) - } - }) recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, this, mainLooperHandler) isEnabled.set(true) @@ -213,8 +205,10 @@ public class ReplayIntegration( } override fun onScreenshotRecorded(bitmap: Bitmap) { + var screen: String? = null + hub?.configureScope { screen = it.screen?.substringAfterLast('.') } captureStrategy?.onScreenshotRecorded(bitmap) { frameTimeStamp -> - addFrame(bitmap, frameTimeStamp) + addFrame(bitmap, frameTimeStamp, screen) } } 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 a70c35a5f6..9acbbe6c11 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 @@ -8,7 +8,6 @@ import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.ERROR import io.sentry.SentryLevel.INFO import io.sentry.SentryOptions -import io.sentry.SentryReplayEvent.ReplayType import io.sentry.SentryReplayEvent.ReplayType.BUFFER import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ScreenshotRecorderConfig @@ -36,42 +35,11 @@ internal class BufferCaptureStrategy( // TODO: capture envelopes for buffered segments instead, but don't send them until buffer is triggered private val bufferedSegments = mutableListOf() - // TODO: rework this bs, it doesn't work with sending replay on restart - private val bufferedScreensLock = Any() - private val bufferedScreens = mutableListOf>() - internal companion object { private const val TAG = "BufferCaptureStrategy" private const val ENVELOPE_PROCESSING_DELAY: Long = 100L } - override fun start( - recorderConfig: ScreenshotRecorderConfig, - segmentId: Int, - replayId: SentryId, - replayType: ReplayType? - ) { - super.start(recorderConfig, segmentId, replayId, replayType) - - hub?.configureScope { - val screen = it.screen?.substringAfterLast('.') - if (screen != null) { - synchronized(bufferedScreensLock) { - bufferedScreens.add(screen to dateProvider.currentTimeMillis) - } - } - } - } - - override fun onScreenChanged(screen: String?) { - synchronized(bufferedScreensLock) { - val lastKnownScreen = bufferedScreens.lastOrNull()?.first - if (screen != null && lastKnownScreen != screen) { - bufferedScreens.add(screen to dateProvider.currentTimeMillis) - } - } - } - override fun pause() { createCurrentSegment("pause") { segment -> if (segment is ReplaySegment.Created) { @@ -138,7 +106,7 @@ internal class BufferCaptureStrategy( val now = dateProvider.currentTimeMillis val bufferLimit = now - options.experimental.sessionReplay.errorReplayDuration - cache?.rotate(bufferLimit) + screenAtStart = cache?.rotate(bufferLimit) bufferedSegments.rotate(bufferLimit) } } @@ -171,21 +139,6 @@ internal class BufferCaptureStrategy( rotateEvents(currentEvents, bufferLimit) } - private fun findAndSetStartScreen(segmentStart: Long) { - synchronized(bufferedScreensLock) { - val startScreen = bufferedScreens.lastOrNull { (_, timestamp) -> - timestamp <= segmentStart - }?.first - // if no screen is found before the segment start, this likely means the buffer is from the - // app start, and the start screen will be taken from the navigation crumbs - if (startScreen != null) { - screenAtStart = startScreen - } - // can clear as we switch to session mode and don't care anymore about buffering - bufferedScreens.clear() - } - } - private fun deleteFile(file: File?) { if (file == null) { return @@ -248,8 +201,6 @@ internal class BufferCaptureStrategy( val height = this.recorderConfig.recordingHeight val width = this.recorderConfig.recordingWidth - findAndSetStartScreen(currentSegmentTimestamp.time) - replayExecutor.submitSafely(options, "$TAG.$taskName") { val segment = createSegmentInternal(duration, currentSegmentTimestamp, replayId, segmentId, height, width) 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 b50ed98929..61a98b6914 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 @@ -40,7 +40,7 @@ internal class SessionCaptureStrategy( // tagged with the replay that might never be sent when we're recording in buffer mode hub?.configureScope { it.replayId = currentReplayId - screenAtStart = it.screen + screenAtStart = it.screen?.substringAfterLast('.') } } 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 99a308f53a..91a17f5192 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 @@ -266,6 +266,23 @@ class ReplayCacheTest { assertTrue(replayCache.replayCacheDir!!.listFiles()!!.none { it.name == "1.jpg" || it.name == "1001.jpg" }) } + @Test + fun `rotate returns first screen in buffer`() { + val replayCache = fixture.getSut( + tmpDir, + frameRate = 1 + ) + + val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) + replayCache.addFrame(bitmap, 1, "MainActivity") + replayCache.addFrame(bitmap, 1001, "SecondActivity") + replayCache.addFrame(bitmap, 2001, "ThirdActivity") + replayCache.addFrame(bitmap, 3001, "FourthActivity") + + val screen = replayCache.rotate(2000) + assertEquals("ThirdActivity", screen) + } + @Test fun `does not persist segment if already closed`() { val replayId = SentryId() 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 03016e349a..e96375dfa6 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 @@ -10,6 +10,8 @@ import io.sentry.Breadcrumb import io.sentry.DateUtils import io.sentry.Hint import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryOptions @@ -78,7 +80,12 @@ class ReplayIntegrationTest { }.whenever(mock).submit(any()) } } - val hub = mock() + val scope = Scope(options) + val hub = mock { + doAnswer { + ((it.arguments[0]) as ScopeCallback).run(scope) + }.whenever(mock).configureScope(any()) + } val replayCache = mock { on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997))) @@ -149,7 +156,6 @@ class ReplayIntegrationTest { replay.register(fixture.hub, fixture.options) assertTrue(replay.isEnabled.get()) - assertEquals(1, fixture.options.scopeObservers.size) assertTrue(recorderCreated) assertTrue(SentryIntegrationPackageStorage.getInstance().integrations.contains("Replay")) } @@ -533,4 +539,22 @@ class ReplayIntegrationTest { assertTrue(scopeCache.exists()) assertFalse(evenOlderReplay.exists()) } + + @Test + fun `onScreenshotRecorded supplies screen from scope to replay cache`() { + val captureStrategy = mock { + doAnswer { + ((it.arguments[1] as ReplayCache.(frameTimestamp: Long) -> Unit)).invoke(fixture.replayCache, 1720693523997) + }.whenever(mock).onScreenshotRecorded(anyOrNull(), any()) + } + val replay = fixture.getSut(context, replayCaptureStrategyProvider = { captureStrategy }) + + fixture.hub.configureScope { it.screen = "MainActivity" } + replay.register(fixture.hub, fixture.options) + replay.start() + + replay.onScreenshotRecorded(mock()) + + verify(fixture.replayCache).addFrame(any(), any(), eq("MainActivity")) + } } From 76539893cf7d1f4e956d4f0b170181aed5ae7053 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 6 Aug 2024 10:20:39 +0200 Subject: [PATCH 04/21] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25cbbdffba..ca4cf89943 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Avoid ArrayIndexOutOfBoundsException on Android cpu data collection ([#3598](https://github.com/getsentry/sentry-java/pull/3598)) - Fix lazy select queries instrumentation ([#3604](https://github.com/getsentry/sentry-java/pull/3604)) +- Session Replay: buffer mode improvements ([#3622](https://github.com/getsentry/sentry-java/pull/3622)) + - Align next segment timestamp with the end of the buffered segment when converting from buffer mode to session mode + - Persist `buffer` replay type for the entire replay when converting from buffer mode to session mode + - Properly store screen names for `buffer` mode ### Chores From 55c2d4bfb9515527334c099cb60dbd6e7c8ab772 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 6 Aug 2024 16:37:35 +0200 Subject: [PATCH 05/21] Decouple gesture tracking from WindowRecorder --- .../android/replay/ReplayIntegration.kt | 30 +++- .../sentry/android/replay/WindowRecorder.kt | 70 +-------- .../replay/capture/BaseCaptureStrategy.kt | 131 +--------------- .../replay/gestures/GestureRecorder.kt | 86 +++++++++++ .../replay/gestures/ReplayGestureConverter.kt | 144 ++++++++++++++++++ .../android/replay/ReplayIntegrationTest.kt | 17 ++- 6 files changed, 278 insertions(+), 200 deletions(-) create mode 100644 sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt create mode 100644 sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt 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 6e6d9ea052..996f7d2ba4 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 @@ -20,6 +20,8 @@ import io.sentry.android.replay.capture.BufferCaptureStrategy import io.sentry.android.replay.capture.CaptureStrategy import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.capture.SessionCaptureStrategy +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.sample import io.sentry.android.replay.util.submitSafely @@ -37,6 +39,7 @@ import java.io.File import java.security.SecureRandom import java.util.LinkedList import java.util.concurrent.atomic.AtomicBoolean +import kotlin.LazyThreadSafetyMode.NONE public class ReplayIntegration( private val context: Context, @@ -62,16 +65,20 @@ public class ReplayIntegration( recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)?, replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)?, replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null, - mainLooperHandler: MainLooperHandler? = null + mainLooperHandler: MainLooperHandler? = null, + gestureRecorderProvider: (() -> GestureRecorder)? = null ) : this(context, dateProvider, recorderProvider, recorderConfigProvider, replayCacheProvider) { this.replayCaptureStrategyProvider = replayCaptureStrategyProvider this.mainLooperHandler = mainLooperHandler ?: MainLooperHandler() + this.gestureRecorderProvider = gestureRecorderProvider } private lateinit var options: SentryOptions private var hub: IHub? = null private var recorder: Recorder? = null + private var gestureRecorder: GestureRecorder? = null private val random by lazy { SecureRandom() } + private val rootViewsSpy by lazy(NONE) { RootViewsSpy.install() } // TODO: probably not everything has to be thread-safe here internal val isEnabled = AtomicBoolean(false) @@ -81,6 +88,7 @@ public class ReplayIntegration( private var replayBreadcrumbConverter: ReplayBreadcrumbConverter = NoOpReplayBreadcrumbConverter.getInstance() private var replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null private var mainLooperHandler: MainLooperHandler = MainLooperHandler() + private var gestureRecorderProvider: (() -> GestureRecorder)? = null private lateinit var recorderConfig: ScreenshotRecorderConfig @@ -100,7 +108,8 @@ public class ReplayIntegration( } this.hub = hub - recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, this, mainLooperHandler) + recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, mainLooperHandler) + gestureRecorder = gestureRecorderProvider?.invoke() ?: GestureRecorder(options, this) isEnabled.set(true) try { @@ -147,6 +156,7 @@ public class ReplayIntegration( captureStrategy?.start(recorderConfig) recorder?.start(recorderConfig) + registerRootViewListeners() } override fun resume() { @@ -197,7 +207,9 @@ public class ReplayIntegration( return } + unregisterRootViewListeners() recorder?.stop() + gestureRecorder?.stop() captureStrategy?.stop() isRecording.set(false) captureStrategy?.close() @@ -252,6 +264,20 @@ public class ReplayIntegration( captureStrategy?.onTouchEvent(event) } + private fun registerRootViewListeners() { + if (recorder is OnRootViewsChangedListener) { + rootViewsSpy.listeners += (recorder as OnRootViewsChangedListener) + } + rootViewsSpy.listeners += gestureRecorder + } + + private fun unregisterRootViewListeners() { + if (recorder is OnRootViewsChangedListener) { + rootViewsSpy.listeners -= (recorder as OnRootViewsChangedListener) + } + rootViewsSpy.listeners -= gestureRecorder + } + private fun cleanupReplays(unfinishedReplayId: String = "") { // clean up old replays options.cacheDirPath?.let { cacheDir -> 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 01147e3a7f..9f8ef4375e 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 @@ -23,18 +23,13 @@ import kotlin.LazyThreadSafetyMode.NONE internal class WindowRecorder( private val options: SentryOptions, private val screenshotRecorderCallback: ScreenshotRecorderCallback? = null, - private val touchRecorderCallback: TouchRecorderCallback? = null, private val mainLooperHandler: MainLooperHandler -) : Recorder { +) : Recorder, OnRootViewsChangedListener { internal companion object { private const val TAG = "WindowRecorder" } - private val rootViewsSpy by lazy(NONE) { - RootViewsSpy.install() - } - private val isRecording = AtomicBoolean(false) private val rootViews = ArrayList>() private var recorder: ScreenshotRecorder? = null @@ -43,15 +38,11 @@ internal class WindowRecorder( Executors.newSingleThreadScheduledExecutor(RecorderExecutorServiceThreadFactory()) } - private val onRootViewsChangedListener = OnRootViewsChangedListener { root, added -> + override fun onRootViewsChanged(root: View, added: Boolean) { if (added) { rootViews.add(WeakReference(root)) recorder?.bind(root) - - root.startGestureTracking() } else { - root.stopGestureTracking() - recorder?.unbind(root) rootViews.removeAll { it.get() == root } @@ -68,11 +59,10 @@ internal class WindowRecorder( } recorder = ScreenshotRecorder(recorderConfig, options, mainLooperHandler, screenshotRecorderCallback) - rootViewsSpy.listeners += onRootViewsChangedListener capturingTask = capturer.scheduleAtFixedRateSafely( options, "$TAG.capture", - 0L, + 100L, 1000L / recorderConfig.frameRate, MILLISECONDS ) { @@ -88,7 +78,6 @@ internal class WindowRecorder( } override fun stop() { - rootViewsSpy.listeners -= onRootViewsChangedListener rootViews.forEach { recorder?.unbind(it.get()) } recorder?.close() rootViews.clear() @@ -103,55 +92,6 @@ internal class WindowRecorder( capturer.gracefullyShutdown(options) } - private fun View.startGestureTracking() { - val window = phoneWindow - if (window == null) { - options.logger.log(DEBUG, "Window is invalid, not tracking gestures") - return - } - - if (touchRecorderCallback == null) { - options.logger.log(DEBUG, "TouchRecorderCallback is null, not tracking gestures") - return - } - - val delegate = window.callback - window.callback = SentryReplayGestureRecorder(options, touchRecorderCallback, delegate) - } - - private fun View.stopGestureTracking() { - val window = phoneWindow - if (window == null) { - options.logger.log(DEBUG, "Window was null in stopGestureTracking") - return - } - - if (window.callback is SentryReplayGestureRecorder) { - val delegate = (window.callback as SentryReplayGestureRecorder).delegate - window.callback = delegate - } - } - - private class SentryReplayGestureRecorder( - private val options: SentryOptions, - private val touchRecorderCallback: TouchRecorderCallback?, - delegate: Window.Callback? - ) : FixedWindowCallback(delegate) { - override fun dispatchTouchEvent(event: MotionEvent?): Boolean { - if (event != null) { - val copy: MotionEvent = MotionEvent.obtainNoHistory(event) - try { - touchRecorderCallback?.onTouchEvent(copy) - } catch (e: Throwable) { - options.logger.log(ERROR, "Error dispatching touch event", e) - } finally { - copy.recycle() - } - } - return super.dispatchTouchEvent(event) - } - } - private class RecorderExecutorServiceThreadFactory : ThreadFactory { private var cnt = 0 override fun newThread(r: Runnable): Thread { @@ -161,7 +101,3 @@ internal class WindowRecorder( } } } - -public interface TouchRecorderCallback { - fun onTouchEvent(event: MotionEvent) -} 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 d888c2e33d..b520a079cc 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 @@ -23,6 +23,7 @@ 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 import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.submitSafely @@ -56,15 +57,12 @@ internal abstract class BaseCaptureStrategy( internal companion object { private const val TAG = "CaptureStrategy" - - // rrweb values - private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50 - private const val CAPTURE_MOVE_EVENT_THRESHOLD = 500 } private val persistingExecutor: ScheduledExecutorService by lazy { Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory()) } + private val gestureConverter = ReplayGestureConverter(dateProvider) protected val isTerminating = AtomicBoolean(false) protected var cache: ReplayCache? = null @@ -94,9 +92,6 @@ internal abstract class BaseCaptureStrategy( persistingExecutor, cacheProvider = { cache } ) - private val currentPositions = LinkedHashMap>(10) - private var touchMoveBaseline = 0L - private var lastCapturedMoveEvent = 0L protected val replayExecutor: ScheduledExecutorService by lazy { executor ?: Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) @@ -169,7 +164,7 @@ internal abstract class BaseCaptureStrategy( } override fun onTouchEvent(event: MotionEvent) { - val rrwebEvents = event.toRRWebIncrementalSnapshotEvent() + val rrwebEvents = gestureConverter.convert(event, recorderConfig) if (rrwebEvents != null) { synchronized(currentEventsLock) { currentEvents += rrwebEvents @@ -199,126 +194,6 @@ internal abstract class BaseCaptureStrategy( } } - private fun MotionEvent.toRRWebIncrementalSnapshotEvent(): List? { - val event = this - return when (event.actionMasked) { - MotionEvent.ACTION_MOVE -> { - // we only throttle move events as those can be overwhelming - val now = dateProvider.currentTimeMillis - if (lastCapturedMoveEvent != 0L && lastCapturedMoveEvent + TOUCH_MOVE_DEBOUNCE_THRESHOLD > now) { - return null - } - lastCapturedMoveEvent = now - - currentPositions.keys.forEach { pId -> - val pIndex = event.findPointerIndex(pId) - - if (pIndex == -1) { - // no data for this pointer - return@forEach - } - - // idk why but rrweb does it like dis - if (touchMoveBaseline == 0L) { - touchMoveBaseline = now - } - - currentPositions[pId]!! += Position().apply { - x = event.getX(pIndex) * recorderConfig.scaleFactorX - y = event.getY(pIndex) * recorderConfig.scaleFactorY - id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE - timeOffset = now - touchMoveBaseline - } - } - - val totalOffset = now - touchMoveBaseline - return if (totalOffset > CAPTURE_MOVE_EVENT_THRESHOLD) { - val moveEvents = mutableListOf() - for ((pointerId, positions) in currentPositions) { - if (positions.isNotEmpty()) { - moveEvents += RRWebInteractionMoveEvent().apply { - this.timestamp = now - this.positions = positions.map { pos -> - pos.timeOffset -= totalOffset - pos - } - this.pointerId = pointerId - } - currentPositions[pointerId]!!.clear() - } - } - touchMoveBaseline = 0L - moveEvents - } else { - null - } - } - - MotionEvent.ACTION_DOWN, - MotionEvent.ACTION_POINTER_DOWN -> { - val pId = event.getPointerId(event.actionIndex) - val pIndex = event.findPointerIndex(pId) - - if (pIndex == -1) { - // no data for this pointer - return null - } - - // new finger down - add a new pointer for tracking movement - currentPositions[pId] = ArrayList() - listOf( - RRWebInteractionEvent().apply { - timestamp = dateProvider.currentTimeMillis - x = event.getX(pIndex) * recorderConfig.scaleFactorX - y = event.getY(pIndex) * recorderConfig.scaleFactorY - id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE - pointerId = pId - interactionType = InteractionType.TouchStart - } - ) - } - MotionEvent.ACTION_UP, - MotionEvent.ACTION_POINTER_UP -> { - val pId = event.getPointerId(event.actionIndex) - val pIndex = event.findPointerIndex(pId) - - if (pIndex == -1) { - // no data for this pointer - return null - } - - // finger lift up - remove the pointer from tracking - currentPositions.remove(pId) - listOf( - RRWebInteractionEvent().apply { - timestamp = dateProvider.currentTimeMillis - x = event.getX(pIndex) * recorderConfig.scaleFactorX - y = event.getY(pIndex) * recorderConfig.scaleFactorY - id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE - pointerId = pId - interactionType = InteractionType.TouchEnd - } - ) - } - MotionEvent.ACTION_CANCEL -> { - // gesture cancelled - remove all pointers from tracking - currentPositions.clear() - listOf( - RRWebInteractionEvent().apply { - timestamp = dateProvider.currentTimeMillis - x = event.x * recorderConfig.scaleFactorX - y = event.y * recorderConfig.scaleFactorY - id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE - pointerId = 0 // the pointerId is not used for TouchCancel, so just set it to 0 - interactionType = InteractionType.TouchCancel - } - ) - } - - else -> null - } - } - private inline fun persistableAtomicNullable( initialValue: T? = null, propertyName: String, diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt new file mode 100644 index 0000000000..e2aacd4c4a --- /dev/null +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt @@ -0,0 +1,86 @@ +package io.sentry.android.replay.gestures + +import android.view.MotionEvent +import android.view.View +import android.view.Window +import io.sentry.SentryLevel.DEBUG +import io.sentry.SentryLevel.ERROR +import io.sentry.SentryOptions +import io.sentry.android.replay.OnRootViewsChangedListener +import io.sentry.android.replay.phoneWindow +import io.sentry.android.replay.util.FixedWindowCallback +import java.lang.ref.WeakReference + +class GestureRecorder( + private val options: SentryOptions, + private val touchRecorderCallback: TouchRecorderCallback, +) : OnRootViewsChangedListener { + + private val rootViews = ArrayList>() + + override fun onRootViewsChanged(root: View, added: Boolean) { + if (added) { + rootViews.add(WeakReference(root)) + root.startGestureTracking() + } else { + root.stopGestureTracking() + rootViews.removeAll { it.get() == root } + } + } + + fun stop() { + rootViews.forEach { it.get()?.stopGestureTracking() } + rootViews.clear() + } + + private fun View.startGestureTracking() { + val window = phoneWindow + if (window == null) { + options.logger.log(DEBUG, "Window is invalid, not tracking gestures") + return + } + + val delegate = window.callback + if (delegate !is SentryReplayGestureRecorder) { + window.callback = SentryReplayGestureRecorder(options, touchRecorderCallback, delegate) + } + } + + private fun View.stopGestureTracking() { + val window = phoneWindow + if (window == null) { + options.logger.log(DEBUG, "Window was null in stopGestureTracking") + return + } + + if (window.callback is SentryReplayGestureRecorder) { + val delegate = (window.callback as SentryReplayGestureRecorder).delegate + window.callback = delegate + } + } + + private class SentryReplayGestureRecorder( + private val options: SentryOptions, + private val touchRecorderCallback: TouchRecorderCallback?, + delegate: Window.Callback? + ) : FixedWindowCallback(delegate) { + override fun dispatchTouchEvent(event: MotionEvent?): Boolean { + if (event != null) { + val copy: MotionEvent = MotionEvent.obtainNoHistory(event) + try { + touchRecorderCallback?.onTouchEvent(copy) + } catch (e: Throwable) { + options.logger.log(ERROR, "Error dispatching touch event", e) + } finally { + copy.recycle() + } + } + return super.dispatchTouchEvent(event) + } + } +} + +public interface TouchRecorderCallback { + fun onTouchEvent(event: MotionEvent) +} + 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 new file mode 100644 index 0000000000..3d57ddaca1 --- /dev/null +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt @@ -0,0 +1,144 @@ +package io.sentry.android.replay.gestures + +import android.view.MotionEvent +import io.sentry.android.replay.ScreenshotRecorderConfig +import io.sentry.rrweb.RRWebIncrementalSnapshotEvent +import io.sentry.rrweb.RRWebInteractionEvent +import io.sentry.rrweb.RRWebInteractionEvent.InteractionType +import io.sentry.rrweb.RRWebInteractionMoveEvent +import io.sentry.rrweb.RRWebInteractionMoveEvent.Position +import io.sentry.transport.ICurrentDateProvider + +class ReplayGestureConverter( + private val dateProvider: ICurrentDateProvider, +) { + + internal companion object { + // rrweb values + private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50 + private const val CAPTURE_MOVE_EVENT_THRESHOLD = 500 + } + + private val currentPositions = LinkedHashMap>(10) + private var touchMoveBaseline = 0L + private var lastCapturedMoveEvent = 0L + + fun convert(event: MotionEvent, recorderConfig: ScreenshotRecorderConfig): List? { + return when (event.actionMasked) { + MotionEvent.ACTION_MOVE -> { + // we only throttle move events as those can be overwhelming + val now = dateProvider.currentTimeMillis + if (lastCapturedMoveEvent != 0L && lastCapturedMoveEvent + TOUCH_MOVE_DEBOUNCE_THRESHOLD > now) { + return null + } + lastCapturedMoveEvent = now + + currentPositions.keys.forEach { pId -> + val pIndex = event.findPointerIndex(pId) + + if (pIndex == -1) { + // no data for this pointer + return@forEach + } + + // idk why but rrweb does it like dis + if (touchMoveBaseline == 0L) { + touchMoveBaseline = now + } + + currentPositions[pId]!! += Position().apply { + x = event.getX(pIndex) * recorderConfig.scaleFactorX + y = event.getY(pIndex) * recorderConfig.scaleFactorY + id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE + timeOffset = now - touchMoveBaseline + } + } + + val totalOffset = now - touchMoveBaseline + return if (totalOffset > CAPTURE_MOVE_EVENT_THRESHOLD) { + val moveEvents = mutableListOf() + for ((pointerId, positions) in currentPositions) { + if (positions.isNotEmpty()) { + moveEvents += RRWebInteractionMoveEvent().apply { + this.timestamp = now + this.positions = positions.map { pos -> + pos.timeOffset -= totalOffset + pos + } + this.pointerId = pointerId + } + currentPositions[pointerId]!!.clear() + } + } + touchMoveBaseline = 0L + moveEvents + } else { + null + } + } + + MotionEvent.ACTION_DOWN, + MotionEvent.ACTION_POINTER_DOWN -> { + val pId = event.getPointerId(event.actionIndex) + val pIndex = event.findPointerIndex(pId) + + if (pIndex == -1) { + // no data for this pointer + return null + } + + // new finger down - add a new pointer for tracking movement + currentPositions[pId] = ArrayList() + listOf( + RRWebInteractionEvent().apply { + timestamp = dateProvider.currentTimeMillis + x = event.getX(pIndex) * recorderConfig.scaleFactorX + y = event.getY(pIndex) * recorderConfig.scaleFactorY + id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE + pointerId = pId + interactionType = InteractionType.TouchStart + } + ) + } + MotionEvent.ACTION_UP, + MotionEvent.ACTION_POINTER_UP -> { + val pId = event.getPointerId(event.actionIndex) + val pIndex = event.findPointerIndex(pId) + + if (pIndex == -1) { + // no data for this pointer + return null + } + + // finger lift up - remove the pointer from tracking + currentPositions.remove(pId) + listOf( + RRWebInteractionEvent().apply { + timestamp = dateProvider.currentTimeMillis + x = event.getX(pIndex) * recorderConfig.scaleFactorX + y = event.getY(pIndex) * recorderConfig.scaleFactorY + id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE + pointerId = pId + interactionType = InteractionType.TouchEnd + } + ) + } + MotionEvent.ACTION_CANCEL -> { + // gesture cancelled - remove all pointers from tracking + currentPositions.clear() + listOf( + RRWebInteractionEvent().apply { + timestamp = dateProvider.currentTimeMillis + x = event.x * recorderConfig.scaleFactorX + y = event.y * recorderConfig.scaleFactorY + id = 0 // html node id, but we don't have it, so hardcode to 0 to align with FE + pointerId = 0 // the pointerId is not used for TouchCancel, so just set it to 0 + interactionType = InteractionType.TouchCancel + } + ) + } + + else -> null + } + } +} 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 e96375dfa6..273c27e563 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 @@ -1,6 +1,7 @@ package io.sentry.android.replay import android.content.Context +import android.gesture.Gesture import android.graphics.Bitmap import android.graphics.Bitmap.CompressFormat.JPEG import android.graphics.Bitmap.Config.ARGB_8888 @@ -27,6 +28,7 @@ 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.capture.CaptureStrategy import io.sentry.android.replay.capture.SessionCaptureStrategyTest.Fixture.Companion.VIDEO_DURATION +import io.sentry.android.replay.gestures.GestureRecorder import io.sentry.cache.PersistingScopeObserver import io.sentry.protocol.SentryException import io.sentry.protocol.SentryId @@ -100,6 +102,7 @@ class ReplayIntegrationTest { recorderProvider: (() -> Recorder)? = null, replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null, recorderConfigProvider: ((configChanged: Boolean) -> ScreenshotRecorderConfig)? = null, + gestureRecorderProvider: (() -> GestureRecorder)? = null, dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance() ): ReplayIntegration { options.run { @@ -112,7 +115,8 @@ class ReplayIntegrationTest { recorderProvider, recorderConfigProvider = recorderConfigProvider, replayCacheProvider = { _, _ -> replayCache }, - replayCaptureStrategyProvider = replayCaptureStrategyProvider + replayCaptureStrategyProvider = replayCaptureStrategyProvider, + gestureRecorderProvider = gestureRecorderProvider ) } } @@ -360,10 +364,16 @@ class ReplayIntegrationTest { } @Test - fun `stop calls stop for recorder and strategy and sets recording to false`() { + fun `stop calls stop for recorders and strategy and sets recording to false`() { val captureStrategy = mock() val recorder = mock() - val replay = fixture.getSut(context, recorderProvider = { recorder }, replayCaptureStrategyProvider = { captureStrategy }) + val gestureRecorder = mock() + val replay = fixture.getSut( + context, + recorderProvider = { recorder }, + replayCaptureStrategyProvider = { captureStrategy }, + gestureRecorderProvider = { gestureRecorder } + ) replay.register(fixture.hub, fixture.options) replay.start() @@ -371,6 +381,7 @@ class ReplayIntegrationTest { verify(captureStrategy).stop() verify(recorder).stop() + verify(gestureRecorder).stop() assertFalse(replay.isRecording) } From 44404f2449cff7c6b033afc1009becb480be7df3 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 6 Aug 2024 14:41:35 +0000 Subject: [PATCH 06/21] Format code --- .../main/java/io/sentry/android/replay/WindowRecorder.kt | 6 ------ .../io/sentry/android/replay/capture/BaseCaptureStrategy.kt | 5 ----- .../io/sentry/android/replay/gestures/GestureRecorder.kt | 3 +-- .../android/replay/gestures/ReplayGestureConverter.kt | 2 +- .../java/io/sentry/android/replay/ReplayIntegrationTest.kt | 1 - 5 files changed, 2 insertions(+), 15 deletions(-) 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 9f8ef4375e..34ceb74b88 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 @@ -1,13 +1,8 @@ package io.sentry.android.replay import android.annotation.TargetApi -import android.view.MotionEvent import android.view.View -import android.view.Window -import io.sentry.SentryLevel.DEBUG -import io.sentry.SentryLevel.ERROR import io.sentry.SentryOptions -import io.sentry.android.replay.util.FixedWindowCallback import io.sentry.android.replay.util.MainLooperHandler import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.scheduleAtFixedRateSafely @@ -17,7 +12,6 @@ import java.util.concurrent.ScheduledFuture import java.util.concurrent.ThreadFactory import java.util.concurrent.TimeUnit.MILLISECONDS import java.util.concurrent.atomic.AtomicBoolean -import kotlin.LazyThreadSafetyMode.NONE @TargetApi(26) internal class WindowRecorder( 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 b520a079cc..97cb386109 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 @@ -29,11 +29,6 @@ import io.sentry.android.replay.util.gracefullyShutdown import io.sentry.android.replay.util.submitSafely import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebEvent -import io.sentry.rrweb.RRWebIncrementalSnapshotEvent -import io.sentry.rrweb.RRWebInteractionEvent -import io.sentry.rrweb.RRWebInteractionEvent.InteractionType -import io.sentry.rrweb.RRWebInteractionMoveEvent -import io.sentry.rrweb.RRWebInteractionMoveEvent.Position import io.sentry.transport.ICurrentDateProvider import java.io.File import java.util.Date diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt index e2aacd4c4a..176d2ad10e 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt @@ -13,7 +13,7 @@ import java.lang.ref.WeakReference class GestureRecorder( private val options: SentryOptions, - private val touchRecorderCallback: TouchRecorderCallback, + private val touchRecorderCallback: TouchRecorderCallback ) : OnRootViewsChangedListener { private val rootViews = ArrayList>() @@ -83,4 +83,3 @@ class GestureRecorder( public interface TouchRecorderCallback { fun onTouchEvent(event: MotionEvent) } - 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 3d57ddaca1..59d6b30bce 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 @@ -10,7 +10,7 @@ import io.sentry.rrweb.RRWebInteractionMoveEvent.Position import io.sentry.transport.ICurrentDateProvider class ReplayGestureConverter( - private val dateProvider: ICurrentDateProvider, + private val dateProvider: ICurrentDateProvider ) { internal companion object { 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 273c27e563..1518b41a81 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 @@ -1,7 +1,6 @@ package io.sentry.android.replay import android.content.Context -import android.gesture.Gesture import android.graphics.Bitmap import android.graphics.Bitmap.CompressFormat.JPEG import android.graphics.Bitmap.Config.ARGB_8888 From 6a2572e4681c08eea510496de5af70638f8ef0ef Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 6 Aug 2024 23:56:34 +0200 Subject: [PATCH 07/21] Add tests --- .../api/sentry-android-replay.api | 15 +- .../android/replay/ScreenshotRecorder.kt | 12 + .../sentry/android/replay/WindowRecorder.kt | 2 +- .../replay/gestures/GestureRecorder.kt | 2 +- .../replay/gestures/GestureRecorderTest.kt | 131 ++++++++++ .../gestures/ReplayGestureConverterTest.kt | 240 ++++++++++++++++++ 6 files changed, 398 insertions(+), 4 deletions(-) create mode 100644 sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt create mode 100644 sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index caf0a3e37c..f103957999 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -49,7 +49,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/TouchRecorderCallback, java/io/Closeable { +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 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 @@ -103,7 +103,18 @@ public final class io/sentry/android/replay/ScreenshotRecorderConfig$Companion { public final fun from (Landroid/content/Context;Lio/sentry/SentryReplayOptions;)Lio/sentry/android/replay/ScreenshotRecorderConfig; } -public abstract interface class io/sentry/android/replay/TouchRecorderCallback { +public final class io/sentry/android/replay/gestures/GestureRecorder : io/sentry/android/replay/OnRootViewsChangedListener { + public fun (Lio/sentry/SentryOptions;Lio/sentry/android/replay/gestures/TouchRecorderCallback;)V + public fun onRootViewsChanged (Landroid/view/View;Z)V + public final fun stop ()V +} + +public final class io/sentry/android/replay/gestures/ReplayGestureConverter { + public fun (Lio/sentry/transport/ICurrentDateProvider;)V + public final fun convert (Landroid/view/MotionEvent;Lio/sentry/android/replay/ScreenshotRecorderConfig;)Ljava/util/List; +} + +public abstract interface class io/sentry/android/replay/gestures/TouchRecorderCallback { public abstract fun onTouchEvent (Landroid/view/MotionEvent;)V } 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 40fb6ef931..9680c2a3ad 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 @@ -288,6 +288,18 @@ public data class ScreenshotRecorderConfig( val frameRate: Int, val bitRate: Int ) { + internal constructor( + scaleFactorX: Float, + scaleFactorY: Float + ) : this( + recordingWidth = 0, + recordingHeight = 0, + scaleFactorX = scaleFactorX, + scaleFactorY = scaleFactorY, + frameRate = 0, + bitRate = 0 + ) + companion object { /** * Since codec block size is 16, so we have to adjust the width and height to it, otherwise 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 34ceb74b88..9e846dfcf0 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 @@ -56,7 +56,7 @@ internal class WindowRecorder( capturingTask = capturer.scheduleAtFixedRateSafely( options, "$TAG.capture", - 100L, + 100L, // delay the first run by a bit, to allow root view listener to register 1000L / recorderConfig.frameRate, MILLISECONDS ) { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt index 176d2ad10e..57302aaac1 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt @@ -59,7 +59,7 @@ class GestureRecorder( } } - private class SentryReplayGestureRecorder( + internal class SentryReplayGestureRecorder( private val options: SentryOptions, private val touchRecorderCallback: TouchRecorderCallback?, delegate: Window.Callback? diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt new file mode 100644 index 0000000000..bb2de2b7c8 --- /dev/null +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt @@ -0,0 +1,131 @@ +package io.sentry.android.replay.gestures + +import android.R +import android.app.Activity +import android.os.Bundle +import android.view.MotionEvent +import android.view.View +import android.widget.LinearLayout +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.SentryOptions +import io.sentry.android.core.internal.gestures.NoOpWindowCallback +import io.sentry.android.replay.gestures.GestureRecorder.SentryReplayGestureRecorder +import io.sentry.android.replay.phoneWindow +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.annotation.Config +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +@Config(sdk = [30]) +class GestureRecorderTest { + internal class Fixture { + + val options = SentryOptions() + + fun getSut( + touchRecorderCallback: TouchRecorderCallback = NoOpTouchRecorderCallback() + ): GestureRecorder { + return GestureRecorder(options, touchRecorderCallback) + } + } + + private val fixture = Fixture() + private class NoOpTouchRecorderCallback : TouchRecorderCallback { + override fun onTouchEvent(event: MotionEvent) = Unit + } + + @Test + fun `when new window added and window callback is already wrapped, does not wrap it again`() { + val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get() + val gestureRecorder = fixture.getSut() + + activity.root.phoneWindow?.callback = SentryReplayGestureRecorder(fixture.options, null, null) + gestureRecorder.onRootViewsChanged(activity.root, true) + + assertFalse((activity.root.phoneWindow?.callback as SentryReplayGestureRecorder).delegate is SentryReplayGestureRecorder) + } + + @Test + fun `when new window added tracks touch events`() { + var called = false + val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get() + val motionEvent = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 0f, 0f, 0) + val gestureRecorder = fixture.getSut( + touchRecorderCallback = object : TouchRecorderCallback { + override fun onTouchEvent(event: MotionEvent) { + assertEquals(MotionEvent.ACTION_DOWN, event.action) + called = true + } + } + ) + + gestureRecorder.onRootViewsChanged(activity.root, true) + + activity.root.phoneWindow?.callback?.dispatchTouchEvent(motionEvent) + assertTrue(called) + } + + @Test + fun `when window removed and window is not sentry recorder does nothing`() { + val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get() + val gestureRecorder = fixture.getSut() + + activity.root.phoneWindow?.callback = NoOpWindowCallback() + gestureRecorder.onRootViewsChanged(activity.root, false) + + assertTrue(activity.root.phoneWindow?.callback is NoOpWindowCallback) + } + + @Test + fun `when window removed stops tracking touch events`() { + val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get() + val gestureRecorder = fixture.getSut() + + gestureRecorder.onRootViewsChanged(activity.root, true) + gestureRecorder.onRootViewsChanged(activity.root, false) + + assertFalse(activity.root.phoneWindow?.callback is SentryReplayGestureRecorder) + } + + @Test + fun `when stopped stops tracking all windows`() { + val activity1 = Robolectric.buildActivity(TestActivity::class.java).setup().get() + val activity2 = Robolectric.buildActivity(TestActivity2::class.java).setup().get() + val gestureRecorder = fixture.getSut() + + gestureRecorder.onRootViewsChanged(activity1.root, true) + gestureRecorder.onRootViewsChanged(activity2.root, true) + gestureRecorder.stop() + + assertFalse(activity1.root.phoneWindow?.callback is SentryReplayGestureRecorder) + assertFalse(activity2.root.phoneWindow?.callback is SentryReplayGestureRecorder) + } +} + +private class TestActivity : Activity() { + lateinit var root: View + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setTheme(R.style.Theme_Holo_Light) + root = LinearLayout(this) + setContentView(root) + actionBar!!.setIcon(R.drawable.ic_lock_power_off) + } +} + +private class TestActivity2 : Activity() { + lateinit var root: View + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setTheme(R.style.Theme_Holo_Light) + root = LinearLayout(this) + setContentView(root) + actionBar!!.setIcon(R.drawable.ic_lock_power_off) + } +} diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt new file mode 100644 index 0000000000..00ae93af4a --- /dev/null +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/ReplayGestureConverterTest.kt @@ -0,0 +1,240 @@ +package io.sentry.android.replay.gestures + +import android.view.MotionEvent +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.android.replay.ScreenshotRecorderConfig +import io.sentry.rrweb.RRWebInteractionEvent +import io.sentry.rrweb.RRWebInteractionMoveEvent +import io.sentry.transport.ICurrentDateProvider +import org.junit.runner.RunWith +import org.robolectric.annotation.Config +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +@Config(sdk = [30]) +class ReplayGestureConverterTest { + internal class Fixture { + var now: Long = 1000L + + fun getSut( + dateProvider: ICurrentDateProvider = ICurrentDateProvider { now } + ): ReplayGestureConverter { + return ReplayGestureConverter(dateProvider) + } + } + + private val fixture = Fixture() + + @Test + fun `convert ACTION_DOWN event`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 100f, 200f, 0) + + val result = sut.convert(event, recorderConfig) + + assertNotNull(result) + assertEquals(1, result.size) + assertTrue(result[0] is RRWebInteractionEvent) + with(result[0] as RRWebInteractionEvent) { + assertEquals(1000L, timestamp) + assertEquals(100f, x) + assertEquals(200f, y) + assertEquals(0, id) + assertEquals(0, pointerId) + assertEquals(RRWebInteractionEvent.InteractionType.TouchStart, interactionType) + } + + event.recycle() + } + + @Test + fun `convert ACTION_MOVE event with debounce`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, 100f, 200f, 0) + + // First call should pass + var result = sut.convert(event, recorderConfig) + assertNotNull(result) + + // Second call within debounce threshold should be null + fixture.now += 40 // Increase time by 40ms + result = sut.convert(event, recorderConfig) + assertNull(result) + + event.recycle() + } + + @Test + fun `convert ACTION_MOVE event with capture threshold`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val downEvent = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 100f, 200f, 0) + val moveEvent = MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, 110f, 210f, 0) + + // Add a pointer to currentPositions + sut.convert(downEvent, recorderConfig) + + // First call should not trigger capture + var result = sut.convert(moveEvent, recorderConfig) + assertNull(result) + + // Second call should trigger capture + fixture.now += 600 // Increase time by 600ms + result = sut.convert(moveEvent, recorderConfig) + assertNotNull(result) + with(result[0] as RRWebInteractionMoveEvent) { + assertEquals(1600L, timestamp) + assertEquals(2, positions!!.size) + assertEquals(110f, positions!![0].x) + assertEquals(210f, positions!![0].y) + assertEquals(0, positions!![0].id) + assertEquals(0, pointerId) + } + + downEvent.recycle() + moveEvent.recycle() + } + + @Test + fun `convert ACTION_UP event`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_UP, 100f, 200f, 0) + + val result = sut.convert(event, recorderConfig) + + assertNotNull(result) + assertEquals(1, result.size) + assertTrue(result[0] is RRWebInteractionEvent) + with(result[0] as RRWebInteractionEvent) { + assertEquals(1000L, timestamp) + assertEquals(100f, x) + assertEquals(200f, y) + assertEquals(0, id) + assertEquals(0, pointerId) + assertEquals(RRWebInteractionEvent.InteractionType.TouchEnd, interactionType) + } + + event.recycle() + } + + @Test + fun `convert ACTION_CANCEL event`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_CANCEL, 100f, 200f, 0) + + val result = sut.convert(event, recorderConfig) + + assertNotNull(result) + assertEquals(1, result.size) + assertTrue(result[0] is RRWebInteractionEvent) + with(result[0] as RRWebInteractionEvent) { + assertEquals(1000L, timestamp) + assertEquals(100f, x) + assertEquals(200f, y) + assertEquals(0, id) + assertEquals(0, pointerId) + assertEquals(RRWebInteractionEvent.InteractionType.TouchCancel, interactionType) + } + + event.recycle() + } + + @Test + fun `convert event with different scale factors`() { + val sut = fixture.getSut() + val customRecorderConfig = ScreenshotRecorderConfig(scaleFactorX = 0.5f, scaleFactorY = 1.5f) + val event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 100f, 200f, 0) + + val result = sut.convert(event, customRecorderConfig) + + assertNotNull(result) + assertEquals(1, result.size) + assertTrue(result[0] is RRWebInteractionEvent) + with(result[0] as RRWebInteractionEvent) { + assertEquals(1000L, timestamp) + assertEquals(50f, x) // 100 * 0.5 + assertEquals(300f, y) // 200 * 1.5 + assertEquals(0, id) + assertEquals(0, pointerId) + assertEquals(RRWebInteractionEvent.InteractionType.TouchStart, interactionType) + } + + event.recycle() + } + + @Test + fun `convert multi-pointer events`() { + val sut = fixture.getSut() + val recorderConfig = ScreenshotRecorderConfig(scaleFactorX = 1f, scaleFactorY = 1f) + + // Simulate first finger down + var event = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 100f, 100f, 0) + var result = sut.convert(event, recorderConfig) + assertNotNull(result) + assertTrue(result[0] is RRWebInteractionEvent) + assertEquals(RRWebInteractionEvent.InteractionType.TouchStart, (result[0] as RRWebInteractionEvent).interactionType) + event.recycle() + + // Simulate second finger down + val properties = MotionEvent.PointerProperties() + properties.id = 1 + properties.toolType = MotionEvent.TOOL_TYPE_FINGER + val pointerProperties = arrayOf(MotionEvent.PointerProperties(), properties) + val pointerCoords = arrayOf( + MotionEvent.PointerCoords().apply { x = 100f; y = 100f }, + MotionEvent.PointerCoords().apply { x = 200f; y = 200f } + ) + event = MotionEvent.obtain(0, 1, MotionEvent.ACTION_POINTER_DOWN or (1 shl MotionEvent.ACTION_POINTER_INDEX_SHIFT), 2, pointerProperties, pointerCoords, 0, 0, 1f, 1f, 0, 0, 0, 0) + fixture.now += 100 // Increase time by 100ms + result = sut.convert(event, recorderConfig) + assertNotNull(result) + assertTrue(result[0] is RRWebInteractionEvent) + assertEquals(RRWebInteractionEvent.InteractionType.TouchStart, (result[0] as RRWebInteractionEvent).interactionType) + assertEquals(1, (result[0] as RRWebInteractionEvent).pointerId) + event.recycle() + + // Simulate move event + pointerCoords[0].x = 90f + pointerCoords[0].y = 90f + pointerCoords[1].x = 210f + pointerCoords[1].y = 210f + event = MotionEvent.obtain(0, 2, MotionEvent.ACTION_MOVE, 2, pointerProperties, pointerCoords, 0, 0, 1f, 1f, 0, 0, 0, 0) + // First call should not trigger capture + result = sut.convert(event, recorderConfig) + assertNull(result) + + fixture.now += 600 // Increase time by 600ms to trigger move capture + result = sut.convert(event, recorderConfig) + assertNotNull(result) + assertTrue((result[0] as RRWebInteractionMoveEvent).positions!!.size == 2) + event.recycle() + + // Simulate second finger up + event = MotionEvent.obtain(0, 3, MotionEvent.ACTION_POINTER_UP or (1 shl MotionEvent.ACTION_POINTER_INDEX_SHIFT), 2, pointerProperties, pointerCoords, 0, 0, 1f, 1f, 0, 0, 0, 0) + fixture.now += 100 // Increase time by 100ms + result = sut.convert(event, recorderConfig) + assertNotNull(result) + assertTrue(result[0] is RRWebInteractionEvent) + assertEquals(RRWebInteractionEvent.InteractionType.TouchEnd, (result[0] as RRWebInteractionEvent).interactionType) + assertEquals(1, (result[0] as RRWebInteractionEvent).pointerId) + event.recycle() + + // Simulate first finger up + event = MotionEvent.obtain(0, 4, MotionEvent.ACTION_UP, 90f, 90f, 0) + fixture.now += 100 // Increase time by 100ms + result = sut.convert(event, recorderConfig) + assertNotNull(result) + assertTrue(result[0] is RRWebInteractionEvent) + assertEquals(RRWebInteractionEvent.InteractionType.TouchEnd, (result[0] as RRWebInteractionEvent).interactionType) + assertEquals(0, (result[0] as RRWebInteractionEvent).pointerId) + event.recycle() + } +} From 3565c9620f156b1fca44559c87243ac11f6ed301 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 7 Aug 2024 00:04:50 +0200 Subject: [PATCH 08/21] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca4cf89943..09ecbba6f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Session Replay: Gesture/touch support for Flutter ([#3623](https://github.com/getsentry/sentry-java/pull/3623)) + ### Fixes - Avoid ArrayIndexOutOfBoundsException on Android cpu data collection ([#3598](https://github.com/getsentry/sentry-java/pull/3598)) From fcdc370ee0ea177d86303cd3856139d9ed34e676 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 8 Aug 2024 10:22:57 +0200 Subject: [PATCH 09/21] Check if bitmap is null for isRedactable --- .../src/main/java/io/sentry/android/replay/util/Views.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt index 58accf0b77..dd2925d85c 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt @@ -53,7 +53,12 @@ internal fun Drawable?.isRedactable(): Boolean { // TODO: otherwise maybe check for the bitmap size and don't redact those that take a lot of height (e.g. a background of a whatsapp chat) return when (this) { is InsetDrawable, is ColorDrawable, is VectorDrawable, is GradientDrawable -> false - is BitmapDrawable -> !bitmap.isRecycled && bitmap.height > 10 && bitmap.width > 10 + is BitmapDrawable -> { + if (bitmap == null) { + return false + } + return !bitmap.isRecycled && bitmap.height > 10 && bitmap.width > 10 + } else -> true } } From 5f1dfd162531f45545f86617abd0f3cf3c6e1ed8 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 8 Aug 2024 13:42:57 +0200 Subject: [PATCH 10/21] Safely get totalPaddingTop --- .../java/io/sentry/android/replay/util/Views.kt | 14 ++++++++++++++ .../replay/viewhierarchy/ViewHierarchyNode.kt | 3 ++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt index dd2925d85c..f45c26d82e 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt @@ -14,6 +14,8 @@ import android.os.Build.VERSION import android.os.Build.VERSION_CODES import android.text.Layout import android.view.View +import android.widget.TextView +import java.lang.NullPointerException /** * Adapted copy of AccessibilityNodeInfo from https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/view/View.java;l=10718 @@ -89,3 +91,15 @@ internal fun Layout?.getVisibleRects(globalRect: Rect, paddingLeft: Int, padding } return rects } + +/** + * [TextView.getVerticalOffset] which is used by [TextView.getTotalPaddingTop] may throw an NPE on + * some devices (Redmi), so we try-catch it specifically for an NPE and then fallback to + * [TextView.getExtendedPaddingTop]. + */ +internal val TextView.totalPaddingTopSafe: Int + get() = try { + totalPaddingTop + } catch (e: NullPointerException) { + extendedPaddingTop + } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt index 1a94b295f7..145cefff3d 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ViewHierarchyNode.kt @@ -9,6 +9,7 @@ import android.widget.TextView import io.sentry.SentryOptions import io.sentry.android.replay.util.isRedactable import io.sentry.android.replay.util.isVisibleToUser +import io.sentry.android.replay.util.totalPaddingTopSafe @TargetApi(26) sealed class ViewHierarchyNode( @@ -245,7 +246,7 @@ sealed class ViewHierarchyNode( layout = view.layout, dominantColor = view.currentTextColor.toOpaque(), paddingLeft = view.totalPaddingLeft, - paddingTop = view.totalPaddingTop, + paddingTop = view.totalPaddingTopSafe, x = view.x, y = view.y, width = view.width, From ab507646a16cb1ef00ebbb12bc9235e02031ab89 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 8 Aug 2024 13:57:29 +0200 Subject: [PATCH 11/21] Fix nullability for isVisibleToUser --- .../src/main/java/io/sentry/android/replay/util/Views.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt index f45c26d82e..e74fc695b5 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt @@ -28,7 +28,7 @@ internal fun View.isVisibleToUser(): Pair { } // An invisible predecessor or one with alpha zero means // that this view is not visible to the user. - var current: Any = this + var current: Any? = this while (current is View) { val view = current val transitionAlpha = if (VERSION.SDK_INT >= VERSION_CODES.Q) view.transitionAlpha else 1f @@ -95,7 +95,7 @@ internal fun Layout?.getVisibleRects(globalRect: Rect, paddingLeft: Int, padding /** * [TextView.getVerticalOffset] which is used by [TextView.getTotalPaddingTop] may throw an NPE on * some devices (Redmi), so we try-catch it specifically for an NPE and then fallback to - * [TextView.getExtendedPaddingTop]. + * [TextView.getExtendedPaddingTop] */ internal val TextView.totalPaddingTopSafe: Int get() = try { From 3febd87b92059b9f6bbd40c301745581c33f2009 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 8 Aug 2024 15:00:50 +0200 Subject: [PATCH 12/21] Use submitSafely for ScreenshotRecorder --- .../main/java/io/sentry/android/replay/ScreenshotRecorder.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 9680c2a3ad..5ec142b3e5 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 @@ -26,6 +26,7 @@ import io.sentry.SentryReplayOptions import io.sentry.android.replay.util.MainLooperHandler import io.sentry.android.replay.util.getVisibleRects import io.sentry.android.replay.util.gracefullyShutdown +import io.sentry.android.replay.util.submitSafely import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.ImageViewHierarchyNode import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.TextViewHierarchyNode @@ -122,7 +123,7 @@ internal class ScreenshotRecorder( val viewHierarchy = ViewHierarchyNode.fromView(root, null, 0, options) root.traverse(viewHierarchy) - recorder.submit { + recorder.submitSafely(options, "screenshot_recorder.redact") { val canvas = Canvas(bitmap) canvas.setMatrix(prescaledMatrix) viewHierarchy.traverse { node -> From 8db1c59d90d3389df5bd1c304ef36c087cfac649 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 8 Aug 2024 15:00:55 +0200 Subject: [PATCH 13/21] Remove redundant code --- .../io/sentry/android/replay/capture/SessionCaptureStrategy.kt | 1 - 1 file changed, 1 deletion(-) 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 61a98b6914..7b416d18b7 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 @@ -124,7 +124,6 @@ internal class SessionCaptureStrategy( } override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) { - val currentSegmentTimestamp = segmentTimestamp ?: return createCurrentSegment("onConfigurationChanged") { segment -> if (segment is ReplaySegment.Created) { segment.capture(hub) From c8a168998d44154682943af9ebe331d861995750 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 8 Aug 2024 15:17:38 +0200 Subject: [PATCH 14/21] Null-check bitmap before accessing it --- .../src/main/java/io/sentry/android/replay/util/Views.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt index e74fc695b5..a44508eac6 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt @@ -56,10 +56,8 @@ internal fun Drawable?.isRedactable(): Boolean { return when (this) { is InsetDrawable, is ColorDrawable, is VectorDrawable, is GradientDrawable -> false is BitmapDrawable -> { - if (bitmap == null) { - return false - } - return !bitmap.isRecycled && bitmap.height > 10 && bitmap.width > 10 + val bmp = bitmap ?: return false + return !bmp.isRecycled && bmp.height > 10 && bmp.width > 10 } else -> true } From 07524243d7a3960af6c279b79c85dcd937406fc7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 12 Aug 2024 09:58:37 +0200 Subject: [PATCH 15/21] Use software canvas for xiaomi devices --- .../android/replay/video/SimpleVideoEncoder.kt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index fd770131d8..2c47bdc5d4 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -34,6 +34,7 @@ import android.graphics.Bitmap import android.media.MediaCodec import android.media.MediaCodecInfo import android.media.MediaFormat +import android.os.Build import android.view.Surface import io.sentry.SentryLevel.DEBUG import io.sentry.SentryOptions @@ -139,10 +140,13 @@ internal class SimpleVideoEncoder( } fun encode(image: Bitmap) { - // NOTE do not use `lockCanvas` like what is done in bitmap2video - // This is because https://developer.android.com/reference/android/media/MediaCodec#createInputSurface() - // says that, "Surface.lockCanvas(android.graphics.Rect) may fail or produce unexpected results." - val canvas = surface?.lockHardwareCanvas() + // it seems that Xiaomi devices have problems with hardware canvas, so we have to use + // lockCanvas instead (Android 12 or above) + val canvas = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && Build.MANUFACTURER.contains("xiaomi", ignoreCase = true)) { + surface?.lockCanvas(null) + } else { + surface?.lockHardwareCanvas() + } canvas?.drawBitmap(image, 0f, 0f, null) surface?.unlockCanvasAndPost(canvas) drainCodec(false) From 3c0ae2f8e91f4ea6b085157ef7acdad7f01c180f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 12 Aug 2024 10:03:43 +0200 Subject: [PATCH 16/21] Remove Android version check for software canvas --- .../java/io/sentry/android/replay/video/SimpleVideoEncoder.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 2c47bdc5d4..8454f90f74 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -141,8 +141,8 @@ internal class SimpleVideoEncoder( fun encode(image: Bitmap) { // it seems that Xiaomi devices have problems with hardware canvas, so we have to use - // lockCanvas instead (Android 12 or above) - val canvas = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && Build.MANUFACTURER.contains("xiaomi", ignoreCase = true)) { + // lockCanvas instead + val canvas = if (Build.MANUFACTURER.contains("xiaomi", ignoreCase = true)) { surface?.lockCanvas(null) } else { surface?.lockHardwareCanvas() From 44745ca5ad78c69d54c9920d025d52b589cb12aa Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 12 Aug 2024 10:56:30 +0200 Subject: [PATCH 17/21] add link to comment --- .../java/io/sentry/android/replay/video/SimpleVideoEncoder.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 8454f90f74..475ce566d1 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -141,7 +141,7 @@ internal class SimpleVideoEncoder( fun encode(image: Bitmap) { // it seems that Xiaomi devices have problems with hardware canvas, so we have to use - // lockCanvas instead + // lockCanvas instead https://stackoverflow.com/a/73520742 val canvas = if (Build.MANUFACTURER.contains("xiaomi", ignoreCase = true)) { surface?.lockCanvas(null) } else { From 052927b2582c41c2136c5ff4fb06d44b1b165e70 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 12 Aug 2024 11:55:47 +0200 Subject: [PATCH 18/21] Fix FileNotFoundException when serializing segment values --- .../replay/capture/BaseCaptureStrategy.kt | 4 ++-- .../replay/capture/BufferCaptureStrategyTest.kt | 17 ++++++++++++++++- .../capture/SessionCaptureStrategyTest.kt | 17 ++++++++++++++++- 3 files changed, 34 insertions(+), 4 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 97cb386109..fcd2d11293 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 @@ -100,10 +100,10 @@ internal abstract class BaseCaptureStrategy( ) { cache = replayCacheProvider?.invoke(replayId, recorderConfig) ?: ReplayCache(options, replayId, recorderConfig) + this.currentReplayId = replayId + this.currentSegment = segmentId this.replayType = replayType ?: (if (this is SessionCaptureStrategy) SESSION else BUFFER) this.recorderConfig = recorderConfig - this.currentSegment = segmentId - this.currentReplayId = replayId segmentTimestamp = DateUtils.getCurrentDateTime() replayStartTimestamp.set(dateProvider.currentTimeMillis) 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 bbad02444d..1cd266ecb2 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 @@ -64,7 +64,7 @@ class BufferCaptureStrategyTest { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope(any()) } - var persistedSegment = mutableMapOf() + var persistedSegment = LinkedHashMap() val replayCache = mock { on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997))) on { persistSegmentValues(any(), anyOrNull()) }.then { @@ -293,4 +293,19 @@ class BufferCaptureStrategyTest { verify(fixture.hub).captureReplay(any(), any()) } + + @Test + fun `replayId should be set and serialized first`() { + val strategy = fixture.getSut() + val replayId = SentryId() + + strategy.start(fixture.recorderConfig, 0, replayId) + + assertEquals( + replayId.toString(), + fixture.persistedSegment.values.first(), + "The replayId must be set first, so when we clean up stale replays" + + "the current replay cache folder is not being deleted." + ) + } } 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 ac593f6c27..2ecc0e2baf 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 @@ -70,7 +70,7 @@ class SessionCaptureStrategyTest { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope(any()) } - var persistedSegment = mutableMapOf() + var persistedSegment = LinkedHashMap() val replayCache = mock { on { frames }.thenReturn(mutableListOf(ReplayFrame(File("1720693523997.jpg"), 1720693523997))) on { persistSegmentValues(any(), anyOrNull()) }.then { @@ -352,4 +352,19 @@ class SessionCaptureStrategyTest { } ) } + + @Test + fun `replayId should be set and serialized first`() { + val strategy = fixture.getSut() + val replayId = SentryId() + + strategy.start(fixture.recorderConfig, 0, replayId) + + assertEquals( + replayId.toString(), + fixture.persistedSegment.values.first(), + "The replayId must be set first, so when we clean up stale replays" + + "the current replay cache folder is not being deleted." + ) + } } From 85b51404c6f1e35a87149b199603c62cb4d25f12 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 12 Aug 2024 09:57:54 +0000 Subject: [PATCH 19/21] Format code --- .../sentry/android/replay/capture/SessionCaptureStrategyTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2ecc0e2baf..12eb10c3f4 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 @@ -364,7 +364,7 @@ class SessionCaptureStrategyTest { replayId.toString(), fixture.persistedSegment.values.first(), "The replayId must be set first, so when we clean up stale replays" + - "the current replay cache folder is not being deleted." + "the current replay cache folder is not being deleted." ) } } From 729ba615839acd4d1e545d088f3c24ae42ca8ad2 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 12 Aug 2024 17:14:25 +0200 Subject: [PATCH 20/21] Use default android codec when exynos is present --- .../android/replay/video/SimpleVideoEncoder.kt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 475ce566d1..baf521a2e6 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -33,6 +33,7 @@ import android.annotation.TargetApi import android.graphics.Bitmap import android.media.MediaCodec import android.media.MediaCodecInfo +import android.media.MediaCodecList import android.media.MediaFormat import android.os.Build import android.view.Surface @@ -51,8 +52,22 @@ internal class SimpleVideoEncoder( val onClose: (() -> Unit)? = null ) { + private val hasExynosCodec: Boolean by lazy(NONE) { + // MediaCodecList ctor will initialize an internal in-memory static cache of codecs, so this + // call is only expensive the first time + MediaCodecList(MediaCodecList.REGULAR_CODECS) + .codecInfos + .any { it.name.contains("c2.exynos") } + } + internal val mediaCodec: MediaCodec = run { - val codec = MediaCodec.createEncoderByType(muxerConfig.mimeType) + // c2.exynos.h264.encoder seems to have problems encoding the video (Pixel and Samsung devices) + // so we use the default encoder instead + val codec = if (hasExynosCodec) { + MediaCodec.createByCodecName("c2.android.avc.encoder") + } else { + MediaCodec.createEncoderByType(muxerConfig.mimeType) + } codec } From 10ead6fefcb90fb2806c947156538ae21770ea5f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 12 Aug 2024 18:04:10 +0200 Subject: [PATCH 21/21] Changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09ecbba6f4..3223682252 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ - Align next segment timestamp with the end of the buffered segment when converting from buffer mode to session mode - Persist `buffer` replay type for the entire replay when converting from buffer mode to session mode - Properly store screen names for `buffer` mode +- Session Replay: fix various crashes and issues ([#3628](https://github.com/getsentry/sentry-java/pull/3628)) + - Fix video not being encoded on Pixel devices + - Fix SIGABRT native crashes on Xiaomi devices when encoding a video + - Fix `RejectedExecutionException` when redacting a screenshot + - Fix `FileNotFoundException` when persisting segment values ### Chores