feat(POM-209): improve logging system (#109)

* Include additional headers with every request: device info, installation id
and application version etc
* Fix connector logging category
* Support sending logs to backend (disabled for now)
* Lower logging levels where appropriate (mostly expected errors to info)
* Support logging additional metadata alongside every log message

Sources/ProcessOut/Sources/Api/Builders/ProcessOutHttpConnectorBuilder.swift

@@ -10,33 +10,30 @@ import Foundation
 /// Builds http connector suitable for communications with ProcessOut API.
 final class ProcessOutHttpConnectorBuilder {
 
-    func with(configuration: HttpConnectorRequestMapperConfiguration) -> Self {
-        self.configuration = configuration
-        return self
-    }
+    /// Connector configuration.
+    var configuration: HttpConnectorRequestMapperConfiguration?
 
-    func with(sessionConfiguration: URLSessionConfiguration) -> Self {
-        self.sessionConfiguration = sessionConfiguration
-        return self
-    }
+    /// Retry strategy to use for failing requests.
+    var retryStrategy: RetryStrategy? = .exponential(maximumRetries: 3, interval: 0.1, rate: 3)
 
-    func with(retryStrategy: RetryStrategy?) -> Self {
-        self.retryStrategy = retryStrategy
-        return self
-    }
+    /// Logger.
+    var logger: POLogger?
 
-    func with(deviceMetadataProvider: DeviceMetadataProvider) -> Self {
-        self.deviceMetadataProvider = deviceMetadataProvider
-        return self
-    }
+    /// Device metadata provider.
+    var deviceMetadataProvider: DeviceMetadataProvider?
 
-    func with(logger: POLogger) -> Self {
-        self.logger = logger
-        return self
-    }
+    /// Session configuration.
+    lazy var sessionConfiguration: URLSessionConfiguration = {
+        let configuration = URLSessionConfiguration.default
+        configuration.urlCache = nil
+        configuration.requestCachePolicy = .reloadIgnoringLocalCacheData
+        configuration.waitsForConnectivity = true
+        configuration.timeoutIntervalForRequest = Constants.requestTimeout
+        return configuration
+    }()
 
     func build() -> HttpConnector {
-        guard let configuration, let logger else {
+        guard let configuration, let logger, let deviceMetadataProvider else {
             fatalError("Unable to create connector without required parameters set.")
         }
         let requestMapper = DefaultHttpConnectorRequestMapper(
@@ -66,30 +63,6 @@ final class ProcessOutHttpConnectorBuilder {
 
     // MARK: - Private Properties
 
-    /// Connector configuration.
-    private var configuration: HttpConnectorRequestMapperConfiguration?
-
-    /// Retry strategy to use for failing requests.
-    private var retryStrategy: RetryStrategy? = .exponential(maximumRetries: 3, interval: 0.1, rate: 3)
-
-    /// Logger.
-    private var logger: POLogger?
-
-    /// Session configuration.
-    private lazy var sessionConfiguration: URLSessionConfiguration = {
-        let configuration = URLSessionConfiguration.default
-        configuration.urlCache = nil
-        configuration.requestCachePolicy = .reloadIgnoringLocalCacheData
-        configuration.waitsForConnectivity = true
-        configuration.timeoutIntervalForRequest = Constants.requestTimeout
-        return configuration
-    }()
-
-    /// Device metadata provider.
-    private lazy var deviceMetadataProvider: DeviceMetadataProvider = {
-        DefaultDeviceMetadataProvider(screen: .main, bundle: .main)
-    }()
-
     private lazy var dateFormatter: DateFormatter = {
         let dateFormatter = DateFormatter()
         dateFormatter.dateFormat = Constants.dateFormat
@@ -111,3 +84,31 @@ final class ProcessOutHttpConnectorBuilder {
         return encoder
     }()
 }
+
+extension ProcessOutHttpConnectorBuilder {
+
+    func with(configuration: HttpConnectorRequestMapperConfiguration) -> Self {
+        self.configuration = configuration
+        return self
+    }
+
+    func with(sessionConfiguration: URLSessionConfiguration) -> Self {
+        self.sessionConfiguration = sessionConfiguration
+        return self
+    }
+
+    func with(retryStrategy: RetryStrategy?) -> Self {
+        self.retryStrategy = retryStrategy
+        return self
+    }
+
+    func with(deviceMetadataProvider: DeviceMetadataProvider) -> Self {
+        self.deviceMetadataProvider = deviceMetadataProvider
+        return self
+    }
+
+    func with(logger: POLogger) -> Self {
+        self.logger = logger
+        return self
+    }
+}

Sources/ProcessOut/Sources/Api/Models/ProcessOutConfiguration.swift

@@ -11,13 +11,17 @@ import Foundation
 public typealias ProcessOutApiConfiguration = ProcessOutConfiguration
 
 /// Defines configuration parameters that are used to create API singleton. In order to create instance
-/// of this structure one should use ``ProcessOutConfiguration/production(projectId:isDebug:)``
+/// of this structure one should use ``ProcessOutConfiguration/production(projectId:appVersion:isDebug:)``
 /// method.
 public struct ProcessOutConfiguration {
 
     /// Project id.
     public let projectId: String
 
+    /// Host application version. Providing this value helps ProcessOut to troubleshoot potential
+    /// issues.
+    public let appVersion: String?
+
     /// Boolean value that indicates whether SDK should operate in debug mode. At this moment it
     /// only affects logging level.
     /// - NOTE: Debug logs may contain sensitive data.
@@ -39,13 +43,14 @@ public struct ProcessOutConfiguration {
 extension ProcessOutConfiguration {
 
     /// Creates production configuration.
-    public static func production(projectId: String, isDebug: Bool = false) -> Self {
+    public static func production(projectId: String, appVersion: String? = nil, isDebug: Bool = false) -> Self {
         // swiftlint:disable force_unwrapping
         let apiBaseUrl = URL(string: "https://api.processout.com")!
         let checkoutBaseUrl = URL(string: "https://checkout.processout.com")!
         // swiftlint:enable force_unwrapping
         return ProcessOutConfiguration(
             projectId: projectId,
+            appVersion: appVersion,
             isDebug: isDebug,
             privateKey: nil,
             apiBaseUrl: apiBaseUrl,
@@ -58,6 +63,7 @@ extension ProcessOutConfiguration {
     public static func test(projectId: String, privateKey: String?, apiBaseUrl: URL, checkoutBaseUrl: URL) -> Self {
         ProcessOutConfiguration(
             projectId: projectId,
+            appVersion: nil,
             isDebug: true,
             privateKey: privateKey,
             apiBaseUrl: apiBaseUrl,

Sources/ProcessOut/Sources/Api/ProcessOut.swift

@@ -105,7 +105,7 @@ public final class ProcessOut {
         static let serviceLoggerCategory = "Service"
         static let repositoryLoggerCategory = "Repository"
         static let connectorLoggerCategory = "Connector"
-        static let systemLoggerSubsystem = "com.processout.processout-ios"
+        static let bundleIdentifier = "com.processout.processout-ios"
     }
 
     // MARK: - Private Properties
@@ -114,15 +114,25 @@ public final class ProcessOut {
     private lazy var repositoryLogger = createLogger(for: Constants.repositoryLoggerCategory)
 
     private lazy var httpConnector: HttpConnector = {
-        let connectorConfiguration = HttpConnectorRequestMapperConfiguration(
+        let configuration = HttpConnectorRequestMapperConfiguration(
             baseUrl: configuration.apiBaseUrl,
             projectId: configuration.projectId,
             privateKey: configuration.privateKey,
-            version: ProcessOut.version
+            version: ProcessOut.version,
+            appVersion: configuration.appVersion
         )
+        let keychain = Keychain(service: Constants.bundleIdentifier)
+        let deviceMetadataProvider = DefaultDeviceMetadataProvider(
+            screen: .main, device: .current, bundle: .main, keychain: keychain
+        )
+        // Connector logs are not sent to backend to avoid recursion. This
+        // may be not ideal because we may loose important events, such
+        // as decoding failures so approach may be reconsidered in future.
+        let logger = createLogger(for: Constants.connectorLoggerCategory, includeRemoteDestination: false)
         let connector = ProcessOutHttpConnectorBuilder()
-            .with(configuration: connectorConfiguration)
+            .with(configuration: configuration)
             .with(logger: logger)
+            .with(deviceMetadataProvider: deviceMetadataProvider)
             .build()
         return connector
     }()
@@ -144,12 +154,18 @@ public final class ProcessOut {
         self.configuration = configuration
     }
 
-    private func createLogger(for category: String) -> POLogger {
+    private func createLogger(for category: String, includeRemoteDestination: Bool = true) -> POLogger {
         let destinations: [LoggerDestination] = [
-            SystemLoggerDestination(subsystem: Constants.systemLoggerSubsystem, category: category)
+            SystemLoggerDestination(subsystem: Constants.bundleIdentifier)
         ]
+        // todo(andrii-vysotskyi): uncomment code bellow when backend will support accepting SDK logs.
+        // if includeRemoteDestination {
+        //     let repository = HttpLogsRepository(connector: httpConnector)
+        //     let service = DefaultLogsService(repository: repository, minimumLevel: .error)
+        //     destinations.append(service)
+        // }
         let minimumLevel: LogLevel = configuration.isDebug ? .debug : .info
-        return POLogger(destinations: destinations, minimumLevel: minimumLevel)
+        return POLogger(destinations: destinations, category: category, minimumLevel: minimumLevel)
     }
 
     private func prewarm() {

Sources/ProcessOut/Sources/Connectors/Http/Implementations/UrlSession/RequestMapper/DefaultHttpConnectorRequestMapper.swift

@@ -6,7 +6,6 @@
 //
 
 import Foundation
-import UIKit.UIDevice
 
 final class DefaultHttpConnectorRequestMapper: HttpConnectorRequestMapper {
 
@@ -38,14 +37,7 @@ final class DefaultHttpConnectorRequestMapper: HttpConnectorRequestMapper {
         if let encodedBody = try encodedRequestBody(request) {
             sessionRequest.httpBody = encodedBody
         }
-        let defaultHeaders = [
-            "Idempotency-Key": request.id,
-            "User-Agent": userAgent,
-            "Accept-Language": Strings.preferredLocalization,
-            "Content-Type": "application/json",
-            "Authorization": try authorization(request: request)
-        ]
-        defaultHeaders.forEach { field, value in
+        defaultHeaders(for: request).forEach { field, value in
             sessionRequest.setValue(value, forHTTPHeaderField: field)
         }
         request.headers.forEach { field, value in
@@ -61,18 +53,7 @@ final class DefaultHttpConnectorRequestMapper: HttpConnectorRequestMapper {
     private let deviceMetadataProvider: DeviceMetadataProvider
     private let logger: POLogger
 
-    private var userAgent: String {
-        let components = [
-            UIDevice.current.systemName,
-            "Version",
-            UIDevice.current.systemVersion,
-            "ProcessOut iOS-Bindings",
-            configuration.version
-        ]
-        return components.joined(separator: "/")
-    }
-
-    // MARK: - Private Methods
+    // MARK: - Request Body Encoding
 
     private func encodedRequestBody(_ request: HttpConnectorRequest<some Decodable>) throws -> Data? {
         let decoratedBody: Encodable?
@@ -92,18 +73,48 @@ final class DefaultHttpConnectorRequestMapper: HttpConnectorRequestMapper {
         }
     }
 
-    private func authorization(request: HttpConnectorRequest<some Decodable>) throws -> String {
+    // MARK: - Request Headers
+
+    private func authorization(request: HttpConnectorRequest<some Decodable>) -> String {
         var value = configuration.projectId + ":"
         if request.requiresPrivateKey {
             if let privateKey = configuration.privateKey {
                 value += privateKey
             } else {
-                logger.info("Private key is required by '\(request.id)' request but not set")
-                throw HttpConnectorFailure.internal
+                preconditionFailure("Private key is required by '\(request.id)' request but not set")
             }
         }
         return "Basic " + Data(value.utf8).base64EncodedString()
     }
+
+    private func defaultHeaders(for request: HttpConnectorRequest<some Decodable>) -> [String: String] {
+        let deviceMetadata = deviceMetadataProvider.deviceMetadata
+        let headers = [
+            "Idempotency-Key": request.id,
+            "User-Agent": userAgent(deviceMetadata: deviceMetadata),
+            "Accept-Language": Strings.preferredLocalization,
+            "Content-Type": "application/json",
+            "Authorization": authorization(request: request),
+            "Installation-Id": deviceMetadata.installationId,
+            "Device-Id": deviceMetadata.id,
+            "Device-System-Name": deviceMetadata.channel,
+            "Device-System-Version": deviceMetadata.systemVersion,
+            "Product-Version": configuration.version,
+            "Host-Application-Version": configuration.appVersion
+        ]
+        return headers.compactMapValues { $0 }
+    }
+
+    private func userAgent(deviceMetadata: DeviceMetadata) -> String {
+        let components = [
+            deviceMetadata.channel,
+            "Version",
+            deviceMetadata.systemVersion,
+            "ProcessOut iOS-Bindings",
+            configuration.version
+        ]
+        return components.joined(separator: "/")
+    }
 }
 
 /// Helps avoid using `JSONSerialization` to encode additional device metadata in request body.

Sources/ProcessOut/Sources/Connectors/Http/Implementations/UrlSession/RequestMapper/HttpConnectorRequestMapperConfiguration.swift

@@ -20,4 +20,7 @@ struct HttpConnectorRequestMapperConfiguration {
 
     /// SDK version.
     let version: String
+
+    /// Host application version.
+    let appVersion: String?
 }

Sources/ProcessOut/Sources/Connectors/Http/Implementations/UrlSession/UrlSessionHttpConnector.swift

@@ -71,7 +71,7 @@ final class UrlSessionHttpConnector: HttpConnector {
         _ valueType: Value.Type, from data: Data?, response: URLResponse?, error: Error?, requestId: String
     ) throws -> Value {
         if let error {
-            logger.debug("Request \(requestId) did fail with error: '\(error.localizedDescription)'.")
+            logger.info("Request \(requestId) did fail with error: '\(error.localizedDescription)'.")
             throw convertToFailure(urlError: error)
         }
         guard let data, let response = response as? HTTPURLResponse else {

Sources/ProcessOut/Sources/Core/DeviceMetadata/DefaultDeviceMetadataProvider.swift

@@ -9,27 +9,47 @@ import UIKit
 
 final class DefaultDeviceMetadataProvider: DeviceMetadataProvider {
 
-    init(screen: UIScreen, bundle: Bundle) {
+    init(screen: UIScreen, device: UIDevice, bundle: Bundle, keychain: Keychain) {
         self.screen = screen
+        self.device = device
         self.bundle = bundle
-        timeZone = .autoupdatingCurrent
+        self.keychain = keychain
     }
 
     // MARK: - DeviceMetadataProvider
 
     var deviceMetadata: DeviceMetadata {
         DeviceMetadata(
+            id: .init(value: deviceId),
+            installationId: .init(value: device.identifierForVendor?.uuidString),
+            systemVersion: .init(value: device.systemVersion),
             appLanguage: bundle.preferredLocalizations.first!, // swiftlint:disable:this force_unwrapping
             appScreenWidth: Int(screen.nativeBounds.width), // Specified in pixels
             appScreenHeight: Int(screen.nativeBounds.height),
-            appTimeZoneOffset: timeZone.secondsFromGMT() / 60,
-            channel: "ios"
+            appTimeZoneOffset: TimeZone.current.secondsFromGMT() / 60,
+            channel: device.systemName.lowercased()
         )
     }
 
+    // MARK: - Private Nested Types
+
+    private enum Constants {
+        static let keychainDeviceId = "DeviceId"
+    }
+
     // MARK: - Private Properties
 
     private let screen: UIScreen
+    private let device: UIDevice
     private let bundle: Bundle
-    private let timeZone: TimeZone
+    private let keychain: Keychain
+
+    private lazy var deviceId: String? = {
+        if let deviceId = keychain.genericPassword(forAccount: Constants.keychainDeviceId) {
+            return deviceId
+        }
+        let deviceId = UUID().uuidString
+        keychain.add(genericPassword: deviceId, account: Constants.keychainDeviceId)
+        return deviceId
+    }()
 }