Skip to content

RUM-15269: Fix DefaultAppStartTimeProvider guard to handle result > createTimeNs#3339

Merged
hamorillo merged 1 commit into
developfrom
hector.morilloprieto/RUM-15269
Apr 7, 2026
Merged

RUM-15269: Fix DefaultAppStartTimeProvider guard to handle result > createTimeNs#3339
hamorillo merged 1 commit into
developfrom
hector.morilloprieto/RUM-15269

Conversation

@hamorillo

Copy link
Copy Markdown
Contributor

What does this PR do?

Extends the fallback guard in DefaultAppStartTimeProvider to also handle the case where the computed appStartTimeNs is greater than DdRumContentProvider.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 set createTimeNs consistently with the production invariant.

Motivation

The CI integration test mustReturnTheCorrectAppStartTime_when_appStartTimeNs failed with a java.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 by Process.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(), diffMs goes negative and result exceeds System.nanoTime().

The existing guard only caught the case where result was too far in the past:

// Before — only catches one direction
if (DdRumContentProvider.createTimeNs - result > THRESHOLD)

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:

val diffMs = getDeviceElapsedRealtimeMillis() - Process.getStartElapsedRealtime()
val result  = getDeviceElapsedTimeNanos() - TimeUnit.MILLISECONDS.toNanos(diffMs)

This mixes two different Android clocks:

  • getDeviceElapsedRealtimeMillis()SystemClock.elapsedRealtime()CLOCK_BOOTTIME, advances during device sleep
  • getDeviceElapsedTimeNanos()System.nanoTime()CLOCK_MONOTONIC, does not advance during sleep

On a device that sleeps after the process starts, CLOCK_BOOTTIME accumulates more time than CLOCK_MONOTONIC, inflating diffMs and making result too small. Since appStartTimeNs feeds RUM startup duration, TTID, fatal crash timeSinceAppStartNs, 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 require Process.getStartUptimeMillis() (available from API 33) to keep the computation in the MONOTONIC domain throughout.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

…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.
@datadog-prod-us1-5

This comment has been minimized.

@codecov-commenter

codecov-commenter commented Apr 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.53%. Comparing base (b87696e) to head (f19d8ce).
⚠️ Report is 25 commits behind head on develop.

Files with missing lines Patch % Lines
.../core/internal/time/DefaultAppStartTimeProvider.kt 90.00% 0 Missing and 1 partial ⚠️
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     
Files with missing lines Coverage Δ
.../core/internal/time/DefaultAppStartTimeProvider.kt 95.00% <90.00%> (-5.00%) ⬇️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hamorillo
hamorillo marked this pull request as ready for review April 7, 2026 06:27
@hamorillo
hamorillo requested review from a team as code owners April 7, 2026 06:27

@0xnm 0xnm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great note about clock type mismatch! We definitely should address this.

@aleksandr-gringauz

Copy link
Copy Markdown
Contributor

A proper fix would require Process.getStartUptimeMillis() (available from API 33) to keep the computation in the MONOTONIC domain throughout.

We could use SystemClock.uptimeMillis here instead of SystemClock.elapsedRealtime. IIUC it is the same clock as System.nanoTime.

But yes - this is a topic for a separate PR.

@hamorillo

Copy link
Copy Markdown
Contributor Author

I've created a ticket for the clock mismatch. RUM-15483

@hamorillo
hamorillo merged commit 1425287 into develop Apr 7, 2026
27 checks passed
@hamorillo
hamorillo deleted the hector.morilloprieto/RUM-15269 branch April 7, 2026 12:29
@ncreated
ncreated restored the hector.morilloprieto/RUM-15269 branch April 9, 2026 09:49
@0xnm
0xnm deleted the hector.morilloprieto/RUM-15269 branch April 24, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants