Skip to content

Commit faef0d5

Browse files
runningcodeclaude
andcommitted
fix(replay): Measure recording deadlines on a monotonic clock (JAVA-575)
Session Replay derives every interval from the wall clock. Two of those intervals are pure control flow, and a clock step corrupts both: - The 1h `sessionDuration` cap was `frameTimestamp - replayStartTimestamp`. A forward step past the cap kills a healthy recording on the next frame; a backward step lets one run indefinitely. - The 50ms ACTION_MOVE debounce was `lastCapturedMoveEvent + 50 > now`. A backward step suppresses every move event until the wall clock catches back up. Both now use `io.sentry.time.Deadline` on the options' `MonotonicTicker`, which is `SystemClock.elapsedRealtimeNanos()` on Android. That clock advances across device suspend just as the wall clock does, so the conversion preserves today's semantics and only removes the step. The cap is still evaluated when a frame is captured rather than when its queued task runs, so a busy replay executor cannot trip it. Everything else JAVA-575 lists — segment timestamps and durations, frame filenames, buffer cutoffs, gesture `timeOffset`s — stays on the wall clock: those values are serialized, and fixing them means re-basing them onto an `AnchoredClock`, which is a v9 change. `// TODO [MAJOR]` markers record that at both `ICurrentDateProvider` injection sites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 744ff64 commit faef0d5

9 files changed

Lines changed: 141 additions & 21 deletions

File tree

