Skip to content

RUM-971: Report ApplicationLaunch view even if first RUM event is not interaction#1591

Merged
0xnm merged 1 commit into
developfrom
nogorodnikov/rum-971/application-start-is-not-reported
Aug 31, 2023
Merged

RUM-971: Report ApplicationLaunch view even if first RUM event is not interaction#1591
0xnm merged 1 commit into
developfrom
nogorodnikov/rum-971/application-start-is-not-reported

Conversation

@0xnm

@0xnm 0xnm commented Aug 29, 2023

Copy link
Copy Markdown
Member

What does this PR do?

Fixes #1582 and maybe fixes #1523.

This PR fixes the issue when ApplicationLaunch is not reported in case if first RUM event coming is not interaction event. In this case session is not yet active, so we create ApplicationLaunch and ApplicationStarted action for nothing - they won't be written, because writer is no-op.

This PR moves the logic of creating ApplicationStarted event up to the application scope, which makes sense - we have to report it only once in the application lifecycle. Also session we be activated once ApplicationStarted event is received by the session scope, meaning that session is created not only by start view or start action events (which makes sense, because we introduced ApplicationLaunch view to be independent from background tracking feature).

There is one leftover which I don't personally enjoy - having applicationDisplayed property in the scopes (I think this property is vague), but getting rid of it is tricky.

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 changed the title RUM-917: Report ApplicationLaunch view even if first RUM event is not interaction RUM-971: Report ApplicationLaunch view even if first RUM event is not interaction Aug 29, 2023
@0xnm
0xnm force-pushed the nogorodnikov/rum-971/application-start-is-not-reported branch from beaae57 to 8090ec4 Compare August 29, 2023 15:05
@codecov-commenter

codecov-commenter commented Aug 29, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1591 (8090ec4) into develop (9b9a19c) will increase coverage by 0.12%.
The diff coverage is 100.00%.

❗ Current head 8090ec4 differs from pull request most recent head e58100d. Consider uploading reports for the commit e58100d to get more accurate results

@@             Coverage Diff             @@
##           develop    #1591      +/-   ##
===========================================
+ Coverage    83.46%   83.57%   +0.12%     
===========================================
  Files          443      443              
  Lines        15118    15122       +4     
  Branches      2262     2264       +2     
===========================================
+ Hits         12617    12638      +21     
+ Misses        1894     1880      -14     
+ Partials       607      604       -3     
Files Changed Coverage Δ
...d/rum/internal/domain/scope/RumApplicationScope.kt 89.47% <100.00%> (+5.26%) ⬆️
...droid/rum/internal/domain/scope/RumSessionScope.kt 95.40% <100.00%> (-0.05%) ⬇️
...d/rum/internal/domain/scope/RumViewManagerScope.kt 96.92% <100.00%> (+0.40%) ⬆️

... and 17 files with indirect coverage changes

@0xnm
0xnm marked this pull request as ready for review August 29, 2023 15:26
@0xnm
0xnm requested a review from a team as a code owner August 29, 2023 15:26
xgouchet
xgouchet previously approved these changes Aug 29, 2023

@xgouchet xgouchet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work

jonathanmos
jonathanmos previously approved these changes Aug 30, 2023
mariusc83
mariusc83 previously approved these changes Aug 31, 2023
@0xnm
0xnm dismissed stale reviews from mariusc83, jonathanmos, and xgouchet via e58100d August 31, 2023 07:55
@0xnm
0xnm force-pushed the nogorodnikov/rum-971/application-start-is-not-reported branch from 8090ec4 to e58100d Compare August 31, 2023 07:55
@0xnm
0xnm merged commit bf9b6c3 into develop Aug 31, 2023
@0xnm
0xnm deleted the nogorodnikov/rum-971/application-start-is-not-reported branch August 31, 2023 08:19
@xgouchet xgouchet added this to the 2.1.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants