diff --git a/CHANGELOG.md b/CHANGELOG.md index a6600fda92..393f4f8da3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ ### Fixes +- Session Replay: Fix first recording segment missing for replays in `buffer` mode ([#5753](https://github.com/getsentry/sentry-java/pull/5753)) - Prevent logs and metrics from remaining queued after a flush scheduling race ([#5756](https://github.com/getsentry/sentry-java/pull/5756)) - Fix main thread identification for tombstone (native crash) events ([#5742](https://github.com/getsentry/sentry-java/pull/5742)) 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 b3b9edae05..b54177bca9 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 @@ -317,6 +317,7 @@ public class ReplayCache(private val options: SentryOptions, private val replayI internal const val SEGMENT_KEY_REPLAY_SCREEN_AT_START = "replay.screen-at-start" internal const val SEGMENT_KEY_REPLAY_RECORDING = "replay.recording" internal const val SEGMENT_KEY_ID = "segment.id" + internal const val SEGMENT_KEY_FLUSHED = "replay.flushed" fun makeReplayCacheDir(options: SentryOptions, replayId: SentryId): File? = if (options.cacheDirPath.isNullOrEmpty()) { @@ -415,8 +416,11 @@ public class ReplayCache(private val options: SentryOptions, private val replayI } cache.frames.sortBy { it.timestamp } - // TODO: this should be removed when we start sending buffered segments on next launch - val normalizedSegmentId = if (replayType == SESSION) segmentId else 0 + val wasFlushed = lastSegment[SEGMENT_KEY_FLUSHED]?.toBooleanStrictOrNull() == true + // In buffer mode, if the buffer was never flushed (no error triggered captureReplay), + // no segments were ever sent, so we normalize to 0. After a flush + conversion to + // session mode, the persisted segmentId is the real sequence number. + val normalizedSegmentId = if (replayType == SESSION || wasFlushed) segmentId else 0 val normalizedTimestamp = if (replayType == SESSION) { segmentTimestamp 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 612517438f..bae0e41179 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 @@ -261,6 +261,7 @@ public class ReplayIntegration( onSegmentSent = { newTimestamp -> captureStrategy?.currentSegment = captureStrategy?.currentSegment!! + 1 captureStrategy?.segmentTimestamp = newTimestamp + captureStrategy?.isFlushed = true }, ) 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 6bb58c5e2a..fbc0ccfd4b 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 @@ -13,6 +13,7 @@ import io.sentry.SentryReplayEvent.ReplayType.BUFFER import io.sentry.SentryReplayEvent.ReplayType.SESSION import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_BIT_RATE +import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FLUSHED import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FRAME_RATE import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_HEIGHT import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_ID @@ -89,6 +90,11 @@ internal abstract class BaseCaptureStrategy( get() = cache?.replayCacheDir override var replayType by persistableAtomic(propertyName = SEGMENT_KEY_REPLAY_TYPE) + // Tracks whether the buffer was flushed (segments sent to server). Used by fromDisk() + // to decide whether to normalize the segment ID to 0 on crash recovery: if never flushed, + // no segments reached the server, so the recovered segment must be 0. + override var isFlushed: Boolean by + persistableAtomic(initialValue = false, propertyName = SEGMENT_KEY_FLUSHED) protected val currentEvents: Deque = ConcurrentLinkedDeque() private val traceIdsLock = Any() 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 0df8a642f6..f6c6f3997a 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 @@ -105,7 +105,6 @@ internal class BufferCaptureStrategy( if (segment is ReplaySegment.Created) { segment.capture(scopes) - // 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 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 6dc391a15e..096a93741b 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 @@ -29,6 +29,7 @@ internal interface CaptureStrategy { val replayCacheDir: File? var replayType: ReplayType var segmentTimestamp: Date? + var isFlushed: Boolean fun start(segmentId: Int = 0, replayId: SentryId = SentryId(), replayType: ReplayType? = null) 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 257941a911..8b64ca5cae 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 @@ -9,6 +9,7 @@ import io.sentry.SentryOptions import io.sentry.SentryReplayEvent.ReplayType import io.sentry.android.replay.ReplayCache.Companion.ONGOING_SEGMENT import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_BIT_RATE +import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FLUSHED import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FRAME_RATE import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_HEIGHT import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_ID @@ -443,7 +444,7 @@ class ReplayCacheTest { } @Test - fun `sets segmentId to 0 for buffer mode`() { + fun `sets segmentId to 0 for buffer mode when not flushed`() { fixture.options.run { cacheDirPath = tmpDir.newFolder()?.absolutePath } val replayId = SentryId() val replayCacheFolder = @@ -474,6 +475,39 @@ class ReplayCacheTest { assertEquals(0, lastSegment.id) } + @Test + fun `preserves segmentId for buffer mode when already flushed`() { + fixture.options.run { cacheDirPath = tmpDir.newFolder()?.absolutePath } + val replayId = SentryId() + val replayCacheFolder = + File(fixture.options.cacheDirPath!!, "replay_$replayId").also { it.mkdirs() } + File(replayCacheFolder, ONGOING_SEGMENT).also { + it.writeText( + """ + $SEGMENT_KEY_HEIGHT=912 + $SEGMENT_KEY_WIDTH=416 + $SEGMENT_KEY_FRAME_RATE=1 + $SEGMENT_KEY_BIT_RATE=75000 + $SEGMENT_KEY_ID=5 + $SEGMENT_KEY_TIMESTAMP=2024-07-11T10:25:21.454Z + $SEGMENT_KEY_REPLAY_TYPE=BUFFER + $SEGMENT_KEY_FLUSHED=true + """ + .trimIndent() + ) + } + + val screenshot = File(replayCacheFolder, "1720693523997.jpg").also { it.createNewFile() } + screenshot.outputStream().use { + Bitmap.createBitmap(1, 1, ARGB_8888).compress(JPEG, 80, it) + it.flush() + } + + val lastSegment = ReplayCache.fromDisk(fixture.options, replayId)!! + + assertEquals(5, lastSegment.id) + } + @Test fun `when screenshot is corrupted, deletes it immediately`() { ShadowBitmapFactory.setAllowInvalidImageData(false)