Simplify how we setup Sentry to make sure it's configured before any …

…spans get created - they don't get reported otherwise
element-hq · Jul 17, 2024 · aafe2d5 · aafe2d5
1 parent b3e377e
commit aafe2d5
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 82 deletions.
diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift
@@ -95,10 +95,6 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
 
         navigationRootCoordinator = NavigationRootCoordinator()
 
-        Self.setupServiceLocator(appSettings: appSettings, appHooks: appHooks)
-
-        ServiceLocator.shared.analytics.startIfEnabled()
-
         stateMachine = AppCoordinatorStateMachine()
 
         navigationRootCoordinator.setRootCoordinator(SplashScreenCoordinator())
@@ -115,6 +111,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
@@ -140,14 +142,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)
 
@@ -734,55 +732,63 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         }
     }
 
-    private func setupSentry() {
-        SentrySDK.start { [weak self] options in
-            #if DEBUG
-            options.enabled = false
-            #endif
-
-            options.dsn = self?.appSettings.bugReportSentryURL.absoluteString
-
-            // 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)
-            options.enableSwizzling = false
-
-            // WatchdogTermination is currently the top issue but we've had zero complaints
-            // so it might very well just all be false positives
-            options.enableWatchdogTerminationTracking = false
-
-            // Disabled as it seems to report a lot of false positives
-            options.enableAppHangTracking = false
-
-            // Most of the network requests are made Rust side, this is useless
-            options.enableNetworkBreadcrumbs = false
-
-            // Doesn't seem to work at all well with SwiftUI
-            options.enableAutoBreadcrumbTracking = false
-
-            // Experimental. Stitches stack traces of asynchronous code together
-            options.swiftAsyncStacktraces = true
-
-            // Uniform sample rate: 1.0 captures 100% of transactions
-            // In Production you will probably want a smaller number such as 0.5 for 50%
-            if AppSettings.isDevelopmentBuild {
-                options.sampleRate = 1.0
-                options.tracesSampleRate = 1.0
-                options.profilesSampleRate = 1.0
-            } else {
-                options.sampleRate = 0.5
-                options.tracesSampleRate = 0.5
-                options.profilesSampleRate = 0.5
-            }
-
-            // This callback is only executed once during the entire run of the program to avoid
-            // multiple callbacks if there are multiple crash events to send (see method documentation)
-            options.onCrashedLastRun = { event in
-                MXLog.error("Sentry detected a crash in the previous run: \(event.eventId.sentryIdString)")
-                ServiceLocator.shared.bugReportService.lastCrashEventID = event.eventId.sentryIdString
-            }
-
-            MXLog.info("SentrySDK started")
+    private static func setupSentry(appSettings: AppSettings) {
+        let options: Options = .init()
+
+        #if DEBUG
+        options.enabled = false
+        #else
+        options.enabled = appSettings.analyticsConsentState == .optedIn
+        #endif
+
+        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)
+        options.enableSwizzling = false
+
+        // WatchdogTermination is currently the top issue but we've had zero complaints
+        // so it might very well just all be false positives
+        options.enableWatchdogTerminationTracking = false
+
+        // Disabled as it seems to report a lot of false positives
+        options.enableAppHangTracking = false
+
+        // Most of the network requests are made Rust side, this is useless
+        options.enableNetworkBreadcrumbs = false
+
+        // Doesn't seem to work at all well with SwiftUI
+        options.enableAutoBreadcrumbTracking = false
+
+        // Experimental. Stitches stack traces of asynchronous code together
+        options.swiftAsyncStacktraces = true
+
+        // Uniform sample rate: 1.0 captures 100% of transactions
+        // In Production you will probably want a smaller number such as 0.5 for 50%
+        if AppSettings.isDevelopmentBuild {
+            options.sampleRate = 1.0
+            options.tracesSampleRate = 1.0
+            options.profilesSampleRate = 1.0
+        } else {
+            options.sampleRate = 0.5
+            options.tracesSampleRate = 0.5
+            options.profilesSampleRate = 0.5
+        }
+
+        // This callback is only executed once during the entire run of the program to avoid
+        // multiple callbacks if there are multiple crash events to send (see method documentation)
+        options.onCrashedLastRun = { event in
+            MXLog.error("Sentry detected a crash in the previous run: \(event.eventId.sentryIdString)")
+            ServiceLocator.shared.bugReportService.lastCrashEventID = event.eventId.sentryIdString
         }
+
+        SentrySDK.start(options: options)
+
+        MXLog.info("SentrySDK started")
     }
 
     private func teardownSentry() {

diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.2.4 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.2.5 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 
 // swiftlint:disable all

diff --git a/ElementX/Sources/Services/Analytics/AnalyticsService.swift b/ElementX/Sources/Services/Analytics/AnalyticsService.swift
@@ -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)
+    let signpost = Signposter()
 
-    /// 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()
-    }
-
     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.
@@ -76,21 +68,19 @@ class AnalyticsService {
         // The order is important here. PostHog ignores the reset if stopped.
         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.")
     }
 

diff --git a/ElementX/Sources/Services/Analytics/Signposter.swift b/ElementX/Sources/Services/Analytics/Signposter.swift
@@ -35,17 +35,15 @@ class Signposter {
         static let appStarted = "AppStarted"
 
         static let homeserver = "homeserver"
-        static let isDevelopmentBuild = "isDevelopmentBuild"
     }
 
     static let subsystem = "ElementX"
     static let category = "PerformanceTests"
 
     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() {

diff --git a/UnitTests/Sources/AnalyticsTests.swift b/UnitTests/Sources/AnalyticsTests.swift
@@ -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)