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.

Sign up for GitHub

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

Jump to bottom

RUMM-2507 Create an ApplicationLaunch view during session initialization #1160

Merged
merged 11 commits into from
Feb 21, 2023
Merged

RUMM-2507 Create an ApplicationLaunch view during session initialization #1160

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0eca578
RUMM-2507 Create an ApplicationLaunch view during session initialization
fuzzybinary Feb 8, 2023
727fd54
RUMM-2507 Add CHANGELOG item
fuzzybinary Feb 8, 2023
34cfc1e
Fix build issue
fuzzybinary Feb 9, 2023
add1dae
RUMM-2507 Fix Integration Tests
fuzzybinary Feb 10, 2023
40e29cf
RUMM-2507 Switch to using RUMStartApplicationCommand
fuzzybinary Feb 14, 2023
0fbf63d
Update Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift
fuzzybinary Feb 15, 2023
a75a83c
Fix test compilation
fuzzybinary Feb 15, 2023
f1fd808
Minor language / signature change.
fuzzybinary Feb 15, 2023
125b1f6
RUMM-2507 Fix tests from launch timing change
fuzzybinary Feb 15, 2023
1ab7b2e
RUMM-2507 Fix application start time logic for SwiftUI
fuzzybinary Feb 16, 2023
dc406a4
RUMM-2507 Fix crash tests to look for applicaiton start on Applicatio…
fuzzybinary Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

