RUMM-1765 Handle App Launch crashes - part 4 #695
Conversation
so it can be used both in RUM and Crash Reporting for sampling session and/or crash report.
Sources/Datadog/FeaturesIntegration/CrashReporting/CrashReportingWithRUMIntegration.swift
Outdated
Show resolved
Hide resolved
…ssion while there was no Background view started.
| @@ -148,7 +277,7 @@ internal struct CrashReportingWithRUMIntegration: CrashReportingIntegration { | |||
| application: original.application, | |||
| connectivity: original.connectivity, | |||
| context: original.context, | |||
| date: crashDate.timeIntervalSince1970.toInt64Milliseconds, | |||
| date: crashDate.timeIntervalSince1970.toInt64Milliseconds - 1, // -1ms to put the crash after view in RUM session | |||
There was a problem hiding this comment.
Could we fix it in BE instead? or FE?
There was a problem hiding this comment.
I asked on frontend channel, let's see 👌. Until it's solved in FE I think we can keep this workaround - all in all, we're making up this view update by ourself, so IMO it's not a big deal that we choose to send it -1ms before the crash. WDYT?
There was a problem hiding this comment.
Sounds good 👍
Indeed, it would be great to cover this on FE!
| lastRUMViewEventInPreviousSession lastRUMViewEvent: RUMEvent<RUMViewEvent>, | ||
| using crashTimings: AdjustedCrashTimings | ||
| ) { | ||
| if crashTimings.realDateNow.timeIntervalSince(crashTimings.realCrashDate) < Constants.viewEventAvailabilityThreshold { |
There was a problem hiding this comment.
Yes, it's been like this since beginning of crash reporting. Problem is that backend considers all RUM sessions as "closed" after 4h, so sending additional view update to it might introduce inconsistency. It was discussed with BE team, and this is the best we came up with.
There was a problem hiding this comment.
Ok 👍 Shouldn't be a big deal in that specific case and totally out of the scope of this PR, but I'm concerned that if we change that value BE side, for any reason, we will have inconsistencies.
Would be great to pull some configuration from BE at some point.
| import Foundation | ||
|
|
||
| /// Sampler, deciding if events should be sent do Datadog or dropped. | ||
| internal struct Sampler { |
There was a problem hiding this comment.
/nit could be handy to make this ExpressibleByFloatLiteral, we would avoid having to update all tests.
extension Sampler: ExpressibleByFloatLiteral {
init(floatLiteral value: Float) {
self.init(samplingRate: value)
}
}There was a problem hiding this comment.
Nice! I wish I could think of it before doing refactoring in the first place. But now as it is done, I think that having explicit Sampler type passed around just reads better - at least it's less magical.
What and why?
📦 This PR adds the actual handling of application launch crashes. When app crashes before starting the RUM session, the crash is uploaded in a new, artificial session indicated either by "ApplicationLaunch" or "Background" view, depending on the app state during crash.
How?
The
RUMOffViewEventsHandlingRuleintroduced in #690 is now reused inCrashReportingWithRUMIntegrationto decided on how to send the crash. The overall logic of processing crashes in RUM looks this:To make it work as described in the original RFC, I had to inject sampling logic into
CrashReportingWithRUMIntegration. To reuse existing code, I isolated it into singleSamplercomponent (defined in "core").Review checklist