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

RUMM-1765 Collect RUM events during application launch - part 1 #677

Conversation

ncreated
Copy link
Member

@ncreated ncreated commented Dec 1, 2021

What and why?

📦 This is the first PR on application launch events tracking. It implements the base logic of starting "ApplicationLaunch" view if any RUM event is received before tracking the first view:

Screenshot 2021-12-01 at 14 55 22

How?

It was known that this feature will collide with existing Background Events Tracking option, which creates "Background" view for capturing off-view events. Most of the effort in this PR is focused on decoupling both and implementing a clear separation of following cases:

  • when event is received and existing session didn't yet track any view → start "ApplicationLaunch" view;
  • when event is received and existing session has tracked some view → start "Background" view (if BET is enabled);

This required introducing a notion of "initial RUM session" - the very first session in current app process. If session times out, next RUM session is not marked as "initial".

To make code more readable and the implementation more explicit, I added two values to the RUMCommand interface, so we can clearly know if a given event should result with a side effect in off-view tracking:

/// Whether or not receiving this command should start the "Background" view if no view is active
/// and ``Datadog.Configuration.Builder.trackBackgroundEvents(_:)`` is enabled.
var canStartBackgroundView: Bool { get }

/// Whether or not receiving this command should start the "ApplicationLaunch" view if no view was yet started in current app process.
var canStartApplicationLaunchView: Bool { get }

Next PRs

There are two key elements missing in this PR, which will be delivered in separate PRs:

  • When the app is launched in background (e.g. due to location change), we should not create "ApplicationLaunch" view - instead, "Background" view should be created if user opted-in for BET.
  • When the app crashes while there is no active RUM view, we need to handle this crash respectively from restarted session. We need to consider if it should be sent within "ApplicationLaunch" view, "Background" view or sampled.

More on these edge cases can be read in "RFC - Collecting App Launch Crashes in Mobile SDKs".

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

…erty

It also fixes the bug where existing, but inactive view scope was preventing "Background" view
from being started. This was corresponding to the scenario when an app is sent to background while
a resource is being loaded - until the resource finished, no "Background" view could be started.
this is to capture "app launch" events before starting first view.
@ncreated ncreated requested a review from a team as a code owner December 1, 2021 14:32
@ncreated ncreated self-assigned this Dec 1, 2021
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ncreated ncreated merged commit 9af710e into ncreated/RUMM-1765-app-launch-events-tracking Dec 6, 2021
@ncreated ncreated deleted the ncreated/RUMM-1765-collect-rum-events-during-application-launch branch December 6, 2021 14:50
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.

3 participants