RUMM-1765 Update CrashContext with necessary info for handling App Launch crashes - part 3

[ncreated]

What and why?

📦 This PR prepares CrashContext to handle crashes during application launch. This is necessary to decide:

• if the crash should be send in "ApplicationLaunch" view (when app crashed during foreground launch);
• if the crash should be send in "Background" view (when app crashed during background launch);
• if the crash should be dropped (e.g. when app crashed during background launch, but Background Events Tracking is disabled OR when crashed session was rejected by sampler).

How?

First, I isolated the code deciding on "ApplicationLaunch" x "Background" x "no" view to RUMOffViewEventsHandlingRule. This logic is quite complex but very important (for customer billing 💸) so it's definitely worth having it separate as it will be used by both RUM and Crash Reporting to take similar decision on handling events or crashes while no active view:

Second, I updated CrashContext with all values necessary for making the decision ☝️ in RUMOffViewEventsHandlingRule. This way, when the app restarts upon the crash, we will be able to run exactly the same logic and decide if the crash should be sent in "ApplicationLaunch" view, "Background" view or if it should be dropped (e.g. due to sampling).

Crash handling upon app restart will be delivered in separate, final PR to this topic.

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

[ncreated]

Here, I updated RUMSessionScope tests to transitively cover all cases in RUMOffViewEventsHandlingRule used internally. I took this approach instead of abstracting RUMOffViewEventsHandlingRule through interface and mocking it for tests. I wanted to optimise code readability in RUMSessionScope, even if the cost was more effort in fuzzy testing.

[ncreated]

I added this missing test, because now RUMWithCrashContextIntegration is injected through DI and we're able to configure the mock.

[maxep]

Very clear! The doc and the separation of concern applied in RUMOffViewEventsHandlingRule help understanding the complexity of this feature.

I'm wondering about the rules applied when session state is nil, I wonder if we shouldn't drop the event instead.

[maxep]

Since a event cannot be sent outside of a session, shouldn't we apply this rule only if we have a session state? this would mean making the state non-optional.
And if it's more convenient to keep it optional, why not dropping the event if the session state is nil?

[ncreated]

This (sessionState: nil) is used later in #695 to decide on handling crashes which occurred as the very first event in the app. For example, in case of:

Datadog.initialize(
   ...
)

fatalError("crash")

we don't have RUMSession yet created, so its persisted (through CrashContext) state will be nil.

I've spent a while trying to model the minimal information we need to persist in CrashContext for differentiating crashes during app launch in foreground vs background and resulted with this RUMSessionState. If you can find a better model, I'd be happy to adopt it and simplify things 🙂 🙌! I'm all for consistency here, making things optional just for convenience might be error-prone.

[maxep]

I think the model is great and simplifies the rule definition!

In the next PRs, will the RUMSessionState.sessionUUID will be leveraged somehow for other purposes than checking if it's sampled ou?
I'm wondering if we could do something like:

internal struct RUMSessionState: Equatable, Codable {
    let isSampled: Bool
    let isInitialSession: Bool
    let hasTrackedAnyView: Bool

    static let `default` = RUMSessionState(isSampled: false, isInitialSession: true, hasTrackedAnyView: false)
}

This way, we could make the state non-optional and clarify the statements in this init?

[ncreated]

In the next PRs, will the RUMSessionState.sessionUUID will be leveraged somehow for other purposes than checking if it's sampled out?

Now I think that in #695 I forgot to handle the case which I spotted when designing RUMSessionState 😰. Indeed, it may look now that sessionUUID is not necessary and we don't use it in #695 now.

The plan was to use sessionUUID in this scenario:

• 1/ SDK has Background Events Tracking enabled and views instrumentation turned ON;
• 2/ It tracks some views, leading to updating lastRUMViewEvent in CrashContext;
• 3/ Then the app is sent to background (so it stops the recent view);
• 4/ No other event occurs in the background except the crash.

In such case, because there is no active view (3) and the app crashed in background (4) we should send the crash in "Background" view (1). No additional sampling should be considered and both crash and its "Background" view should be sent to the sessionUUID used for tracking other views in this session (2).

That said: no, isSampled: Bool wouldn't be enough for RUMSessionState - we also need to know its uuid for above case ☝️. Knowing that .nullUUID means "not sampled", it also simplifies few more edge cases.

I will update next PR (#695) with necessary logic for handling this case 👍. Otherwise the RUMSessionState persistence in CrashContext doesn't make sense at all.

[maxep]

Makes sense then 👍