‎sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ public class ReplayIntegration(
223223
options,
224224
scopes,
225225
dateProvider,
226+
options.monotonicTicker,
226227
replayExecutor,
227228
persistingExecutor,
228229
replayCacheProvider,
@@ -232,6 +233,7 @@ public class ReplayIntegration(
232233
options,
233234
scopes,
234235
dateProvider,
236+
options.monotonicTicker,
235237
replayExecutor,
236238
persistingExecutor,
237239
replayCacheProvider,

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,16 @@ import io.sentry.android.replay.gestures.ReplayGestureConverter
2929
import io.sentry.android.replay.util.ReplayRunnable
3030
import io.sentry.protocol.SentryId
3131
import io.sentry.rrweb.RRWebEvent
32+
import io.sentry.time.Deadline
33+
import io.sentry.time.MonotonicTicker
3234
import io.sentry.transport.ICurrentDateProvider
3335
import java.io.File
3436
import java.util.Date
3537
import java.util.Deque
3638
import java.util.concurrent.ConcurrentLinkedDeque
3739
import java.util.concurrent.ScheduledExecutorService
40+
import java.util.concurrent.TimeUnit.MILLISECONDS
3841
import java.util.concurrent.atomic.AtomicBoolean
39-
import java.util.concurrent.atomic.AtomicLong
4042
import java.util.concurrent.atomic.AtomicReference
4143
import kotlin.properties.ReadWriteProperty
4244
import kotlin.reflect.KProperty
@@ -46,7 +48,12 @@ import kotlin.reflect.KProperty
4648
internal abstract class BaseCaptureStrategy(
4749
private val options: SentryOptions,
4850
private val scopes: IScopes?,
51+
// TODO [MAJOR]: every value this provider stamps is serialized — segment timestamps and
52+
// durations, frame filenames, buffer cutoffs, gesture offsets — and they are all subtracted from
53+
// each other, so a clock step corrupts a recording. Replacing it with an io.sentry.time
54+
// .AnchoredClock re-bases those timestamps and is therefore v9-only. See JAVA-575.
4955
private val dateProvider: ICurrentDateProvider,
56+
private val ticker: MonotonicTicker,
5057
protected val replayExecutor: ScheduledExecutorService,
5158
protected val persistingExecutor: ScheduledExecutorService,
5259
private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null,
@@ -57,7 +64,7 @@ internal abstract class BaseCaptureStrategy(
5764
private const val MAX_CONTEXT_VALUES = 100
5865
}
5966

60-
private val gestureConverter = ReplayGestureConverter(dateProvider)
67+
private val gestureConverter = ReplayGestureConverter(dateProvider, ticker)
6168

6269
protected val isTerminating = AtomicBoolean(false)
6370
protected var cache: ReplayCache? = null
@@ -79,7 +86,11 @@ internal abstract class BaseCaptureStrategy(
7986
if (newValue == null) null else DateUtils.getTimestamp(newValue),
8087
)
8188
}
82-
protected val replayStartTimestamp = AtomicLong()
89+
/**
90+
* When the recording reaches its `sessionDuration` cap, or `null` while nothing is being
91+
* recorded. Only [SessionCaptureStrategy] enforces a cap; buffer mode records indefinitely.
92+
*/
93+
@Volatile protected var replayDeadline: Deadline? = null
8394
protected var screenAtStart by
8495
persistableAtomicNullable<String>(propertyName = SEGMENT_KEY_REPLAY_SCREEN_AT_START)
8596
override var currentReplayId: SentryId by
@@ -109,7 +120,7 @@ internal abstract class BaseCaptureStrategy(
109120
this.replayType = replayType ?: (if (this is SessionCaptureStrategy) SESSION else BUFFER)
110121

111122
segmentTimestamp = DateUtils.getCurrentDateTime()
112-
replayStartTimestamp.set(dateProvider.currentTimeMillis)
123+
replayDeadline = Deadline.after(ticker, options.sessionReplay.sessionDuration, MILLISECONDS)
113124
}
114125

115126
override fun resume() {
@@ -125,7 +136,7 @@ internal abstract class BaseCaptureStrategy(
125136
replayExecutor.submit(
126137
ReplayRunnable("$TAG.stop") {
127138
cache?.close()
128-
replayStartTimestamp.set(0)
139+
replayDeadline = null
129140
segmentTimestamp = null
130141
currentReplayId = SentryId.EMPTY_ID
131142
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
2020
import io.sentry.android.replay.util.ReplayRunnable
2121
import io.sentry.clientreport.DiscardReason.RATELIMIT_BACKOFF
2222
import io.sentry.protocol.SentryId
23+
import io.sentry.time.MonotonicTicker
2324
import io.sentry.transport.ICurrentDateProvider
2425
import io.sentry.util.FileUtils
2526
import java.io.File
@@ -44,6 +45,7 @@ internal class BufferCaptureStrategy(
4445
private val options: SentryOptions,
4546
private val scopes: IScopes?,
4647
private val dateProvider: ICurrentDateProvider,
48+
private val ticker: MonotonicTicker,
4749
executor: ScheduledExecutorService,
4850
persistingExecutor: ScheduledExecutorService,
4951
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null,
@@ -52,6 +54,7 @@ internal class BufferCaptureStrategy(
5254
options,
5355
scopes,
5456
dateProvider,
57+
ticker,
5558
executor,
5659
persistingExecutor,
5760
replayCacheProvider = replayCacheProvider,
@@ -173,7 +176,14 @@ internal class BufferCaptureStrategy(
173176
// we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of
174177
// execution
175178
val captureStrategy =
176-
SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, persistingExecutor)
179+
SessionCaptureStrategy(
180+
options,
181+
scopes,
182+
dateProvider,
183+
ticker,
184+
replayExecutor,
185+
persistingExecutor,
186+
)
177187
captureStrategy.recorderConfig = recorderConfig
178188
captureStrategy.start(
179189
segmentId = currentSegment,

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import io.sentry.android.replay.ScreenshotRecorderConfig
1111
import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
1212
import io.sentry.android.replay.util.ReplayRunnable
1313
import io.sentry.protocol.SentryId
14+
import io.sentry.time.MonotonicTicker
1415
import io.sentry.transport.ICurrentDateProvider
1516
import io.sentry.util.FileUtils
1617
import java.util.Date
@@ -32,6 +33,7 @@ internal class SessionCaptureStrategy(
3233
private val options: SentryOptions,
3334
private val scopes: IScopes?,
3435
private val dateProvider: ICurrentDateProvider,
36+
ticker: MonotonicTicker,
3537
executor: ScheduledExecutorService,
3638
persistingExecutor: ScheduledExecutorService,
3739
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null,
@@ -40,6 +42,7 @@ internal class SessionCaptureStrategy(
4042
options,
4143
scopes,
4244
dateProvider,
45+
ticker,
4346
executor,
4447
persistingExecutor,
4548
replayCacheProvider,
@@ -111,6 +114,7 @@ internal class SessionCaptureStrategy(
111114
// reflecting the exact time of when it was captured
112115
val currentConfig = recorderConfig
113116
val frameTimestamp = dateProvider.currentTimeMillis
117+
val deadlineExceeded = replayDeadline?.hasPassed() == true
114118
replayExecutor.submit(
115119
ReplayRunnable("$TAG.add_frame") {
116120
cache?.store(frameTimestamp)
@@ -158,7 +162,7 @@ internal class SessionCaptureStrategy(
158162
}
159163
}
160164

161-
if (frameTimestamp - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration) {
165+
if (deadlineExceeded) {
162166
options.replayController.stop()
163167
options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording")
164168
}

‎sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,28 @@ import io.sentry.rrweb.RRWebInteractionEvent
77
import io.sentry.rrweb.RRWebInteractionEvent.InteractionType
88
import io.sentry.rrweb.RRWebInteractionMoveEvent
99
import io.sentry.rrweb.RRWebInteractionMoveEvent.Position
10+
import io.sentry.time.Deadline
11+
import io.sentry.time.MonotonicTicker
1012
import io.sentry.transport.ICurrentDateProvider
13+
import java.util.concurrent.TimeUnit.MILLISECONDS
1114

12-
internal class ReplayGestureConverter(private val dateProvider: ICurrentDateProvider) {
15+
internal class ReplayGestureConverter(
16+
// TODO [MAJOR]: the timestamps and timeOffsets stamped from this provider are serialized, so
17+
// moving them onto an io.sentry.time.AnchoredClock is v9-only. See JAVA-575.
18+
private val dateProvider: ICurrentDateProvider,
19+
private val ticker: MonotonicTicker,
20+
) {
1321
internal companion object {
1422
// rrweb values
15-
private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50
23+
private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50L
1624
private const val CAPTURE_MOVE_EVENT_THRESHOLD = 500
1725
}
1826

1927
private val currentPositions = LinkedHashMap<Int, ArrayList<Position>>(10)
2028
private var touchMoveBaseline = 0L
21-
private var lastCapturedMoveEvent = 0L
29+
30+
/** When the next move event may be captured, or `null` before the first one. */
31+
private var moveDebounce: Deadline? = null
2232

2333
fun convert(
2434
event: MotionEvent,
@@ -28,12 +38,10 @@ internal class ReplayGestureConverter(private val dateProvider: ICurrentDateProv
2838
MotionEvent.ACTION_MOVE -> {
2939
// we only throttle move events as those can be overwhelming
3040
val now = dateProvider.currentTimeMillis
31-
if (
32-
lastCapturedMoveEvent != 0L && lastCapturedMoveEvent + TOUCH_MOVE_DEBOUNCE_THRESHOLD > now
33-
) {
41+
if (moveDebounce?.hasPassed() == false) {
3442
return null
3543
}
36-
lastCapturedMoveEvent = now
44+
moveDebounce = Deadline.after(ticker, TOUCH_MOVE_DEBOUNCE_THRESHOLD, MILLISECONDS)
3745

3846
currentPositions.keys.forEach { pId ->
3947
val pIndex = event.findPointerIndex(pId)

‎sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,6 +1254,7 @@ class ReplayIntegrationTest {
12541254
ICurrentDateProvider {
12551255
System.currentTimeMillis() + fixture.options.sessionReplay.sessionSegmentDuration
12561256
},
1257+
fixture.options.monotonicTicker,
12571258
// run tasks synchronously in tests
12581259
mock {
12591260
whenever(mock.submit(any<Runnable>())).doAnswer {
@@ -1638,6 +1639,7 @@ class ReplayIntegrationTest {
16381639
options,
16391640
null,
16401641
CurrentDateProvider.getInstance(),
1642+
options.monotonicTicker,
16411643
executor =
16421644
mock {
16431645
whenever(mock.submit(any<Runnable>())).doAnswer {

‎sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import io.sentry.android.replay.capture.BufferCaptureStrategyTest.Fixture.Compan
2323
import io.sentry.clientreport.DiscardReason
2424
import io.sentry.clientreport.DiscardedEvent
2525
import io.sentry.protocol.SentryId
26+
import io.sentry.time.TestMonotonicTicker
2627
import io.sentry.transport.CurrentDateProvider
2728
import io.sentry.transport.ICurrentDateProvider
2829
import io.sentry.transport.RateLimiter
@@ -108,6 +109,8 @@ class BufferCaptureStrategyTest {
108109
?.discardedEvents
109110
.orEmpty()
110111

112+
val ticker = TestMonotonicTicker()
113+
111114
fun getSut(
112115
dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(),
113116
replayCacheDir: File? = null,
@@ -117,6 +120,7 @@ class BufferCaptureStrategyTest {
117120
options,
118121
scopes,
119122
dateProvider,
123+
ticker,
120124
mock {
121125
whenever(it.submit(any<Runnable>())).doAnswer { invocation ->
122126
(invocation.arguments[0] as Runnable).run()

‎sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,14 @@ import io.sentry.protocol.SentryId
3030
import io.sentry.rrweb.RRWebBreadcrumbEvent
3131
import io.sentry.rrweb.RRWebMetaEvent
3232
import io.sentry.rrweb.RRWebOptionsEvent
33+
import io.sentry.time.TestMonotonicTicker
3334
import io.sentry.transport.CurrentDateProvider
3435
import io.sentry.transport.ICurrentDateProvider
3536
import java.io.File
3637
import java.util.Date
3738
import java.util.concurrent.ScheduledExecutorService
39+
import java.util.concurrent.TimeUnit.MILLISECONDS
40+
import java.util.concurrent.TimeUnit.SECONDS
3841
import kotlin.test.Test
3942
import kotlin.test.assertContentEquals
4043
import kotlin.test.assertEquals
@@ -108,6 +111,8 @@ class SessionCaptureStrategyTest {
108111
bitRate = 20_000,
109112
)
110113

114+
val ticker = TestMonotonicTicker()
115+
111116
fun getSut(
112117
dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(),
113118
replayCacheDir: File? = null,
@@ -125,6 +130,7 @@ class SessionCaptureStrategyTest {
125130
options,
126131
scopes,
127132
dateProvider,
133+
ticker,
128134
replayExecutor,
129135
mock {
130136
doAnswer { invocation ->
@@ -447,6 +453,37 @@ class SessionCaptureStrategyTest {
447453
strategy.start()
448454
strategy.onConfigurationChanged(mock<ScreenshotRecorderConfig>())
449455
now += fixture.options.sessionReplay.sessionDuration * 2
456+
fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration * 2, MILLISECONDS)
457+
458+
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
459+
460+
verify(fixture.options.replayController).stop()
461+
}
462+
463+
@Test
464+
fun `onScreenshotRecorded does not stop replay when the wall clock jumps past the deadline`() {
465+
var now = System.currentTimeMillis()
466+
val strategy = fixture.getSut(dateProvider = { now })
467+
strategy.start()
468+
strategy.onConfigurationChanged(mock<ScreenshotRecorderConfig>())
469+
// the device corrects its clock two hours forwards a second into the recording
470+
now += fixture.options.sessionReplay.sessionDuration * 2
471+
fixture.ticker.advance(1, SECONDS)
472+
473+
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
474+
475+
verify(fixture.options.replayController, never()).stop()
476+
}
477+
478+
@Test
479+
fun `onScreenshotRecorded stops replay when the wall clock steps backwards past the deadline`() {
480+
var now = System.currentTimeMillis()
481+
val strategy = fixture.getSut(dateProvider = { now })
482+
strategy.start()
483+
strategy.onConfigurationChanged(mock<ScreenshotRecorderConfig>())
484+
// an hour of recording, during which the device drags its clock back to before the start
485+
fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration, MILLISECONDS)
486+
now -= 1000
450487

451488
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
452489

0 commit comments

Comments
 (0)