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 Do not start "ApplicationLaunch" view when app launches in background - part 2 #685

3 changes: 3 additions & 0 deletions Sources/Datadog/Core/Feature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@ internal struct FeaturesCommonDependencies {
let performance: PerformancePreset
let httpClient: HTTPClient
let mobileDevice: MobileDevice
/// Time of SDK initialization, measured in device date.
let sdkInitDate: Date
let dateProvider: DateProvider
let dateCorrector: DateCorrectorType
let userInfoProvider: UserInfoProvider
let networkConnectionInfoProvider: NetworkConnectionInfoProviderType
let carrierInfoProvider: CarrierInfoProviderType
let launchTimeProvider: LaunchTimeProviderType
let appStateListener: AppStateListening
}

internal struct FeatureStorage {
Expand Down
31 changes: 19 additions & 12 deletions Sources/Datadog/Core/System/AppStateListener.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,23 @@ import class UIKit.UIApplication
internal struct AppStateHistory: Equatable {
/// Snapshot of the app state at `date`
struct Snapshot: Equatable {
/// If the app is running in the foreground and currently receiving events.
let isActive: Bool
/// Date of recording this snapshot.
let date: Date
}

var initialState: Snapshot
var changes = [Snapshot]()
var finalDate: Date
var finalState: Snapshot {
fileprivate(set) var initialState: Snapshot
fileprivate(set) var changes = [Snapshot]()

/// Date of last the update to `AppStateHistory`.
fileprivate(set) var recentDate: Date

/// The most recent app state `Snapshot`.
var currentState: Snapshot {
return Snapshot(
isActive: (changes.last ?? initialState).isActive,
date: finalDate
date: recentDate
)
}

Expand All @@ -37,7 +43,7 @@ internal struct AppStateHistory: Equatable {
date: range.lowerBound
)
// move final state to upperBound
taken.finalDate = range.upperBound
taken.recentDate = range.upperBound
// filter changes outside of the range
taken.changes = taken.changes.filter { range.contains($0.date) }
return taken
Expand All @@ -46,7 +52,7 @@ internal struct AppStateHistory: Equatable {
var foregroundDuration: TimeInterval {
var duration: TimeInterval = 0.0
var lastActiveStartDate: Date?
let allEvents = [initialState] + changes + [finalState]
let allEvents = [initialState] + changes + [currentState]
for event in allEvents {
if let startDate = lastActiveStartDate {
duration += event.date.timeIntervalSince(startDate)
Expand All @@ -61,16 +67,16 @@ internal struct AppStateHistory: Equatable {
}

var didRunInBackground: Bool {
return !initialState.isActive || !finalState.isActive
return !initialState.isActive || !currentState.isActive
}

private func isActive(at date: Date) -> Bool {
if date <= initialState.date {
// we assume there was no change before initial state
return initialState.isActive
} else if finalState.date <= date {
} else if currentState.date <= date {
// and no change after final state
return finalState.isActive
return currentState.isActive
}
var active = initialState
for change in changes {
Expand All @@ -83,6 +89,7 @@ internal struct AppStateHistory: Equatable {
}
}

/// Provides history of app foreground / background states.
internal protocol AppStateListening: AnyObject {
var history: AppStateHistory { get }
}
Expand All @@ -95,7 +102,7 @@ internal class AppStateListener: AppStateListening {

var history: AppStateHistory {
var current = publisher.currentValue
current.finalDate = dateProvider.currentDate()
current.recentDate = dateProvider.currentDate()
return current
}

Expand All @@ -115,7 +122,7 @@ internal class AppStateListener: AppStateListening {
self.publisher = ValuePublisher(
initialValue: AppStateHistory(
initialState: currentState,
finalDate: currentState.date
recentDate: currentState.date
)
)

Expand Down
7 changes: 4 additions & 3 deletions Sources/Datadog/Datadog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,14 @@ public class Datadog {
performance: configuration.common.performance,
httpClient: HTTPClient(proxyConfiguration: configuration.common.proxyConfiguration),
mobileDevice: MobileDevice(),
sdkInitDate: dateProvider.currentDate(),
dateProvider: dateProvider,
dateCorrector: dateCorrector,
userInfoProvider: userInfoProvider,
networkConnectionInfoProvider: networkConnectionInfoProvider,
carrierInfoProvider: carrierInfoProvider,
launchTimeProvider: launchTimeProvider
launchTimeProvider: launchTimeProvider,
appStateListener: AppStateListener(dateProvider: dateProvider)
)

if let internalMonitoringConfiguration = configuration.internalMonitoring {
Expand Down Expand Up @@ -273,8 +275,7 @@ public class Datadog {
if let urlSessionAutoInstrumentationConfiguration = configuration.urlSessionAutoInstrumentation {
urlSessionAutoInstrumentation = URLSessionAutoInstrumentation(
configuration: urlSessionAutoInstrumentationConfiguration,
dateProvider: dateProvider,
appStateListener: AppStateListener(dateProvider: dateProvider)
commonDependencies: commonDependencies
)
}

Expand Down
4 changes: 4 additions & 0 deletions Sources/Datadog/RUM/RUMFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ internal final class RUMFeature {

// MARK: - Dependencies

let sdkInitDate: Date
let dateProvider: DateProvider
let dateCorrector: DateCorrectorType
let appStateListener: AppStateListening
let userInfoProvider: UserInfoProvider
let networkConnectionInfoProvider: NetworkConnectionInfoProviderType
let carrierInfoProvider: CarrierInfoProviderType
Expand Down Expand Up @@ -167,8 +169,10 @@ internal final class RUMFeature {
self.configuration = configuration

// Bundle dependencies
self.sdkInitDate = commonDependencies.sdkInitDate
self.dateProvider = commonDependencies.dateProvider
self.dateCorrector = commonDependencies.dateCorrector
self.appStateListener = commonDependencies.appStateListener
self.userInfoProvider = commonDependencies.userInfoProvider
self.networkConnectionInfoProvider = commonDependencies.networkConnectionInfoProvider
self.carrierInfoProvider = commonDependencies.carrierInfoProvider
Expand Down
12 changes: 9 additions & 3 deletions Sources/Datadog/RUM/RUMMonitor/Scopes/RUMApplicationScope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ internal typealias RUMSessionListener = (String, Bool) -> Void

/// Injection container for common dependencies used by all `RUMScopes`.
internal struct RUMScopeDependencies {
let appStateListener: AppStateListening
let userInfoProvider: RUMUserInfoProvider
let launchTimeProvider: LaunchTimeProviderType
let connectivityInfoProvider: RUMConnectivityInfoProvider
Expand All @@ -36,6 +37,9 @@ internal class RUMApplicationScope: RUMScope, RUMContextProvider {
/// RUM Sessions sampling rate.
internal let samplingRate: Float

/// The start time of the application, measured in device date. It equals the time of SDK init.
private let applicationStartTime: Date
Copy link
Member

Choose a reason for hiding this comment

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

I think the name can be confused with application launch time. Can we rename it to something like scopeStartTime or sdkInitTime?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, makes sense - for that reason I called it sdkInitTime in upstream, but in RUMMonitor this information was translated to applicationStartTime. What proposed could be simpler 👍 - applied in 99ffde2 to avoid rebasing.


/// Automatically detect background events
internal let backgroundEventTrackingEnabled: Bool

Expand All @@ -47,10 +51,12 @@ internal class RUMApplicationScope: RUMScope, RUMContextProvider {
rumApplicationID: String,
dependencies: RUMScopeDependencies,
samplingRate: Float,
applicationStartTime: Date,
backgroundEventTrackingEnabled: Bool
) {
self.dependencies = dependencies
self.samplingRate = samplingRate
self.applicationStartTime = applicationStartTime
self.backgroundEventTrackingEnabled = backgroundEventTrackingEnabled
self.context = RUMContext(
rumApplicationID: rumApplicationID,
Expand All @@ -70,7 +76,7 @@ internal class RUMApplicationScope: RUMScope, RUMContextProvider {

func process(command: RUMCommand) -> Bool {
if sessionScope == nil {
startInitialSession(on: command)
startInitialSession()
}

if let currentSession = sessionScope {
Expand All @@ -93,13 +99,13 @@ internal class RUMApplicationScope: RUMScope, RUMContextProvider {
_ = refreshedSession.process(command: command)
}

private func startInitialSession(on command: RUMCommand) {
private func startInitialSession() {
let initialSession = RUMSessionScope(
isInitialSession: true,
parent: self,
dependencies: dependencies,
samplingRate: samplingRate,
startTime: command.time,
startTime: applicationStartTime,
backgroundEventTrackingEnabled: backgroundEventTrackingEnabled
)
sessionScope = initialSession
Expand Down
15 changes: 10 additions & 5 deletions Sources/Datadog/RUM/RUMMonitor/Scopes/RUMSessionScope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
let isInitialSession: Bool
/// RUM Session sampling rate.
private let samplingRate: Float
/// The start time of this Session.
/// The start time of this Session, measured in device date. In initial session this is the time of SDK init.
private let sessionStartTime: Date
/// Time of the last RUM interaction noticed by this Session.
private var lastInteractionTime: Date
Expand Down Expand Up @@ -129,9 +129,14 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
// Consider starting an active view, "ApplicationLaunch" view or "Background" view
if let startViewCommand = command as? RUMStartViewCommand {
startView(on: startViewCommand)
} else if isInitialSession && !hasTrackedAnyView && command.canStartApplicationLaunchView {
startApplicationLaunchView(on: command)
} else if backgroundEventTrackingEnabled && !hasActiveView && command.canStartBackgroundView {
} else if isInitialSession && !hasTrackedAnyView { // if initial session with no views history
let appInForeground = dependencies.appStateListener.history.currentState.isActive
if appInForeground && command.canStartApplicationLaunchView { // when app is in foreground, start "ApplicationLaunch" view
startApplicationLaunchView(on: command)
} else if backgroundEventTrackingEnabled && command.canStartBackgroundView { // when app is in background and BET is enabled, start "Background" view
startBackgroundView(on: command)
}
} else if backgroundEventTrackingEnabled && !hasActiveView && command.canStartBackgroundView { // if existing session with views history and BET is enabled
startBackgroundView(on: command)
}

Expand Down Expand Up @@ -186,7 +191,7 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
name: Constants.applicationLaunchViewName,
attributes: command.attributes,
customTimings: [:],
startTime: command.time
startTime: sessionStartTime
)
)
}
Expand Down
9 changes: 7 additions & 2 deletions Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,14 @@ internal class RUMViewScope: RUMScope, RUMContextProvider {
}
case let command as RUMAddUserActionCommand where isActiveView:
if userActionScope == nil {
addDiscreteUserAction(on: command)
if command.actionType == .custom {
// send it instantly without waiting for child events (e.g. resource associated to this action)
sendDiscreteCustomUserAction(on: command)
} else {
addDiscreteUserAction(on: command)
}
} else if command.actionType == .custom {
// still let it go, just instantly without any dependencies
// still let it go, just instantly without waiting for child events (e.g. resource associated to this action)
Comment on lines 173 to +181
Copy link
Member

Choose a reason for hiding this comment

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

/nit this can be:

if command.actionType == .custom {
    // send it instantly without waiting for child events (e.g. resource associated to this action)
    sendDiscreteCustomUserAction(on: command)
} else if userActionScope == nil {
    addDiscreteUserAction(on: command)
} else {
    reportActionDropped(type: command.actionType, name: command.name)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, it's simpler 👍. Applied in 99ffde2 to avoid rebasing.

sendDiscreteCustomUserAction(on: command)
Comment on lines 173 to 182
Copy link
Member Author

Choose a reason for hiding this comment

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

While testing ALT feature, I found this bug in .custom actions tracking. Here, I fix it - now we treat .custom actions the same, no matter if added while any other action scope exists or not. I also added test for this case.

} else {
reportActionDropped(type: command.actionType, name: command.name)
Expand Down
2 changes: 2 additions & 0 deletions Sources/Datadog/RUMMonitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public class RUMMonitor: DDRUMMonitor, RUMCommandSubscriber {
applicationScope: RUMApplicationScope(
rumApplicationID: rumFeature.configuration.applicationID,
dependencies: RUMScopeDependencies(
appStateListener: rumFeature.appStateListener,
userInfoProvider: RUMUserInfoProvider(userInfoProvider: rumFeature.userInfoProvider),
launchTimeProvider: rumFeature.launchTimeProvider,
connectivityInfoProvider: RUMConnectivityInfoProvider(
Expand All @@ -198,6 +199,7 @@ public class RUMMonitor: DDRUMMonitor, RUMCommandSubscriber {
onSessionStart: rumFeature.onSessionStart
),
samplingRate: rumFeature.configuration.sessionSamplingRate,
applicationStartTime: rumFeature.sdkInitDate,
backgroundEventTrackingEnabled: rumFeature.configuration.backgroundEventTrackingEnabled
),
dateProvider: rumFeature.dateProvider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,15 @@ internal final class URLSessionAutoInstrumentation: RUMCommandPublisher {

convenience init?(
configuration: FeaturesConfiguration.URLSessionAutoInstrumentation,
dateProvider: DateProvider,
appStateListener: AppStateListening
commonDependencies: FeaturesCommonDependencies
) {
do {
self.init(
swizzler: try URLSessionSwizzler(),
interceptor: URLSessionInterceptor(
configuration: configuration,
dateProvider: dateProvider,
appStateListener: appStateListener
dateProvider: commonDependencies.dateProvider,
appStateListener: commonDependencies.appStateListener
)
)
} catch {
Expand Down
4 changes: 3 additions & 1 deletion Tests/DatadogBenchmarkTests/BenchmarkMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ extension FeaturesCommonDependencies {
performance: .benchmarksPreset,
httpClient: HTTPClient(),
mobileDevice: MobileDevice(),
sdkInitDate: Date(),
dateProvider: SystemDateProvider(),
dateCorrector: DateCorrectorMock(),
userInfoProvider: UserInfoProvider(),
networkConnectionInfoProvider: NetworkConnectionInfoProvider(),
carrierInfoProvider: CarrierInfoProvider(),
launchTimeProvider: LaunchTimeProvider()
launchTimeProvider: LaunchTimeProvider(),
appStateListener: AppStateListener(dateProvider: SystemDateProvider())
)
}
}
Loading