- [IMPROVEMENT] Always create an ApplicationLaunch view on session initialization. See [#1160][]

# 1.15.0 / 23-01-2023

- [BUGFIX] Fix 'Could not allocate memory' after corrupted TLV. See [#1089][] (Thanks [@cltnschlosser][])
Expand Down Expand Up @@ -430,6 +432,7 @@
[#1071]: https://github.com/DataDog/dd-sdk-ios/pull/1071
[#1089]: https://github.com/DataDog/dd-sdk-ios/pull/1089
[#1145]: https://github.com/DataDog/dd-sdk-ios/pull/1145
[#1160]: https://github.com/DataDog/dd-sdk-ios/pull/1160
[@00fa9a]: https://github.com/00FA9A
[@britton-earnin]: https://github.com/Britton-Earnin
[@hengyu]: https://github.com/Hengyu
Expand Down
24 changes: 7 additions & 17 deletions Sources/Datadog/RUM/RUMMonitor/RUMCommand.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,23 @@ internal protocol RUMCommand {
/// 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 }
/// Whether or not this command is considered a user intaraction
var isUserInteraction: Bool { get }
}

internal struct RUMApplicationStartCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
var canStartBackgroundView = false
var isUserInteraction = false
}

// MARK: - RUM View related commands

internal struct RUMStartViewCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, it should start its own view, not the "Background"
let canStartApplicationLaunchView = false // no, it should start its own view, not the "ApplicationLaunch"
let isUserInteraction = true // a new View means there was a navigation, it's considered a User interaction

/// The value holding stable identity of the RUM View.
Expand Down Expand Up @@ -58,7 +62,6 @@ internal struct RUMStopViewCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, we don't expect receiving it without an active view
let canStartApplicationLaunchView = false // no, we don't expect receiving it without an active view
let isUserInteraction = false // a view can be stopped and in most cases should not be considered an interaction (if it's stopped because the user navigate inside the same app, the startView will happen shortly after this)

/// The value holding stable identity of the RUM View.
Expand All @@ -69,7 +72,6 @@ internal struct RUMAddCurrentViewErrorCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = true // yes, we want to track errors in "Background" view
let canStartApplicationLaunchView = true // yes, we want to track errors in "ApplicationLaunch" view
let isUserInteraction = false // an error is not an interactive event

/// The error message.
Expand Down Expand Up @@ -128,7 +130,6 @@ internal struct RUMAddViewTimingCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, it doesn't make sense to start "Background" view on receiving custom timing, as it will be `0ns` timing
let canStartApplicationLaunchView = false // no, it doesn't make sense to start "ApplicationLaunch" view on receiving custom timing, as it will be `0ns` timing
let isUserInteraction = false // a custom view timing is not an interactive event

/// The name of the timing. It will be used as a JSON key, whereas the value will be the timing duration,
Expand Down Expand Up @@ -159,7 +160,6 @@ internal struct RUMStartResourceCommand: RUMResourceCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = true // yes, we want to track resources in "Background" view
let canStartApplicationLaunchView = true // yes, we want to track resources in "ApplicationLaunch" view
let isUserInteraction = false // a resource is not an interactive event

/// Resource url
Expand All @@ -177,7 +177,6 @@ internal struct RUMAddResourceMetricsCommand: RUMResourceCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartResourceCommand`)
let canStartApplicationLaunchView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartResourceCommand`)
let isUserInteraction = false // an error is not an interactive event

/// Resource metrics.
Expand All @@ -189,7 +188,6 @@ internal struct RUMStopResourceCommand: RUMResourceCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartResourceCommand`)
let canStartApplicationLaunchView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartResourceCommand`)
let isUserInteraction = false // a resource is not an interactive event

/// A type of the Resource
Expand All @@ -205,7 +203,6 @@ internal struct RUMStopResourceWithErrorCommand: RUMResourceCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartResourceCommand`)
let canStartApplicationLaunchView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartResourceCommand`)
let isUserInteraction = false // a resource is not an interactive event

/// The error message.
Expand Down Expand Up @@ -279,7 +276,6 @@ internal struct RUMStartUserActionCommand: RUMUserActionCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = true // yes, we want to track actions in "Background" view (e.g. it makes sense for custom actions)
let canStartApplicationLaunchView = true // yes, we want to track actions in "ApplicationLaunch" view (e.g. it makes sense for custom actions)
let isUserInteraction = true // a user action definitely is a User Interacgion

let actionType: RUMUserActionType
Expand All @@ -291,7 +287,6 @@ internal struct RUMStopUserActionCommand: RUMUserActionCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartUserActionCommand`)
let canStartApplicationLaunchView = false // no, we don't expect receiving it without an active view (started earlier on `RUMStartUserActionCommand`)
let isUserInteraction = true // a user action definitely is a User Interacgion

let actionType: RUMUserActionType
Expand All @@ -303,7 +298,6 @@ internal struct RUMAddUserActionCommand: RUMUserActionCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = true // yes, we want to track actions in "Background" view (e.g. it makes sense for custom actions)
let canStartApplicationLaunchView = true // yes, we want to track actions in "ApplicationLaunch" view (e.g. it makes sense for custom actions)
let isUserInteraction = true // a user action definitely is a User Interacgion

let actionType: RUMUserActionType
Expand All @@ -315,7 +309,6 @@ internal struct RUMAddFeatureFlagEvaluationCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = true // yes, we don't want to miss evaluation of flags that may affect background tasks
let canStartApplicationLaunchView = true // yes, we don't want to miss evaluation of feature flags during application launch
let isUserInteraction = false
let name: String
let value: Encodable
Expand All @@ -334,7 +327,6 @@ internal struct RUMAddLongTaskCommand: RUMCommand {
var time: Date
var attributes: [AttributeKey: AttributeValue]
let canStartBackgroundView = false // no, we don't expect receiving long tasks in "Background" view
let canStartApplicationLaunchView = true // yes, we want to track long tasks in "ApplicationLaunch" view (e.g. any hitches before presenting first UI)
let isUserInteraction = false // a long task is not an interactive event

let duration: TimeInterval
Expand All @@ -345,7 +337,6 @@ internal struct RUMAddLongTaskCommand: RUMCommand {
/// RUM Events received from WebView should keep the active session alive, therefore they fire this command to do so. (ref: RUMM-1793)
internal struct RUMKeepSessionAliveCommand: RUMCommand {
let canStartBackgroundView = false
let canStartApplicationLaunchView = false
let isUserInteraction = false
var time: Date
var attributes: [AttributeKey: AttributeValue]
Expand All @@ -355,7 +346,6 @@ internal struct RUMKeepSessionAliveCommand: RUMCommand {

internal struct RUMUpdatePerformanceMetric: RUMCommand {
let canStartBackgroundView = false
let canStartApplicationLaunchView = false
let isUserInteraction = false
let metric: PerformanceMetric
let value: Double
Expand Down
16 changes: 14 additions & 2 deletions Sources/Datadog/RUM/RUMMonitor/Scopes/RUMApplicationScope.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal class RUMApplicationScope: RUMScope, RUMContextProvider {

func process(command: RUMCommand, context: DatadogContext, writer: Writer) -> Bool {
if sessionScope == nil {
startInitialSession(context: context)
startInitialSession(on: command, context: context, writer: writer)
Comment on lines 38 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

It rather seems we're starting initial session (along with the ApplicationLaunch view) on first command received by RUMApplicationScope. It looks different than what we say in PR description (and what we want):

Now, initializing the Datadog SDK with RUM will create the ApplicationLaunch view by default, staring at the process launch time.

No? If that is true, it would make more sense to just send the new command right after application scope init, somewhere here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I think I get it - we always do it (start the AL view), it is just that we await for any other command to do it. Makes sense - if the app starts idle and nothing happens, no session will be started 👍.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my thought yes. It also made things a little bit cleaner as I could just use the context / writer as part of the command I was already recieving.

}

if let currentSession = sessionScope {
Expand All @@ -60,16 +60,28 @@ internal class RUMApplicationScope: RUMScope, RUMContextProvider {
_ = refreshedSession.process(command: command, context: context, writer: writer)
}

private func startInitialSession(context: DatadogContext) {
private func startInitialSession(on command: RUMCommand, context: DatadogContext, writer: Writer) {
let initialSession = RUMSessionScope(
isInitialSession: true,
parent: self,
startTime: context.sdkInitDate,
dependencies: dependencies,
isReplayBeingRecorded: context.srBaggage?.isReplayBeingRecorded
)

sessionScope = initialSession
sessionScopeDidUpdate(initialSession)
if context.applicationStateHistory.currentSnapshot.state != .background {
// Immediately start the ApplicationLaunchView for the new session
_ = sessionScope?.process(
command: RUMApplicationStartCommand(
time: command.time,
attributes: command.attributes
),
context: context,
writer: writer
)
}
}

private func sessionScopeDidUpdate(_ sessionScope: RUMSessionScope) {
Expand Down
40 changes: 24 additions & 16 deletions Sources/Datadog/RUM/RUMMonitor/Scopes/RUMSessionScope.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
return true // discard all events in this session
}

if let startViewCommand = command as? RUMStartViewCommand {
if let startApplicationCommand = command as? RUMApplicationStartCommand {
startApplicationLaunchView(on: startApplicationCommand, context: context, writer: writer)
} else if let startViewCommand = command as? RUMStartViewCommand {
// Start view scope explicitly on receiving "start view" command
startView(on: startViewCommand, context: context)
} else if !hasActiveView {
Expand All @@ -150,8 +152,6 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
)

switch handlingRule {
case .handleInApplicationLaunchView where command.canStartApplicationLaunchView:
startApplicationLaunchView(on: command, context: context)
case .handleInBackgroundView where command.canStartBackgroundView:
startBackgroundView(on: command, context: context)
default:
Expand Down Expand Up @@ -207,20 +207,28 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
)
}

private func startApplicationLaunchView(on command: RUMCommand, context: DatadogContext) {
private func startApplicationLaunchView(on command: RUMApplicationStartCommand, context: DatadogContext, writer: Writer) {
var startTime = sessionStartTime
if context.launchTime?.isActivePrewarm == false,
let processStartTime = context.launchTime?.launchDate {
startTime = processStartTime
}

let scope = RUMViewScope(
isInitialView: true,
parent: self,
dependencies: dependencies,
identity: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL,
path: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL,
name: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewName,
attributes: command.attributes,
customTimings: [:],
startTime: startTime,
serverTimeOffset: context.serverTimeOffset
)

viewScopes.append(
RUMViewScope(
isInitialView: true,
parent: self,
dependencies: dependencies,
identity: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL,
path: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL,
name: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewName,
attributes: command.attributes,
customTimings: [:],
startTime: sessionStartTime,
serverTimeOffset: context.serverTimeOffset
)
scope
)
}

Expand Down
22 changes: 18 additions & 4 deletions Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,28 @@ internal class RUMViewScope: RUMScope, RUMContextProvider {
// Propagate to User Action scope
userActionScope = userActionScope?.scope(byPropagating: command, context: context, writer: writer)

// Send "application start" action if this is the very first view tracked in the app
let hasSentNoViewUpdatesYet = version == 0
if isInitialView, hasSentNoViewUpdatesYet {
sendApplicationStartAction(context: context, writer: writer)
needsViewUpdate = true
}

// Apply side effects
switch command {
// Application Launch
case is RUMApplicationStartCommand:
sendApplicationStartAction(context: context, writer: writer)
if !isInitialView || viewPath != RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL {
DD.telemetry.error(
id: "application-start-error",
message: "An RUMApplicationStartCommand got sent to something other than the ApplicationLaunch view.",
kind: nil,
stack: nil
)
Copy link
Member

Choose a reason for hiding this comment

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

/nit You can use other signature available

Suggested change
DD.telemetry.error(
id: "application-start-error",
message: "An RUMApplicationStartCommand got sent to something other than the ApplicationLaunch view.",
kind: nil,
stack: nil
)
DD.telemetry.error("An RUMApplicationStartCommand got sent to something other than the ApplicationLaunch view.")

The id is inferred using file:line:message.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add the command type as part of the message, I'm afraid something is not super actionable :)

}
// Application Launch also serves as a StartView command for this view
didReceiveStartCommand = true
needsViewUpdate = true

// View commands
case let command as RUMStartViewCommand where identity.equals(command.identity):
if didReceiveStartCommand {
Expand Down Expand Up @@ -346,9 +360,9 @@ internal class RUMViewScope: RUMScope, RUMContextProvider {
// a RUM view on `SwiftUI.View/onAppear`.
//
// In that case, we consider the time between the application
// launch and the first view start as the application loading
// launch and the sdkInitialization as the application loading
// time.
loadingTime = viewStartTime.timeIntervalSince(launchDate).toInt64Nanoseconds
loadingTime = context.sdkInitDate.timeIntervalSince(launchDate).toInt64Nanoseconds
}

let actionEvent = RUMActionEvent(
Expand Down
12 changes: 7 additions & 5 deletions Tests/DatadogIntegrationTests/Scenarios/RUM/RUMManualInstrumentationScenarioTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class RUMManualInstrumentationScenarioTests: IntegrationTests, RUMCommonAsserts
let session = try XCTUnwrap(RUMSessionMatcher.singleSession(from: recordedRUMRequests))
sendCIAppLog(session)

let launchView = try XCTUnwrap(session.applicationLaunchView)
XCTAssertEqual(launchView.actionEvents[0].action.type, .applicationStart)

let view1 = session.viewVisits[0]
XCTAssertEqual(view1.name, "SendRUMFixture1View")
XCTAssertEqual(view1.path, "Example.SendRUMFixture1ViewController")
Expand All @@ -67,13 +70,12 @@ class RUMManualInstrumentationScenarioTests: IntegrationTests, RUMCommonAsserts
"Expected architecture to start with 'x86_64', 'i486' or 'arm'. Got \(architecture)"
)
}
XCTAssertEqual(view1.viewEvents.last?.view.action.count, 2)
XCTAssertEqual(view1.viewEvents.last?.view.action.count, 1)
XCTAssertEqual(view1.viewEvents.last?.view.resource.count, 1)
XCTAssertEqual(view1.viewEvents.last?.view.error.count, 1)
XCTAssertEqual(view1.actionEvents[0].action.type, .applicationStart)
XCTAssertEqual(view1.actionEvents[1].action.type, .tap)
XCTAssertEqual(view1.actionEvents[1].action.resource?.count, 1, "Action should track one successful Resource")
XCTAssertEqual(view1.actionEvents[1].action.error?.count, 1, "Action should track second Resource failure as Error")
XCTAssertEqual(view1.actionEvents[0].action.type, .tap)
XCTAssertEqual(view1.actionEvents[0].action.resource?.count, 1, "Action should track one successful Resource")
XCTAssertEqual(view1.actionEvents[0].action.error?.count, 1, "Action should track second Resource failure as Error")
XCTAssertEqual(view1.resourceEvents[0].resource.url, "https://foo.com/resource/1")
XCTAssertEqual(view1.resourceEvents[0].resource.statusCode, 200)
XCTAssertEqual(view1.resourceEvents[0].resource.type, .image)
Expand Down
12 changes: 8 additions & 4 deletions Tests/DatadogIntegrationTests/Scenarios/RUM/RUMModalViewsScenarioTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ class RUMModalViewsScenarioTests: IntegrationTests, RUMCommonAsserts {
let session = try XCTUnwrap(RUMSessionMatcher.singleSession(from: recordedRUMRequests))
sendCIAppLog(session)

let launchView = try XCTUnwrap(session.applicationLaunchView)
XCTAssertEqual(launchView.actionEvents[0].action.type, .applicationStart)
XCTAssertGreaterThan(launchView.actionEvents[0].action.loadingTime!, 0)

let visits = session.viewVisits
XCTAssertEqual(visits[0].name, "Screen")
XCTAssertEqual(visits[0].path, "Example.RUMMVSViewController")
XCTAssertEqual(visits[0].actionEvents[0].action.type, .applicationStart)
XCTAssertGreaterThan(visits[0].actionEvents[0].action.loadingTime!, 0)
RUMSessionMatcher.assertViewWasEventuallyInactive(visits[0]) // go to modal "Modal"

XCTAssertEqual(visits[1].name, "Modal")
Expand Down Expand Up @@ -139,11 +141,13 @@ class RUMModalViewsScenarioTests: IntegrationTests, RUMCommonAsserts {
let session = try XCTUnwrap(RUMSessionMatcher.singleSession(from: recordedRUMRequests))
sendCIAppLog(session)

let applicationLaunchView = try XCTUnwrap(session.applicationLaunchView)
XCTAssertEqual(applicationLaunchView.actionEvents[0].action.type, .applicationStart)
XCTAssertGreaterThan(applicationLaunchView.actionEvents[0].action.loadingTime!, 0)

let visits = session.viewVisits
XCTAssertEqual(visits[0].name, "Screen")
XCTAssertEqual(visits[0].path, "Example.RUMMVSViewController")
XCTAssertEqual(visits[0].actionEvents[0].action.type, .applicationStart)
XCTAssertGreaterThan(visits[0].actionEvents[0].action.loadingTime!, 0)
RUMSessionMatcher.assertViewWasEventuallyInactive(visits[0]) // go to modal "Modal"

XCTAssertEqual(visits[1].name, "Modal")
Expand Down
Loading