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-1702: Start session when RUM is initialized #1832

Merged

Conversation

0xnm
Copy link
Contributor

@0xnm 0xnm commented Jan 22, 2024

What does this PR do?

This change starts RUM session during the Rum.enable call. The general rule is like that:

  • If application process has foreground type, then session will be started (with interaction time set at the moment of initialization), and ApplicationLaunch view with application_started event will be created.
  • Otherwise, if application process has something different from foreground as type, session will be initialized only if background tracking is enabled, no ApplicationLaunch view will be created.

This PR is using the notion of SdkInit class, even though it is possible to use only ApplicationStarted event in order to be on-par with the similar logic in iOS SDK. Also it is still using event system for the session initialization in order to minimize the footprint of the change (it will be easier to rollback it if needed).

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)

@0xnm 0xnm force-pushed the nogorodnikov/rum-1702/start-session-when-rum-is-initialized branch from 2c389f1 to 797d76a Compare January 22, 2024 10:46
@codecov-commenter
Copy link

codecov-commenter commented Jan 22, 2024

Codecov Report

Merging #1832 (14025c5) into develop (8145782) will decrease coverage by 0.05%.
The diff coverage is 97.67%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1832      +/-   ##
===========================================
- Coverage    83.49%   83.45%   -0.05%     
===========================================
  Files          467      467              
  Lines        16491    16476      -15     
  Branches      2483     2482       -1     
===========================================
- Hits         13769    13749      -20     
- Misses        2043     2045       +2     
- Partials       679      682       +3     
Files Coverage Δ
...rum/src/main/kotlin/com/datadog/android/rum/Rum.kt 95.56% <100.00%> (+0.82%) ⬆️
...d/rum/internal/domain/scope/RumApplicationScope.kt 90.41% <100.00%> (-4.15%) ⬇️
...g/android/rum/internal/domain/scope/RumRawEvent.kt 100.00% <100.00%> (ø)
...d/rum/internal/domain/scope/RumViewManagerScope.kt 96.99% <ø> (ø)
.../android/rum/internal/monitor/DatadogRumMonitor.kt 83.95% <100.00%> (-0.30%) ⬇️
...droid/rum/internal/domain/scope/RumSessionScope.kt 96.75% <95.45%> (+0.52%) ⬆️

... and 26 files with indirect coverage changes

@0xnm 0xnm force-pushed the nogorodnikov/rum-1702/start-session-when-rum-is-initialized branch from 797d76a to 198cbb6 Compare January 22, 2024 12:24
@0xnm 0xnm force-pushed the nogorodnikov/rum-1702/start-session-when-rum-is-initialized branch from 198cbb6 to 14025c5 Compare January 22, 2024 12:29
@0xnm 0xnm marked this pull request as ready for review January 22, 2024 13:50
@0xnm 0xnm requested review from a team as code owners January 22, 2024 13:50
Copy link
Contributor

@fuzzybinary fuzzybinary left a comment

Choose a reason for hiding this comment

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

This LGTM. Agree we should have @ncreated take a look and make sure the logic is the same.

Copy link
Contributor

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

It looks okay 👍. Good idea to map the new "sdk init" command into existing "app start" to minimise the change impact 👌.

@0xnm 0xnm merged commit 2bc3526 into develop Jan 23, 2024
23 checks passed
@0xnm 0xnm deleted the nogorodnikov/rum-1702/start-session-when-rum-is-initialized branch January 23, 2024 14:37
@xgouchet xgouchet added this to the 2.6.0 milestone Feb 19, 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

5 participants