Skip to content

Conversation

@satween
Copy link
Contributor

@satween satween commented Nov 18, 2025

What does this PR do?

  • Adds new method to the TimeProvider allowing to get the amount of time (in milliseconds) since process start. The api for that was added only for Android SDK 24, so for previous versions class load time gonna be used.
  • Fixes some assert messages that was copy-pasted and contained wrong values.
  • Reduces code duplications for TimeInfo instance creation.

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)

@satween satween requested review from a team as code owners November 18, 2025 14:40
@satween satween requested a review from ncreated November 18, 2025 14:41
Comment on lines 17 to 18
* @property deviceTimeSinceProcessStartMs the time spend since application's process start (for Android SDK >= 24) or
* between the class being loaded for Android < N as a fallback.
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious if it is fine to have a different behavior for the same property. Maybe it is worth to make this property nullable and return null for API 23? And then it will be up to the caller what to do.

Otherwise we may see inconsistent data if the result of this property is used in some data we send.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I was trying to find a balance between support complexity and value it provides:

  1. getStartElapsedRealtime is supported from the version 24 of Android SDK, current minSdk is 23. So the amount of such devices is relatively small.
  2. This attribute represents the amount of time spend from some moment in the past and represents relativity, not the actual time, from that perspective seems like there no so big difference from which point it would be calculated as the point remains the same for each device.
  3. After @aleksandr-gringauz's suggestion, I changed logic to use AppStartTimeProvider so the value will be more even precise

@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2025

Codecov Report

❌ Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.13%. Comparing base (2a08737) to head (2633d21).
⚠️ Report is 16 commits behind head on develop.

Files with missing lines Patch % Lines
...n/com/datadog/android/core/internal/CoreFeature.kt 0.00% 1 Missing ⚠️
...tadog/android/core/internal/NoOpContextProvider.kt 0.00% 1 Missing ⚠️
...tadog/android/core/internal/NoOpInternalSdkCore.kt 66.67% 1 Missing ⚠️
.../com/datadog/android/internal/time/TimeProvider.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3013      +/-   ##
===========================================
- Coverage    71.32%   71.13%   -0.19%     
===========================================
  Files          859      861       +2     
  Lines        31315    31308       -7     
  Branches      5276     5276              
===========================================
- Hits         22334    22268      -66     
- Misses        7511     7534      +23     
- Partials      1470     1506      +36     
Files with missing lines Coverage Δ
...kotlin/com/datadog/android/api/context/TimeInfo.kt 100.00% <100.00%> (ø)
...kotlin/com/datadog/android/core/InternalSdkCore.kt 0.00% <ø> (ø)
...og/android/core/internal/DatadogContextProvider.kt 97.73% <100.00%> (-0.39%) ⬇️
...n/com/datadog/android/core/internal/DatadogCore.kt 77.59% <100.00%> (-0.60%) ⬇️
.../core/internal/time/DefaultAppStartTimeProvider.kt 100.00% <100.00%> (ø)
...g/android/core/internal/time/KronosTimeProvider.kt 100.00% <ø> (ø)
...adog/android/core/internal/time/TimeProviderExt.kt 100.00% <100.00%> (ø)
...tadog/android/internal/time/DefaultTimeProvider.kt 0.00% <ø> (ø)
...ndroid/telemetry/internal/TelemetryEventHandler.kt 83.85% <100.00%> (-2.23%) ⬇️
...n/com/datadog/android/core/internal/CoreFeature.kt 84.73% <0.00%> (-0.24%) ⬇️
... and 3 more

... and 30 files with indirect coverage changes

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

@satween satween force-pushed the tvaleev/feature/RUM-10907 branch 3 times, most recently from 251e297 to 97946d9 Compare November 18, 2025 18:16
@satween satween force-pushed the tvaleev/feature/RUM-10907 branch from 97946d9 to 2633d21 Compare November 19, 2025 15:45
* The offsets are always 0.
*/
class DefaultTimeProvider : TimeProvider {
override fun getDeviceTimestamp(): Long = System.currentTimeMillis()
Copy link
Member

Choose a reason for hiding this comment

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

is this change needed?

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.

6 participants