Skip to content

RUM-1702: Start session when RUM is initialized#1832

Merged
0xnm merged 1 commit into
developfrom
nogorodnikov/rum-1702/start-session-when-rum-is-initialized
Jan 23, 2024
Merged

RUM-1702: Start session when RUM is initialized#1832
0xnm merged 1 commit into
developfrom
nogorodnikov/rum-1702/start-session-when-rum-is-initialized

Conversation

@0xnm

@0xnm 0xnm commented Jan 22, 2024

Copy link
Copy Markdown
Member

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

codecov-commenter commented Jan 22, 2024

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.67442% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.45%. Comparing base (8145782) to head (14025c5).
⚠️ Report is 1822 commits behind head on develop.

Files with missing lines Patch % Lines
...droid/rum/internal/domain/scope/RumSessionScope.kt 95.45% 0 Missing and 1 partial ⚠️
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 with missing lines 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

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

@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

@fuzzybinary fuzzybinary left a comment

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.

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

@ncreated ncreated left a comment

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.

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

5 participants