-
Notifications
You must be signed in to change notification settings - Fork 134
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-2606 Launch Time Publisher #1030
Changes from 4 commits
60a8aff
fbc11e4
f4d4fef
3be6dc8
e747497
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* | ||
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. | ||
* This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
* Copyright 2019-2020 Datadog, Inc. | ||
*/ | ||
|
||
import Foundation | ||
|
||
#if SPM_BUILD | ||
import _Datadog_Private | ||
#endif | ||
|
||
internal struct LaunchTimePublisher: ContextValuePublisher { | ||
private typealias AppLaunchHandler = __dd_private_AppLaunchHandler | ||
|
||
let initialValue: LaunchTime? | ||
|
||
init() { | ||
initialValue = LaunchTime( | ||
launchTime: AppLaunchHandler.shared.launchTime?.doubleValue, | ||
launchDate: AppLaunchHandler.shared.launchDate, | ||
isActivePrewarm: AppLaunchHandler.shared.isActivePrewarm | ||
) | ||
} | ||
|
||
func publish(to receiver: @escaping ContextValueReceiver<LaunchTime?>) { | ||
let launchDate = AppLaunchHandler.shared.launchDate | ||
let isActivePrewarm = AppLaunchHandler.shared.isActivePrewarm | ||
|
||
AppLaunchHandler.shared.setApplicationDidBecomeActiveCallback { launchTime in | ||
let value = LaunchTime( | ||
launchTime: launchTime, | ||
launchDate: launchDate, | ||
isActivePrewarm: isActivePrewarm | ||
) | ||
receiver(value) | ||
} | ||
} | ||
|
||
func cancel() { | ||
AppLaunchHandler.shared.setApplicationDidBecomeActiveCallback { _ in } | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -314,13 +314,23 @@ internal class RUMViewScope: RUMScope, RUMContextProvider { | |
var attributes = self.attributes | ||
var loadingTime: Int64? = nil | ||
|
||
if context.launchTime.isActivePrewarm { | ||
if context.launchTime?.isActivePrewarm == true { | ||
// Set `active_pre_warm` attribute to true in case | ||
// of pre-warmed app | ||
// of pre-warmed app. | ||
attributes[Constants.activePrewarm] = true | ||
} else { | ||
} else if let launchTime = context.launchTime?.launchTime { | ||
// Report Application Launch Time only if not pre-warmed | ||
loadingTime = context.launchTime.launchTime.toInt64Nanoseconds | ||
loadingTime = launchTime.toInt64Nanoseconds | ||
} else if let launchDate = context.launchTime?.launchDate { | ||
// The launchTime can be `nil` if the application is not yet | ||
// active (UIApplicationDidBecomeActiveNotification). That is | ||
// the case when instrumenting a SwiftUI application that start | ||
maxep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// 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 | ||
// time. | ||
loadingTime = viewStartTime.timeIntervalSince(launchDate).toInt64Nanoseconds | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we introduce a regression now? Previously there was no unhandled control flow: // before:
} else {
loadingTime = context.launchTime.launchTime.toInt64Nanoseconds
} and now there is: // now:
} else if let launchDate = context.launchTime.launchDate {
// ...
} // <--- unhandled There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems that non-optional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, now it describes much better with the |
||
|
||
let actionEvent = RUMActionEvent( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the
nil
value is quite unexpected and will be there only for a short while when SDK is being warmed up (this is what we mean by "evaluated"), right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. In practice, this value will be evaluated when we subscribe to the publisher, it will be done when initialising the core.