Skip to content

RUM-1336 Add session start reason to events#1755

Merged
xgouchet merged 3 commits into
developfrom
xgouchet/RUM-1336/session_start_reason
Dec 11, 2023
Merged

RUM-1336 Add session start reason to events#1755
xgouchet merged 3 commits into
developfrom
xgouchet/RUM-1336/session_start_reason

Conversation

@xgouchet

@xgouchet xgouchet commented Dec 8, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a session_precondition attribute to all RUM event to track the reason the session was created.

Motivation

Get more information on what leads to a new session starting, as part of our Session Management work

@xgouchet
xgouchet requested review from a team as code owners December 8, 2023 08:26
@codecov-commenter

codecov-commenter commented Dec 8, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1755 (040cd6a) into develop (f38f6be) will increase coverage by 0.06%.
Report is 6 commits behind head on develop.
The diff coverage is 95.74%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1755      +/-   ##
===========================================
+ Coverage    83.34%   83.40%   +0.06%     
===========================================
  Files          468      468              
  Lines        16304    16383      +79     
  Branches      2431     2442      +11     
===========================================
+ Hits         13587    13663      +76     
  Misses        2050     2050              
- Partials       667      670       +3     
Files Coverage Δ
.../datadog/android/rum/internal/domain/RumContext.kt 100.00% <100.00%> (+4.35%) ⬆️
...ndroid/rum/internal/domain/scope/RumActionScope.kt 96.51% <100.00%> (+0.04%) ⬆️
...roid/rum/internal/domain/scope/RumResourceScope.kt 93.92% <100.00%> (-0.29%) ⬇️
.../android/rum/internal/domain/scope/RumViewScope.kt 94.37% <100.00%> (+0.07%) ⬆️
...droid/rum/internal/domain/scope/RumSessionScope.kt 96.23% <95.24%> (+0.72%) ⬆️
...g/android/rum/internal/domain/scope/RumEventExt.kt 92.98% <92.50%> (-0.08%) ⬇️

... and 18 files with indirect coverage changes

val sessionState = RumSessionScope.State.fromString(
featureContext[SESSION_STATE] as? String
)
val sessionStartReason = RumSessionScope.StartReason.fromString(

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.

we need to update toMap method then as well, to write it to the context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch

} else if (isExpired) {
if (backgroundTrackingEnabled && isBackgroundEvent) {
renewSession(nanoTime)
renewSession(nanoTime, StartReason.INACTIVITY_TIMEOUT)

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.

I think here we can qualify it as background_launch, maybe? If I'm not wrong, we can hit this line in case if application/session was started by some background event (but I may have outdated memories of this logic).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per the ticket:

Note

The following will not be implemented yet because we don’t track non-interactive sessions.

@xgouchet
xgouchet requested a review from 0xnm December 11, 2023 09:34
@xgouchet
xgouchet merged commit 2f894e9 into develop Dec 11, 2023
@xgouchet
xgouchet deleted the xgouchet/RUM-1336/session_start_reason branch December 11, 2023 10:31
@xgouchet xgouchet added this to the 2.4.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

Development

Successfully merging this pull request may close these issues.

4 participants