fix(replay): Measure recording deadlines on a monotonic clock (JAVA-575) - #6090
Draft
runningcode wants to merge 3 commits into
Draft
fix(replay): Measure recording deadlines on a monotonic clock (JAVA-575)#6090runningcode wants to merge 3 commits into
runningcode wants to merge 3 commits into
Conversation
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>
📲 Install BuildsAndroid
|
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Session Replay derives every interval and window from the wall clock. Two of those intervals are pure control flow, and this PR moves them onto
io.sentry.time.Deadline, backed by the options'MonotonicTicker:sessionDurationcapframeTimestamp - replayStartTimestamp >= sessionDurationDeadline.after(ticker, sessionDuration, MILLISECONDS)ACTION_MOVEdebouncelastCapturedMoveEvent + 50 > nowDeadline.after(ticker, 50, MILLISECONDS)On Android the ticker is
SystemClock.elapsedRealtimeNanos(), which advances across device suspend exactly as the wall clock does — so the conversion preserves today's semantics and only removes the sensitivity to a clock step.The cap is still evaluated when a frame is captured rather than when its queued task runs, so a busy replay executor still cannot trip it (covered by the existing
executor delay does not trigger a segment boundarytest).Deadlinealso retires two0-means-unset sentinels (replayStartTimestamp,lastCapturedMoveEvent) in favor of a nullable field, which is whatDeadline's own javadoc argues for.What this PR deliberately does not change
Everything else JAVA-575 lists stays on the wall clock:
SessionCaptureStrategy,BufferCaptureStrategy)ReplayCache.createVideoOf/rotatewindowstimeOffsetsThose values are serialized, and fixing them means re-basing every emitted replay timestamp onto an
AnchoredClock— a change to what customers receive, and therefore v9. A "wall clock for serialization, ticks for windows" hybrid is not an option either: the replay cache survives process death and is replayed on the next launch, and a monotonic tick is meaningless across processes, so frame timestamps must stay epoch on disk.// TODO [MAJOR]markers at bothICurrentDateProviderinjection sites record what changes at v9 and why.💡 Motivation and Context
A backward wall-clock step mid-recording (NTP correction, carrier time, user change — all realistic on phones) makes frames look "newer than now"; a forward step makes a recording look older than it is. For the two sites converted here that means:
Session replay deadline exceeded (1h).Audit finding B3 from JAVA-557.
💚 How did you test it?
./gradlew :sentry-android-replay:testReleaseUnitTest :sentry-android-core:testReleaseUnitTestFour new regression tests, each of which fails on the arithmetic being replaced:
onScreenshotRecorded does not stop replay when the wall clock jumps past the deadlineonScreenshotRecorded stops replay when the wall clock steps backwards past the deadlinewall clock stepping backwards does not suppress move eventswall clock stepping forwards does not lift the debounce earlyThey drive a
TestMonotonicTickerand the fake wall clock independently. The existing tests now advance both together, which is what normal operation looks like.📝 Checklist
sendDefaultPIIis enabled.No
.apidiff: every type touched isinternalKotlin, andReplayIntegration's public constructors are unchanged — the ticker is read fromSentryOptionswhere the strategies are built.🔮 Next steps
The serialized half of JAVA-575 wants one
AnchoredClockper recording, replacingICurrentDateProviderthroughout the replay module, so that every emitted epoch is projected from a single anchor and all internal diffs become tick diffs. That belongs in the v9 bucket alongside JAVA-572 / JAVA-577 / JAVA-642 / JAVA-578.🤖 Generated with Claude Code