Skip to content

Commit 3cf91ba

Browse files
runningcodeclaude
andauthored
fix(replay): Measure recording deadlines on a monotonic clock (JAVA-575) (#6090)
* 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> * changelog * ref(replay): Trim the v9 AnchoredClock TODOs Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(replay): Advance the ticker during the executor delay The executor-delay test only moved the wall clock while the frame task was queued, so it no longer guarded the 1h cap once that cap moved onto the monotonic ticker: checking the deadline inside the task would still pass. Advancing the ticker as well makes it fail on that regression. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * ref(replay): Suppress detekt size checks on the capture strategies The monotonic ticker brings the capture-strategy constructors to seven parameters now that main injects a persisting executor, and the executor-delay ticker advance tips SessionCaptureStrategyTest over the LargeClass threshold. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent dcd2037 commit 3cf91ba

10 files changed

Lines changed: 155 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Fixes
6+
7+
- Measure Session Replay's 1h recording cap and its touch-move debounce on a monotonic clock, so a device time change no longer stops a healthy recording early, keeps a finished one alive, or suppresses gesture events ([#6090](https://github.com/getsentry/sentry-java/pull/6090))
8+
39
## 8.61.0
410

511
### Fixes

‎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
@@ -228,6 +228,7 @@ public class ReplayIntegration(
228228
options,
229229
scopes,
230230
dateProvider,
231+
options.monotonicTicker,
231232
replayExecutor,
232233
persistingExecutor,
233234
replayCacheProvider,
@@ -237,6 +238,7 @@ public class ReplayIntegration(
237238
options,
238239
scopes,
239240
dateProvider,
241+
options.monotonicTicker,
240242
replayExecutor,
241243
persistingExecutor,
242244
replayCacheProvider,

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

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,24 +35,30 @@ import io.sentry.android.replay.gestures.ReplayGestureConverter
3535
import io.sentry.android.replay.util.ReplayRunnable
3636
import io.sentry.protocol.SentryId
3737
import io.sentry.rrweb.RRWebEvent
38+
import io.sentry.time.Deadline
39+
import io.sentry.time.MonotonicTicker
3840
import io.sentry.transport.ICurrentDateProvider
3941
import java.io.File
4042
import java.util.Date
4143
import java.util.Deque
4244
import java.util.concurrent.ConcurrentLinkedDeque
4345
import java.util.concurrent.ScheduledExecutorService
46+
import java.util.concurrent.TimeUnit.MILLISECONDS
4447
import java.util.concurrent.atomic.AtomicBoolean
45-
import java.util.concurrent.atomic.AtomicLong
4648
import java.util.concurrent.atomic.AtomicReference
4749
import kotlin.properties.ReadWriteProperty
4850
import kotlin.reflect.KProperty
4951

5052
@SuppressLint("UseRequiresApi")
5153
@TargetApi(26)
52-
internal abstract class BaseCaptureStrategy(
54+
internal abstract class BaseCaptureStrategy
55+
@Suppress("LongParameterList")
56+
constructor(
5357
private val options: SentryOptions,
5458
private val scopes: IScopes?,
55-
private val dateProvider: ICurrentDateProvider,
59+
// TODO [v9]: We should consider replacing this with AnchoredClock in V9.
60+
dateProvider: ICurrentDateProvider,
61+
private val ticker: MonotonicTicker,
5662
protected val replayExecutor: ScheduledExecutorService,
5763
protected val persistingExecutor: ScheduledExecutorService,
5864
private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null,
@@ -63,7 +69,7 @@ internal abstract class BaseCaptureStrategy(
6369
private const val MAX_CONTEXT_VALUES = 100
6470
}
6571

66-
private val gestureConverter = ReplayGestureConverter(dateProvider)
72+
private val gestureConverter = ReplayGestureConverter(dateProvider, ticker)
6773

6874
protected val isTerminating = AtomicBoolean(false)
6975
protected var cache: ReplayCache? = null
@@ -85,7 +91,11 @@ internal abstract class BaseCaptureStrategy(
8591
if (newValue == null) null else DateUtils.getTimestamp(newValue),
8692
)
8793
}
88-
protected val replayStartTimestamp = AtomicLong()
94+
/**
95+
* When the recording reaches its `sessionDuration` cap, or `null` while nothing is being
96+
* recorded. Only [SessionCaptureStrategy] enforces a cap; buffer mode records indefinitely.
97+
*/
98+
@Volatile protected var replayDeadline: Deadline? = null
8999
private var recordingMetadata: RecordingMetadata? by
90100
persistableAtomicNullable(propertyName = "") { _, _, newValue ->
91101
persistMetadataValue(SEGMENT_KEY_RELEASE, newValue?.release)
@@ -123,7 +133,7 @@ internal abstract class BaseCaptureStrategy(
123133
this.replayType = replayType ?: (if (this is SessionCaptureStrategy) SESSION else BUFFER)
124134

125135
segmentTimestamp = DateUtils.getCurrentDateTime()
126-
replayStartTimestamp.set(dateProvider.currentTimeMillis)
136+
replayDeadline = Deadline.after(ticker, options.sessionReplay.sessionDuration, MILLISECONDS)
127137
// the same values MainEventProcessor applies to current-run segments
128138
recordingMetadata =
129139
RecordingMetadata(
@@ -152,7 +162,7 @@ internal abstract class BaseCaptureStrategy(
152162
replayExecutor.submit(
153163
ReplayRunnable("$TAG.stop") {
154164
cache?.close()
155-
replayStartTimestamp.set(0)
165+
replayDeadline = null
156166
segmentTimestamp = null
157167
currentReplayId = SentryId.EMPTY_ID
158168
}

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

Lines changed: 14 additions & 2 deletions
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 java.io.File
2526
import java.util.Date
@@ -39,10 +40,13 @@ import java.util.concurrent.ScheduledExecutorService
3940
*/
4041
@SuppressLint("UseRequiresApi")
4142
@TargetApi(26)
42-
internal class BufferCaptureStrategy(
43+
internal class BufferCaptureStrategy
44+
@Suppress("LongParameterList")
45+
constructor(
4346
private val options: SentryOptions,
4447
private val scopes: IScopes?,
4548
private val dateProvider: ICurrentDateProvider,
49+
private val ticker: MonotonicTicker,
4650
executor: ScheduledExecutorService,
4751
persistingExecutor: ScheduledExecutorService,
4852
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null,
@@ -51,6 +55,7 @@ internal class BufferCaptureStrategy(
5155
options,
5256
scopes,
5357
dateProvider,
58+
ticker,
5459
executor,
5560
persistingExecutor,
5661
replayCacheProvider = replayCacheProvider,
@@ -172,7 +177,14 @@ internal class BufferCaptureStrategy(
172177
// we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of
173178
// execution
174179
val captureStrategy =
175-
SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, persistingExecutor)
180+
SessionCaptureStrategy(
181+
options,
182+
scopes,
183+
dateProvider,
184+
ticker,
185+
replayExecutor,
186+
persistingExecutor,
187+
)
176188
captureStrategy.recorderConfig = recorderConfig
177189
captureStrategy.start(
178190
segmentId = currentSegment,

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

Lines changed: 8 additions & 2 deletions
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 java.util.Date
1617
import java.util.concurrent.ScheduledExecutorService
@@ -27,10 +28,13 @@ import java.util.concurrent.ScheduledExecutorService
2728
*
2829
* See [BufferCaptureStrategy] for the on-error counterpart.
2930
*/
30-
internal class SessionCaptureStrategy(
31+
internal class SessionCaptureStrategy
32+
@Suppress("LongParameterList")
33+
constructor(
3134
private val options: SentryOptions,
3235
private val scopes: IScopes?,
3336
private val dateProvider: ICurrentDateProvider,
37+
ticker: MonotonicTicker,
3438
executor: ScheduledExecutorService,
3539
persistingExecutor: ScheduledExecutorService,
3640
replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null,
@@ -39,6 +43,7 @@ internal class SessionCaptureStrategy(
3943
options,
4044
scopes,
4145
dateProvider,
46+
ticker,
4247
executor,
4348
persistingExecutor,
4449
replayCacheProvider,
@@ -110,6 +115,7 @@ internal class SessionCaptureStrategy(
110115
// reflecting the exact time of when it was captured
111116
val currentConfig = recorderConfig
112117
val frameTimestamp = dateProvider.currentTimeMillis
118+
val deadlineExceeded = replayDeadline?.hasPassed() == true
113119
replayExecutor.submit(
114120
ReplayRunnable("$TAG.add_frame") {
115121
cache?.store(frameTimestamp)
@@ -157,7 +163,7 @@ internal class SessionCaptureStrategy(
157163
}
158164
}
159165

160-
if (frameTimestamp - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration) {
166+
if (deadlineExceeded) {
161167
options.replayController.stop()
162168
options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording")
163169
}

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,27 @@ 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 [v9]: Replace this with AnchoredClock in V9
17+
private val dateProvider: ICurrentDateProvider,
18+
private val ticker: MonotonicTicker,
19+
) {
1320
internal companion object {
1421
// rrweb values
15-
private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50
22+
private const val TOUCH_MOVE_DEBOUNCE_THRESHOLD = 50L
1623
private const val CAPTURE_MOVE_EVENT_THRESHOLD = 500
1724
}
1825

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

2332
fun convert(
2433
event: MotionEvent,
@@ -28,12 +37,10 @@ internal class ReplayGestureConverter(private val dateProvider: ICurrentDateProv
2837
MotionEvent.ACTION_MOVE -> {
2938
// we only throttle move events as those can be overwhelming
3039
val now = dateProvider.currentTimeMillis
31-
if (
32-
lastCapturedMoveEvent != 0L && lastCapturedMoveEvent + TOUCH_MOVE_DEBOUNCE_THRESHOLD > now
33-
) {
40+
if (moveDebounce?.hasPassed() == false) {
3441
return null
3542
}
36-
lastCapturedMoveEvent = now
43+
moveDebounce = Deadline.after(ticker, TOUCH_MOVE_DEBOUNCE_THRESHOLD, MILLISECONDS)
3744

3845
currentPositions.keys.forEach { pId ->
3946
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
@@ -1377,6 +1377,7 @@ class ReplayIntegrationTest {
13771377
ICurrentDateProvider {
13781378
System.currentTimeMillis() + fixture.options.sessionReplay.sessionSegmentDuration
13791379
},
1380+
fixture.options.monotonicTicker,
13801381
// run tasks synchronously in tests
13811382
mock {
13821383
whenever(mock.submit(any<Runnable>())).doAnswer {
@@ -1761,6 +1762,7 @@ class ReplayIntegrationTest {
17611762
options,
17621763
null,
17631764
CurrentDateProvider.getInstance(),
1765+
options.monotonicTicker,
17641766
executor =
17651767
mock {
17661768
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: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,14 @@ import io.sentry.protocol.SentryId
3636
import io.sentry.rrweb.RRWebBreadcrumbEvent
3737
import io.sentry.rrweb.RRWebMetaEvent
3838
import io.sentry.rrweb.RRWebOptionsEvent
39+
import io.sentry.time.TestMonotonicTicker
3940
import io.sentry.transport.CurrentDateProvider
4041
import io.sentry.transport.ICurrentDateProvider
4142
import java.io.File
4243
import java.util.Date
4344
import java.util.concurrent.ScheduledExecutorService
45+
import java.util.concurrent.TimeUnit.MILLISECONDS
46+
import java.util.concurrent.TimeUnit.SECONDS
4447
import kotlin.test.Test
4548
import kotlin.test.assertContentEquals
4649
import kotlin.test.assertEquals
@@ -62,6 +65,7 @@ import org.mockito.kotlin.never
6265
import org.mockito.kotlin.verify
6366
import org.mockito.kotlin.whenever
6467

68+
@Suppress("LargeClass")
6569
class SessionCaptureStrategyTest {
6670
@get:Rule val tmpDir = TemporaryFolder()
6771

@@ -114,6 +118,8 @@ class SessionCaptureStrategyTest {
114118
bitRate = 20_000,
115119
)
116120

121+
val ticker = TestMonotonicTicker()
122+
117123
fun getSut(
118124
dateProvider: ICurrentDateProvider = CurrentDateProvider.getInstance(),
119125
replayCacheDir: File? = null,
@@ -131,6 +137,7 @@ class SessionCaptureStrategyTest {
131137
options,
132138
scopes,
133139
dateProvider,
140+
ticker,
134141
replayExecutor,
135142
mock {
136143
doAnswer { invocation ->
@@ -441,6 +448,7 @@ class SessionCaptureStrategyTest {
441448

442449
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
443450
now += fixture.options.sessionReplay.sessionDuration
451+
fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration, MILLISECONDS)
444452
tasks.forEach(Runnable::run)
445453

446454
verify(fixture.scopes, never()).captureReplay(any(), any())
@@ -498,6 +506,37 @@ class SessionCaptureStrategyTest {
498506
strategy.start()
499507
strategy.onConfigurationChanged(mock<ScreenshotRecorderConfig>())
500508
now += fixture.options.sessionReplay.sessionDuration * 2
509+
fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration * 2, MILLISECONDS)
510+
511+
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
512+
513+
verify(fixture.options.replayController).stop()
514+
}
515+
516+
@Test
517+
fun `onScreenshotRecorded does not stop replay when the wall clock jumps past the deadline`() {
518+
var now = System.currentTimeMillis()
519+
val strategy = fixture.getSut(dateProvider = { now })
520+
strategy.start()
521+
strategy.onConfigurationChanged(mock<ScreenshotRecorderConfig>())
522+
// the device corrects its clock two hours forwards a second into the recording
523+
now += fixture.options.sessionReplay.sessionDuration * 2
524+
fixture.ticker.advance(1, SECONDS)
525+
526+
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
527+
528+
verify(fixture.options.replayController, never()).stop()
529+
}
530+
531+
@Test
532+
fun `onScreenshotRecorded stops replay when the wall clock steps backwards past the deadline`() {
533+
var now = System.currentTimeMillis()
534+
val strategy = fixture.getSut(dateProvider = { now })
535+
strategy.start()
536+
strategy.onConfigurationChanged(mock<ScreenshotRecorderConfig>())
537+
// an hour of recording, during which the device drags its clock back to before the start
538+
fixture.ticker.advance(fixture.options.sessionReplay.sessionDuration, MILLISECONDS)
539+
now -= 1000
501540

502541
strategy.onScreenshotRecorded(mock<Bitmap>()) {}
503542

0 commit comments

Comments
 (0)