Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

0xnm
Copy link
Contributor

@0xnm 0xnm commented Apr 2, 2024

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
Copy link

codecov-commenter commented Apr 3, 2024

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
@@ -436,7 +436,8 @@ internal open class RumViewScope(
stack = it.stack,
state = it.state
)
}.ifEmpty { null }
}.ifEmpty { null },
timeSinceAppStart = event.timeSinceAppStartNs?.let { TimeUnit.NANOSECONDS.toMillis(it) }
Copy link
Collaborator

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
23 checks passed
@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.

None yet

4 participants