Skip to content

RUM-17112: Remove default = Time() from RumRawEvent subclasses#3608

Merged
kikoveiga merged 1 commit into
developfrom
kikoveiga/rum-17112/remove-default-time
Jul 8, 2026
Merged

RUM-17112: Remove default = Time() from RumRawEvent subclasses#3608
kikoveiga merged 1 commit into
developfrom
kikoveiga/rum-17112/remove-default-time

Conversation

@kikoveiga

@kikoveiga kikoveiga commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Removes the = Time() default from all RumRawEvent subclasses and makes eventTime an explicit constructor argument, sourced through the injected TimeProvider.
  • Adds companion factories Time.now(timeProvider); Time.fromTimestampMillis(timestamp, timeProvider) and Time.fromNanoTime(nanoTime, timeProvider) replace the old extensions.
  • Adds a TimeForgeryFactory and reuses it in tests.

Motivation

The no-arg Time() called System.currentTimeMillis() and System.nanoTime() apis directly, bypassing TimeProvider. This finished the goal of #3011 and makes RUM event timing fully deterministic and testable.

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)

@kikoveiga
kikoveiga force-pushed the kikoveiga/rum-17112/remove-default-time branch from dd56a76 to 501ba86 Compare July 5, 2026 17:38
@codecov-commenter

codecov-commenter commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.86992% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.88%. Comparing base (fbb4569) to head (bf0f783).
⚠️ Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
.../android/rum/internal/monitor/DatadogRumMonitor.kt 78.72% 7 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3608      +/-   ##
===========================================
- Coverage    72.89%   72.88%   -0.01%     
===========================================
  Files          975      975              
  Lines        35277    35298      +21     
  Branches      5972     5974       +2     
===========================================
+ Hits         25712    25724      +12     
- Misses        7905     7915      +10     
+ Partials      1660     1659       -1     
Files with missing lines Coverage Δ
...in/com/datadog/android/rum/internal/domain/Time.kt 100.00% <100.00%> (ø)
...d/rum/internal/domain/scope/RumApplicationScope.kt 94.57% <100.00%> (+0.04%) ⬆️
...g/android/rum/internal/domain/scope/RumRawEvent.kt 100.00% <100.00%> (ø)
.../android/rum/internal/domain/scope/RumViewScope.kt 94.70% <100.00%> (-0.49%) ⬇️
...roid/rum/internal/startup/RumAppStartupDetector.kt 100.00% <100.00%> (ø)
.../rum/internal/startup/RumAppStartupDetectorImpl.kt 96.59% <100.00%> (ø)
.../android/rum/internal/monitor/DatadogRumMonitor.kt 87.83% <78.72%> (-0.68%) ⬇️

... 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.

@kikoveiga
kikoveiga force-pushed the kikoveiga/rum-17112/remove-default-time branch 6 times, most recently from 2803b08 to bf0f783 Compare July 8, 2026 15:22
@kikoveiga
kikoveiga marked this pull request as ready for review July 8, 2026 15:27
@kikoveiga
kikoveiga requested review from a team as code owners July 8, 2026 15:27
abrooksv
abrooksv previously approved these changes Jul 8, 2026
@kikoveiga
kikoveiga force-pushed the kikoveiga/rum-17112/remove-default-time branch from bf0f783 to 8f747cf Compare July 8, 2026 16:23
@kikoveiga kikoveiga self-assigned this Jul 8, 2026
@kikoveiga
kikoveiga merged commit ac4207d into develop Jul 8, 2026
27 checks passed
@kikoveiga
kikoveiga deleted the kikoveiga/rum-17112/remove-default-time branch July 8, 2026 17:41
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.

3 participants