fix: make WireLogger concurrency safe - WPB-11890 (#2204)

wireapp · Nov 28, 2024 · fbef97f · fbef97f
1 parent 7089658
commit fbef97f
Show file tree

Hide file tree

Showing 17 changed files with 107 additions and 147 deletions.
diff --git a/wire-ios-system/Source/ExpiringActivity.swift b/wire-ios-system/Source/ExpiringActivity.swift
@@ -47,14 +47,14 @@ public func withExpiringActivity(reason: String, block: @escaping () async throw
 
 actor ExpiringActivityManager {
 
-    let api: ExpiringActivityInterface
-    var task: Task<Void, Error>?
+    let api: any ExpiringActivityInterface
+    var task: Task<Void, any Error>?
 
     init() {
         self.init(api: ProcessInfo.processInfo)
     }
 
-    init(api: ExpiringActivityInterface) {
+    init(api: any ExpiringActivityInterface) {
         self.api = api
     }
 
@@ -90,7 +90,7 @@ actor ExpiringActivityManager {
         }
     }
 
-    func startWork(block: @escaping () async throws -> Void, semaphore: DispatchSemaphore) -> Task<Void, Error> {
+    func startWork(block: @escaping () async throws -> Void, semaphore: DispatchSemaphore) -> Task<Void, any Error> {
         let task = Task {
             defer {
                 WireLogger.backgroundActivity.debug("Releasing semaphore")

diff --git a/wire-ios-system/Source/Logging/Flow.swift b/wire-ios-system/Source/Logging/Flow.swift
@@ -63,7 +63,7 @@ open class Flow {
     /// - Parameters:
     ///   - description: A short single line string describing a point of interest.
 
-    public func checkpoint(description: LogConvertible) {
+    public func checkpoint(description: any LogConvertible) {
         logger.info(FlowLog(
             name: name,
             event: .init(type: .checkpoint, description: description.logDescription, outcome: .success)
@@ -81,7 +81,7 @@ open class Flow {
     /// - Parameters:
     ///   - error: The failure reason.
 
-    public func fail(_ error: Error) {
+    public func fail(_ error: any Error) {
         logger.error(FlowLog(
             name: name,
             event: .init(type: .end, description: String(describing: error), outcome: .failure)
@@ -93,7 +93,7 @@ open class Flow {
     /// - Parameters:
     ///   - reason: The failure reason.
 
-    public func fail(_ reason: LogConvertible) {
+    public func fail(_ reason: any LogConvertible) {
         logger.error(FlowLog(
             name: name,
             event: .init(type: .end, description: reason.logDescription, outcome: .failure)

diff --git a/wire-ios-system/Source/Logging/LogAttributes.swift b/wire-ios-system/Source/Logging/LogAttributes.swift
@@ -16,9 +16,9 @@
 // along with this program. If not, see http://www.gnu.org/licenses/.
 //
 
-public typealias LogAttributes = [LogAttributesKey: Encodable]
+public typealias LogAttributes = [LogAttributesKey: any Encodable]
 
-public enum LogAttributesKey: String, Comparable {
+public enum LogAttributesKey: String, Comparable, Sendable {
 
     case selfClientId = "self_client_id"
     case selfUserId = "self_user_id"
@@ -43,5 +43,5 @@ public enum LogAttributesKey: String, Comparable {
 }
 
 public extension LogAttributes {
-    static var safePublic = [LogAttributesKey.public: true]
+    static let safePublic = [LogAttributesKey.public: true]
 }
diff --git a/wire-ios-system/Source/Logging/LoggerProtocol.swift b/wire-ios-system/Source/Logging/LoggerProtocol.swift
@@ -33,7 +33,7 @@ public protocol LoggerProtocol {
 
 extension LoggerProtocol {
 
-    func attributesDescription(from attributes: LogAttributes) -> String {
+    public func attributesDescription(from attributes: LogAttributes) -> String {
         var logAttributes = attributes
 
         // drop attributes used for visibility and category

diff --git a/wire-ios-system/Source/Logging/SystemLogger.swift b/wire-ios-system/Source/Logging/SystemLogger.swift
@@ -23,11 +23,11 @@ public protocol FileLoggerDestination {
     var log: URL? { get }
 }
 
-struct SystemLogger: LoggerProtocol {
+public struct SystemLogger: LoggerProtocol {
 
     let persistQueue = DispatchQueue(label: "persistQueue")
 
-    var logFiles: [URL] {
+    public var logFiles: [URL] {
         []
     }
 
@@ -42,43 +42,45 @@ struct SystemLogger: LoggerProtocol {
         }
     }
 
-    func debug(_ message: any LogConvertible, attributes: LogAttributes...) {
+    public init() {}
+
+    public func debug(_ message: any LogConvertible, attributes: LogAttributes...) {
         log(message, attributes: attributes, osLogType: .debug)
     }
 
-    func info(_ message: any LogConvertible, attributes: LogAttributes...) {
+    public func info(_ message: any LogConvertible, attributes: LogAttributes...) {
         log(message, attributes: attributes, osLogType: .info)
     }
 
-    func notice(_ message: any LogConvertible, attributes: LogAttributes...) {
+    public func notice(_ message: any LogConvertible, attributes: LogAttributes...) {
         log(message, attributes: attributes, osLogType: .default)
     }
 
-    func warn(_ message: any LogConvertible, attributes: LogAttributes...) {
+    public func warn(_ message: any LogConvertible, attributes: LogAttributes...) {
         log(message, attributes: attributes, osLogType: .fault)
     }
 
-    func error(_ message: any LogConvertible, attributes: LogAttributes...) {
+    public func error(_ message: any LogConvertible, attributes: LogAttributes...) {
         log(message, attributes: attributes, osLogType: .error)
     }
 
-    func critical(_ message: any LogConvertible, attributes: LogAttributes...) {
+    public func critical(_ message: any LogConvertible, attributes: LogAttributes...) {
         log(message, attributes: attributes, osLogType: .fault)
     }
 
-    func addTag(_ key: LogAttributesKey, value: String?) {
+    public func addTag(_ key: LogAttributesKey, value: String?) {
         // do nothing, as it's only available on datadog
     }
 
-    private func log(_ message: LogConvertible, attributes: [LogAttributes], osLogType: OSLogType) {
+    private func log(_ message: any LogConvertible, attributes: [LogAttributes], osLogType: OSLogType) {
         var mergedAttributes: LogAttributes = [:]
         attributes.forEach {
             mergedAttributes.merge($0) { _, new in new }
         }
 
         var logger = OSLog.default
         if let tag = mergedAttributes[.tag] as? String {
-            logger = loggers[tag] ?? OSLog(subsystem: Bundle.main.bundleIdentifier ?? "main", category: tag)
+            logger = OSLog(subsystem: Bundle.main.bundleIdentifier ?? "main", category: tag)
         }
 
         let message = "\(message.logDescription)\(attributesDescription(from: mergedAttributes))"
@@ -90,5 +92,3 @@ struct SystemLogger: LoggerProtocol {
         }
     }
 }
-
-private var loggers: [String: OSLog] = [:]
diff --git a/wire-ios-system/Source/Logging/WireLogger.swift b/wire-ios-system/Source/Logging/WireLogger.swift
@@ -16,12 +16,18 @@
 // along with this program. If not, see http://www.gnu.org/licenses/.
 //
 
-public struct WireLogger: LoggerProtocol {
+public struct WireLogger: LoggerProtocol, Sendable {
 
-    private static var provider = AggregatedLogger(loggers: [
-        SystemLogger(),
-        CocoaLumberjackLogger()
-    ])
+    public static func initialize(loggers: [any LoggerProtocol]) {
+        guard provider == nil else {
+            assertionFailure("WireLogger.initialize called more than once")
+            return
+        }
+
+        provider = AggregatedLogger(loggers: loggers)
+    }
+
+    private static nonisolated(unsafe) var provider: (any LoggerProtocol)?
 
     public let tag: String
 
@@ -34,46 +40,46 @@ public struct WireLogger: LoggerProtocol {
     // MARK: - LoggerProtocol
 
     public var logFiles: [URL] {
-        Self.provider.logFiles
+        Self.provider?.logFiles ?? []
     }
 
     public func addTag(_ key: LogAttributesKey, value: String?) {
-        Self.provider.addTag(key, value: value)
+        Self.provider?.addTag(key, value: value)
     }
 
     public func debug(_ message: any LogConvertible, attributes: LogAttributes...) {
         guard shouldLogMessage(message) else { return }
-        Self.provider.debug(message, attributes: finalizedAttributes(attributes))
+        Self.provider?.debug(message, attributes: finalizedAttributes(attributes))
     }
 
     public func info(_ message: any LogConvertible, attributes: LogAttributes...) {
         guard shouldLogMessage(message) else { return }
-        Self.provider.info(message, attributes: finalizedAttributes(attributes))
+        Self.provider?.info(message, attributes: finalizedAttributes(attributes))
     }
 
     public func notice(_ message: any LogConvertible, attributes: LogAttributes...) {
         guard shouldLogMessage(message) else { return }
-        Self.provider.notice(message, attributes: finalizedAttributes(attributes))
+        Self.provider?.notice(message, attributes: finalizedAttributes(attributes))
     }
 
     public func warn(_ message: any LogConvertible, attributes: LogAttributes...) {
         guard shouldLogMessage(message) else { return }
-        Self.provider.warn(message, attributes: finalizedAttributes(attributes))
+        Self.provider?.warn(message, attributes: finalizedAttributes(attributes))
     }
 
     public func error(_ message: any LogConvertible, attributes: LogAttributes...) {
         guard shouldLogMessage(message) else { return }
-        Self.provider.error(message, attributes: finalizedAttributes(attributes))
+        Self.provider?.error(message, attributes: finalizedAttributes(attributes))
     }
 
     public func critical(_ message: any LogConvertible, attributes: LogAttributes...) {
         guard shouldLogMessage(message) else { return }
-        Self.provider.critical(message, attributes: finalizedAttributes(attributes))
+        Self.provider?.critical(message, attributes: finalizedAttributes(attributes))
     }
 
     // MARK: - Private Helpers
 
-    private func shouldLogMessage(_ message: LogConvertible) -> Bool {
+    private func shouldLogMessage(_ message: any LogConvertible) -> Bool {
         !message.logDescription.isEmpty
     }
 
@@ -90,10 +96,6 @@ public struct WireLogger: LoggerProtocol {
     // MARK: Static Functions
 
     public static var logFiles: [URL] {
-        provider.logFiles
-    }
-
-    public static func addLogger(_ logger: LoggerProtocol) {
-        provider.addLogger(logger)
+        provider?.logFiles ?? []
     }
 }
diff --git a/wire-ios-system/Source/Logging/WireLoggerObjc.swift b/wire-ios-system/Source/Logging/WireLoggerObjc.swift
@@ -32,7 +32,7 @@ public final class WireLoggerObjc: NSObject {
     }
 
     @objc(logSaveCoreDataError:)
-    static func logSaveCoreData(error: Error) {
+    static func logSaveCoreData(error: any Error) {
         WireLogger.localStorage.error("Failed to save: \(error)", attributes: .safePublic)
     }
 }
diff --git a/wire-ios-system/Source/ZMSLog.swift b/wire-ios-system/Source/ZMSLog.swift
@@ -47,7 +47,7 @@ public final class ZMSLogEntry: NSObject {
 ///     zmLog.warn("A serious warning!")
 ///
 @objc
-public final class ZMSLog: NSObject {
+public final class ZMSLog: NSObject, Sendable {
 
     public typealias LogHook = (_ level: ZMLogLevel, _ tag: String?, _ message: String) -> Void
     public typealias LogEntryHook = (

diff --git a/wire-ios-system/Tests/ZMLogTests.swift b/wire-ios-system/Tests/ZMLogTests.swift
@@ -20,10 +20,9 @@ import XCTest
 
 @testable import WireSystem
 
-class ZMLogTests: XCTestCase {
+final class ZMLogTests: XCTestCase {
 
     override func setUp() {
-        super.setUp()
         ZMSLog.debug_resetAllLevels()
         ZMSLog.clearLogs()
     }
@@ -32,7 +31,6 @@ class ZMLogTests: XCTestCase {
         ZMSLog.debug_resetAllLevels()
         ZMSLog.stopRecording()
         ZMSLog.removeAllLogHooks()
-        super.tearDown()
     }
 
     func testNumberOfPreviousZipLogURLs() {
@@ -207,6 +205,7 @@ extension ZMLogTests {
 
 extension ZMLogTests {
 
+    @MainActor
     func testThatLogHookIsCalledWithError() {
 
         // GIVEN
@@ -255,6 +254,7 @@ extension ZMLogTests {
         ZMSLog.removeLogHook(token: token)
     }
 
+    @MainActor
     func testThatLogHookIsCalledWithWarning() {
 
         // GIVEN
@@ -304,6 +304,7 @@ extension ZMLogTests {
         ZMSLog.removeLogHook(token: token)
     }
 
+    @MainActor
     func testThatLogHookIsCalledWithDebugIfEnabled() {
 
         // GIVEN
@@ -362,6 +363,7 @@ extension ZMLogTests {
         Thread.sleep(forTimeInterval: 0.2)
     }
 
+    @MainActor
     func testThatCallsMultipleLogHook() {
 
         // GIVEN

diff --git a/wire-ios/Wire Notification Service Extension/NotificationService.swift b/wire-ios/Wire Notification Service Extension/NotificationService.swift
@@ -29,7 +29,7 @@ final class NotificationService: UNNotificationServiceExtension {
 
     override init() {
         super.init()
-        WireAnalytics.Datadog.enable()
+        WireAnalytics.setup()
     }
 
     // MARK: - Methods

diff --git a/wire-ios/Wire-iOS Share Extension/ShareExtensionViewController.swift b/wire-ios/Wire-iOS Share Extension/ShareExtensionViewController.swift
@@ -136,7 +136,7 @@ final class ShareExtensionViewController: SLComposeServiceViewController {
     }
 
     private func setUpDatadog() {
-        WireAnalytics.Datadog.enable()
+        WireAnalytics.setup()
     }
 
     override func viewDidLoad() {

diff --git a/wire-ios/Wire-iOS Tests/WireAnalytics_DatadogTests.swift b/wire-ios/Wire-iOS Tests/WireAnalytics_DatadogTests.swift