Skip to content

RUM-2910: Report time since the application start for crashes in RUM#1961

Merged
0xnm merged 3 commits into
developfrom
nogorodnikov/rum-2910/report-time-since-app-start-for-crashes
Apr 3, 2024
Merged

RUM-2910: Report time since the application start for crashes in RUM#1961
0xnm merged 3 commits into
developfrom
nogorodnikov/rum-2910/report-time-since-app-start-for-crashes

Conversation

@0xnm

@0xnm 0xnm commented Apr 2, 2024

Copy link
Copy Markdown
Member

What does this PR do?

This PR adds the following:

  • Report time since application start for JVM crashes to RUM
  • Report time since application start for NDK crashes to RUM

This is achieved by introducing error.time_since_app_start attribute (in milliseconds). Application start is defined as the start of the application process (the same definition as for the ApplicationLaunch view).

Also this PR moves application start time provider to the core module, because:

  • It is needed for NDK crashes (there is no way to get reliably application start time in NDK; we could use static initialization during the library load, but the library may be loaded later at the application lifecycle)
  • In case of Android API below N we can rely on the DatadogCore class loading, which happens before RumFeature class loading and thus should give more precise timestamp.

This PR doesn't add support of reporting the similar time for the fatal ANRs, because additional development is needed there to keep somehow application start time between application launches.

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)

@codecov-commenter

codecov-commenter commented Apr 3, 2024

Copy link
Copy Markdown

Codecov Report

Merging #1961 (62aaed6) into develop (a107d7c) will increase coverage by 0.07%.
The diff coverage is 94.12%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1961      +/-   ##
===========================================
+ Coverage    83.24%   83.31%   +0.07%     
===========================================
  Files          489      489              
  Lines        17857    17860       +3     
  Branches      2661     2660       -1     
===========================================
+ Hits         14865    14880      +15     
+ Misses        2264     2252      -12     
  Partials       728      728              
Files Coverage Δ
...ain/kotlin/com/datadog/android/core/DatadogCore.kt 81.98% <100.00%> (+0.25%) ⬆️
...n/com/datadog/android/core/internal/CoreFeature.kt 86.77% <100.00%> (+0.09%) ⬆️
.../core/internal/time/DefaultAppStartTimeProvider.kt 100.00% <100.00%> (ø)
...dog/android/ndk/internal/DatadogNdkCrashHandler.kt 82.25% <100.00%> (+0.11%) ⬆️
...dog/android/ndk/internal/NdkCrashReportsFeature.kt 69.64% <100.00%> (+3.64%) ⬆️
...g/android/rum/internal/DatadogLateCrashReporter.kt 86.44% <100.00%> (-0.26%) ⬇️
...lin/com/datadog/android/rum/internal/RumFeature.kt 92.97% <ø> (+0.89%) ⬆️
...d/rum/internal/domain/scope/RumApplicationScope.kt 93.41% <100.00%> (-0.07%) ⬇️
...g/android/rum/internal/domain/scope/RumRawEvent.kt 100.00% <100.00%> (ø)
.../android/rum/internal/domain/scope/RumViewScope.kt 93.66% <100.00%> (-0.28%) ⬇️
... and 3 more

... and 19 files with indirect coverage changes

@0xnm
0xnm marked this pull request as ready for review April 3, 2024 07:23
@0xnm
0xnm requested review from a team as code owners April 3, 2024 07:23
)
}.ifEmpty { null }
}.ifEmpty { null },
timeSinceAppStart = event.timeSinceAppStartNs?.let { TimeUnit.NANOSECONDS.toMillis(it) }

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.

maybe you should extract this in a function ?

@0xnm
0xnm merged commit c14efb3 into develop Apr 3, 2024
@0xnm
0xnm deleted the nogorodnikov/rum-2910/report-time-since-app-start-for-crashes branch April 3, 2024 09:45
@xgouchet xgouchet added this to the 2.8.x milestone Apr 5, 2024
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