RUM-15269: Fix DefaultAppStartTimeProvider guard to handle result > createTimeNs#3339
Merged
Merged
Conversation
…reateTimeNs The existing guard only caught the case where getStartElapsedRealtime() returns a value too small (result too far in the past). When it returns a value larger than the current elapsedRealtime(), diffMs goes negative and result exceeds System.nanoTime(), bypassing the guard entirely and causing mustReturnTheCorrectAppStartTime_when_appStartTimeNs to fail on CI. Extended the condition to also fall back to DdRumContentProvider.createTimeNs when result > createTimeNs, since app start can never appear after content provider initialisation. Added a unit test covering this new direction. Updated the three uptime tests to explicitly set createTimeNs to a value consistent with the production invariant (createTimeNs >= result) so the guard does not interfere with what those tests verify.
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3339 +/- ##
===========================================
- Coverage 71.70% 71.53% -0.18%
===========================================
Files 946 946
Lines 34895 34901 +6
Branches 5906 5908 +2
===========================================
- Hits 25020 24963 -57
- Misses 8244 8273 +29
- Partials 1631 1665 +34
🚀 New features to boost your workflow:
|
hamorillo
marked this pull request as ready for review
April 7, 2026 06:27
0xnm
approved these changes
Apr 7, 2026
0xnm
left a comment
Member
There was a problem hiding this comment.
Great note about clock type mismatch! We definitely should address this.
Contributor
We could use SystemClock.uptimeMillis here instead of But yes - this is a topic for a separate PR. |
aleksandr-gringauz
approved these changes
Apr 7, 2026
Contributor
Author
|
I've created a ticket for the clock mismatch. RUM-15483 |
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.
What does this PR do?
Extends the fallback guard in
DefaultAppStartTimeProviderto also handle the case where the computedappStartTimeNsis greater thanDdRumContentProvider.createTimeNs, which is an impossible value since the process always starts before the content provider initialises. Adds a unit test covering this direction, and updates existing uptime tests to setcreateTimeNsconsistently with the production invariant.Motivation
The CI integration test
mustReturnTheCorrectAppStartTime_when_appStartTimeNsfailed with ajava.lang.AssertionError. The full assertion message was truncated in the CI log, so we cannot confirm the exact values involved, but I think the root cause is a buggy value returned byProcess.getStartElapsedRealtime().This API is known to occasionally return wrong values on API 28+ (confirmed in RUM-13082 and documented in this article). I think that when it returns a value larger than the current
elapsedRealtime(),diffMsgoes negative andresultexceedsSystem.nanoTime().The existing guard only caught the case where
resultwas too far in the past:When
result > createTimeNs, this expression is negative and the guard silently passes the invalid value through.Additional context — BOOTTIME vs MONOTONIC clock mismatch
While investigating the failure we also noticed a separate structural issue in the formula:
This mixes two different Android clocks:
getDeviceElapsedRealtimeMillis()→SystemClock.elapsedRealtime()—CLOCK_BOOTTIME, advances during device sleepgetDeviceElapsedTimeNanos()→System.nanoTime()—CLOCK_MONOTONIC, does not advance during sleepOn a device that sleeps after the process starts,
CLOCK_BOOTTIMEaccumulates more time thanCLOCK_MONOTONIC, inflatingdiffMsand makingresulttoo small. SinceappStartTimeNsfeeds RUM startup duration, TTID, fatal crashtimeSinceAppStartNs, and NDK crash timestamps, a wrong value here silently corrupts multiple RUM and crash metrics. This is not addressed by this PR. A proper fix would requireProcess.getStartUptimeMillis()(available from API 33) to keep the computation in the MONOTONIC domain throughout.Review checklist (to be filled by reviewers)