Simplify how we setup Sentry to make sure it's configured before any spans get created - they don't get reported otherwise

This commit is contained in:
Stefan Ceriu 2024-07-17 14:47:51 +03:00 committed by Stefan Ceriu
parent 95d53f1edf
commit 65dd6f3496
4 changed files with 73 additions and 81 deletions

View File

@ -96,10 +96,6 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
navigationRootCoordinator = NavigationRootCoordinator()
Self.setupServiceLocator(appSettings: appSettings, appHooks: appHooks)
ServiceLocator.shared.analytics.startIfEnabled()
stateMachine = AppCoordinatorStateMachine()
navigationRootCoordinator.setRootCoordinator(SplashScreenCoordinator())
@ -116,6 +112,12 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
notificationManager = NotificationManager(notificationCenter: UNUserNotificationCenter.current(),
appSettings: appSettings)
Self.setupSentry(appSettings: appSettings)
Self.setupServiceLocator(appSettings: appSettings, appHooks: appHooks)
ServiceLocator.shared.analytics.startIfEnabled()
windowManager.delegate = self
notificationManager.delegate = self
@ -141,14 +143,10 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
registerBackgroundAppRefresh()
ServiceLocator.shared.analytics.isRunningPublisher
.removeDuplicates()
.sink { [weak self] isRunning in
if isRunning {
self?.setupSentry()
} else {
self?.teardownSentry()
}
appSettings.$analyticsConsentState
.dropFirst() // Called above before configuring the ServiceLocator
.sink { _ in
Self.setupSentry(appSettings: appSettings)
}
.store(in: &cancellables)
@ -743,13 +741,20 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
}
}
private func setupSentry() {
SentrySDK.start { [weak self] options in
private static func setupSentry(appSettings: AppSettings) {
let options: Options = .init()
#if DEBUG
options.enabled = false
#else
options.enabled = appSettings.analyticsConsentState == .optedIn
#endif
options.dsn = self?.appSettings.bugReportSentryURL.absoluteString
options.dsn = appSettings.bugReportSentryURL.absoluteString
if AppSettings.isDevelopmentBuild {
options.environment = "development"
}
// Sentry swizzling shows up quite often as the heaviest stack trace when profiling
// We don't need any of the features it powers (see docs)
@ -790,9 +795,10 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
ServiceLocator.shared.bugReportService.lastCrashEventID = event.eventId.sentryIdString
}
SentrySDK.start(options: options)
MXLog.info("SentrySDK started")
}
}
private func teardownSentry() {
SentrySDK.close()

View File

@ -38,19 +38,11 @@ class AnalyticsService {
/// A signpost client for performance testing the app. This client doesn't respect the
/// `isRunning` state or behave any differently when `start`/`reset` are called.
let signpost = Signposter(isDevelopmentBuild: AppSettings.isDevelopmentBuild)
/// Whether or not the object is enabled and sending events to the server.
private let isRunningSubject: CurrentValueSubject<Bool, Never> = .init(false)
var isRunningPublisher: CurrentValuePublisher<Bool, Never> {
isRunningSubject.asCurrentValuePublisher()
}
let signpost = Signposter()
init(client: AnalyticsClientProtocol, appSettings: AppSettings) {
self.client = client
self.appSettings = appSettings
isRunningSubject.send(client.isRunning)
}
/// Whether to show the user the analytics opt in prompt.
@ -77,20 +69,18 @@ class AnalyticsService {
reset()
client.stop()
isRunningSubject.send(false)
MXLog.info("Stopped.")
}
/// Starts the analytics client if the user has opted in, otherwise does nothing.
func startIfEnabled() {
guard isEnabled, !isRunningPublisher.value else { return }
guard isEnabled, !client.isRunning else { return }
client.start(analyticsConfiguration: appSettings.analyticsConfiguration)
// Sanity check in case something went wrong.
guard client.isRunning else { return }
isRunningSubject.send(true)
MXLog.info("Started.")
}

View File

@ -35,7 +35,6 @@ class Signposter {
static let appStarted = "AppStarted"
static let homeserver = "homeserver"
static let isDevelopmentBuild = "isDevelopmentBuild"
}
static let subsystem = "ElementX"
@ -43,9 +42,8 @@ class Signposter {
private var appStartupSpan: Span
init(isDevelopmentBuild: Bool) {
init() {
appStartupSpan = SentrySDK.startTransaction(name: Name.appStartup, operation: Name.appStarted)
appStartupSpan.setData(value: isDevelopmentBuild, key: Name.isDevelopmentBuild)
}
// MARK: - Login
@ -55,7 +53,7 @@ class Signposter {
func beginLogin() {
loginState = signposter.beginInterval(Name.login)
loginSpan = appStartupSpan.startChild(operation: "\(Name.login)")
loginSpan = appStartupSpan.startChild(operation: "\(Name.login)", description: "\(Name.login)")
}
func endLogin() {
@ -80,7 +78,7 @@ class Signposter {
appStartupSpan.setTag(value: serverName, key: Name.homeserver)
firstSyncState = signposter.beginInterval(Name.firstSync)
firstSyncSpan = appStartupSpan.startChild(operation: "\(Name.firstSync)")
firstSyncSpan = appStartupSpan.startChild(operation: "\(Name.firstSync)", description: "\(Name.firstSync)")
}
func endFirstSync() {
@ -100,7 +98,7 @@ class Signposter {
func beginFirstRooms() {
firstRoomsState = signposter.beginInterval(Name.firstRooms)
firstRoomsSpan = appStartupSpan.startChild(operation: "\(Name.firstRooms)")
firstRoomsSpan = appStartupSpan.startChild(operation: "\(Name.firstRooms)", description: "\(Name.firstRooms)")
}
func endFirstRooms() {

View File

@ -76,7 +76,6 @@ class AnalyticsTests: XCTestCase {
// Given a fresh install of the app Analytics should be disabled
XCTAssertEqual(appSettings.analyticsConsentState, .unknown)
XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
XCTAssertFalse(ServiceLocator.shared.analytics.isRunningPublisher.value)
XCTAssertFalse(analyticsClient.startAnalyticsConfigurationCalled)
}
@ -87,7 +86,6 @@ class AnalyticsTests: XCTestCase {
// Then analytics should be disabled
XCTAssertEqual(appSettings.analyticsConsentState, .optedOut)
XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
XCTAssertFalse(ServiceLocator.shared.analytics.isRunningPublisher.value)
XCTAssertFalse(analyticsClient.isRunning)
// Analytics client should have been stopped
XCTAssertTrue(analyticsClient.stopCalled)