[SR] Buffer mode improvements

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
 

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 <init> (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 {

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(

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)
 
@@ -176,8 +168,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()
  }
@@ -212,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)
  }
  }
 

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<Date>(propertyName = SEGMENT_KEY_TIMESTAMP) { _, _, newValue ->
+ override var segmentTimestamp by persistableAtomicNullable<Date>(propertyName = SEGMENT_KEY_TIMESTAMP) { _, _, newValue ->
  cache?.persistSegmentValues(SEGMENT_KEY_TIMESTAMP, if (newValue == null) null else DateUtils.getTimestamp(newValue))
  }
  protected val replayStartTimestamp = AtomicLong()
@@ -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<ReplayType>(propertyName = SEGMENT_KEY_REPLAY_TYPE)
+ override var replayType by persistableAtomic<ReplayType>(propertyName = SEGMENT_KEY_REPLAY_TYPE)
  protected val currentEvents: LinkedList<RRWebEvent> = 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,

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt

@@ -20,6 +20,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(
@@ -34,41 +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<ReplaySegment.Created>()
 
- // TODO: rework this bs, it doesn't work with sending replay on restart
- private val bufferedScreensLock = Any()
- private val bufferedScreens = mutableListOf<Pair<String, Long>>()
-
  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
- ) {
- super.start(recorderConfig, segmentId, replayId)
-
- 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) {
@@ -90,7 +61,7 @@ internal class BufferCaptureStrategy(
 
  override fun captureReplay(
  isTerminating: Boolean,
- onSegmentSent: () -> Unit
+ onSegmentSent: (Date) -> Unit
  ) {
  val sampled = random.sample(options.experimental.sessionReplay.errorSampleRate)
 
@@ -121,8 +92,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)
  }
  }
  }
@@ -136,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)
  }
  }
@@ -159,7 +129,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
  }
 
@@ -169,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
@@ -246,11 +201,9 @@ 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, BUFFER)
+ createSegmentInternal(duration, currentSegmentTimestamp, replayId, segmentId, height, width)
  onSegmentCreated(segment)
  }
  }

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt

@@ -25,11 +25,14 @@ internal interface CaptureStrategy {
  var currentSegment: Int
  var currentReplayId: SentryId
  val replayCacheDir: File?
+ var replayType: ReplayType
+ var segmentTimestamp: Date?
 
  fun start(
  recorderConfig: ScreenshotRecorderConfig,
  segmentId: Int = 0,
- replayId: SentryId = SentryId()
+ replayId: SentryId = SentryId(),
+ replayType: ReplayType? = null
  )
 
  fun stop()
@@ -38,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)
 
@@ -194,7 +197,6 @@ internal interface CaptureStrategy {
 
  replay.urls = urls
  return ReplaySegment.Created(
- videoDuration = videoDuration,
  replay = replay,
  recording = recording
  )
@@ -219,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() {