Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Ensure no gaps in segment timestamps when converting strategies
  • Loading branch information
romtsn committed Aug 5, 2024
commit dd8f9f38b2b6d0c4c02da6653f443330dea605b3
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal interface CaptureStrategy {
var currentReplayId: SentryId
val replayCacheDir: File?
var replayType: ReplayType
var segmentTimestamp: Date?

fun start(
recorderConfig: ScreenshotRecorderConfig,
Expand All @@ -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)

Expand Down Expand Up @@ -196,7 +197,6 @@ internal interface CaptureStrategy {

replay.urls = urls
return ReplaySegment.Created(
videoDuration = videoDuration,
replay = replay,
recording = recording
)
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
}