Basics: make ObservabilityScope conform to Sendable (#6089)

New `actor HTTPClient` can't take `ObservabilityScope` as an argument to its functions as it's not `Sendable`. This is the only major API difference between `HTTPClient` and `LegacyHTTPClient`, which we'd like to resolve.

Marked `SwiftToolObservabilityHandler.OutputHandler`, `ThreadSafeBox`, `ThreadSafeArrayStore`, and `ThreadSafeKeyValueStore` as `@unchecked Sendable`. Made `ObservabilityMetadata: Sendable` by requiring its keys and values to be sendable. Added additional `Sendable` requirement on `DiagnosticsHandler`.

This allows passing `ObservabilityScope` instances to `HTTPClient/execute`.

Author: MaxDesiatov

Sources/Basics/HTTPClient/HTTPClient.swift

@@ -51,10 +51,12 @@ public actor HTTPClient {
     ///
     /// - Parameters:
     ///   - request: The ``HTTPClientRequest`` to perform.
+    ///   - observabilityScope: the observability scope to emit diagnostics on.
     ///   - progress: A closure to handle response download progress.
     /// - Returns: A response value returned by underlying ``HTTPClient.Implementation``.
     public func execute(
         _ request: Request,
+        observabilityScope: ObservabilityScope? = nil,
         progress: ProgressHandler? = nil
     ) async throws -> Response {
         // merge configuration
@@ -85,16 +87,18 @@ public actor HTTPClient {
             request.headers.add(name: "Authorization", value: authorization)
         }
 
-        return try await executeWithStrategies(request: request, progress: progress, requestNumber: 0)
+        return try await executeWithStrategies(request: request, requestNumber: 0, observabilityScope, progress)
     }
 
     private func executeWithStrategies(
         request: Request,
-        progress: ProgressHandler?,
-        requestNumber: Int
+        requestNumber: Int,
+        _ observabilityScope: ObservabilityScope?,
+        _ progress: ProgressHandler?
     ) async throws -> Response {
         // apply circuit breaker if necessary
         if self.shouldCircuitBreak(request: request) {
+            observabilityScope?.emit(warning: "Circuit breaker triggered for \(request.url)")
             throw HTTPClientError.circuitBreakerTriggered
         }
 
@@ -115,17 +119,19 @@ public actor HTTPClient {
         self.recordErrorIfNecessary(response: response, request: request)
 
         // handle retry strategy
-        if let retryDelayInNanoseconds = self.calculateRetry(
+        if let retryDelay = self.calculateRetry(
             response: response,
             request: request,
             requestNumber: requestNumber
-        )?.nanoseconds() {
+        ), let retryDelayInNanoseconds = retryDelay.nanoseconds() {
+            observabilityScope?.emit(warning: "\(request.url) failed, retrying in \(retryDelay)")
             try await Task.sleep(nanoseconds: UInt64(retryDelayInNanoseconds))
 
             return try await self.executeWithStrategies(
                 request: request,
-                progress: progress,
-                requestNumber: requestNumber + 1
+                requestNumber: requestNumber + 1,
+                observabilityScope,
+                progress
             )
         }
         // check for valid response codes

Sources/Basics/Observability.swift

@@ -36,16 +36,16 @@ public class ObservabilitySystem {
     }
 
     /// Create an ObservabilitySystem with a single diagnostics handler.
-    public convenience init(_ handler: @escaping (ObservabilityScope, Diagnostic) -> Void) {
+    public convenience init(_ handler: @escaping @Sendable (ObservabilityScope, Diagnostic) -> Void) {
         self.init(SingleDiagnosticsHandler(handler))
     }
 
     private struct SingleDiagnosticsHandler: ObservabilityHandlerProvider, DiagnosticsHandler {
-        var diagnosticsHandler: DiagnosticsHandler  { self }
+        var diagnosticsHandler: DiagnosticsHandler { self }
 
-        let underlying: (ObservabilityScope, Diagnostic) -> Void
+        let underlying: @Sendable (ObservabilityScope, Diagnostic) -> Void
 
-        init(_ underlying: @escaping (ObservabilityScope, Diagnostic) -> Void) {
+        init(_ underlying: @escaping @Sendable (ObservabilityScope, Diagnostic) -> Void) {
             self.underlying = underlying
         }
 
@@ -61,12 +61,12 @@ public protocol ObservabilityHandlerProvider {
 
 // MARK: - ObservabilityScope
 
-public final class ObservabilityScope: DiagnosticsEmitterProtocol, CustomStringConvertible {
+public final class ObservabilityScope: DiagnosticsEmitterProtocol, Sendable, CustomStringConvertible {
     public let description: String
     private let parent: ObservabilityScope?
     private let metadata: ObservabilityMetadata?
 
-    private var diagnosticsHandler: DiagnosticsHandlerWrapper
+    private let diagnosticsHandler: DiagnosticsHandlerWrapper
 
     fileprivate init(
         description: String,
@@ -150,7 +150,7 @@ public final class ObservabilityScope: DiagnosticsEmitterProtocol, CustomStringC
 
 // MARK: - Diagnostics
 
-public protocol DiagnosticsHandler {
+public protocol DiagnosticsHandler: Sendable {
     func handleDiagnostic(scope: ObservabilityScope, diagnostic: Diagnostic)
 }
 
@@ -252,7 +252,7 @@ public struct DiagnosticsEmitter: DiagnosticsEmitterProtocol {
     }
 }
 
-public struct Diagnostic: CustomStringConvertible {
+public struct Diagnostic: Sendable, CustomStringConvertible {
     public let severity: Severity
     public let message: String
     public internal (set) var metadata: ObservabilityMetadata?
@@ -324,7 +324,7 @@ public struct Diagnostic: CustomStringConvertible {
         Self(severity: .debug, message: message.description, metadata: metadata)
     }
 
-    public enum Severity: Comparable {
+    public enum Severity: Comparable, Sendable {
         case error
         case warning
         case info
@@ -380,10 +380,10 @@ public struct Diagnostic: CustomStringConvertible {
 // FIXME: we currently require that Value conforms to CustomStringConvertible which sucks
 // ideally Value would conform to Equatable but that has generic requirement
 // luckily, this is about to change so we can clean this up soon
-public struct ObservabilityMetadata: CustomDebugStringConvertible {
+public struct ObservabilityMetadata: Sendable, CustomDebugStringConvertible {
     public typealias Key = ObservabilityMetadataKey
 
-    private var _storage = [AnyKey: Any]()
+    private var _storage = [AnyKey: Sendable]()
 
     public init() {}
 
@@ -414,7 +414,7 @@ public struct ObservabilityMetadata: CustomDebugStringConvertible {
     ///
     /// - Parameter body: The closure to be invoked for each item stored in this `ObservabilityMetadata`,
     /// passing the type-erased key and the associated value.
-    public func forEach(_ body: (AnyKey, Any) throws -> Void) rethrows {
+    public func forEach(_ body: (AnyKey, Sendable) throws -> Void) rethrows {
         try self._storage.forEach { key, value in
             try body(key, value)
         }
@@ -473,7 +473,7 @@ public struct ObservabilityMetadata: CustomDebugStringConvertible {
     }
 
     /// A type-erased `ObservabilityMetadataKey` used when iterating through the `ObservabilityMetadata` using its `forEach` method.
-    public struct AnyKey {
+    public struct AnyKey: Sendable {
         /// The key's type represented erased to an `Any.Type`.
         public let keyType: Any.Type
 
@@ -485,7 +485,7 @@ public struct ObservabilityMetadata: CustomDebugStringConvertible {
 
 public protocol ObservabilityMetadataKey {
     /// The type of value uniquely identified by this key.
-    associatedtype Value
+    associatedtype Value: Sendable
 }
 
 extension ObservabilityMetadata.AnyKey: Hashable {
@@ -569,11 +569,11 @@ extension ObservabilityMetadata {
         }
     }
 
-    private enum LegacyLocationKey: Key {
+    private enum LegacyLocationKey: Key, Sendable {
         typealias Value = DiagnosticLocationWrapper
     }
 
-    public struct DiagnosticLocationWrapper: CustomStringConvertible {
+    public struct DiagnosticLocationWrapper: Sendable, CustomStringConvertible {
         let underlying: DiagnosticLocation
 
         public init (_ underlying: DiagnosticLocation) {
@@ -597,11 +597,11 @@ extension ObservabilityMetadata {
         }
     }
 
-    private enum LegacyDataKey: Key {
+    private enum LegacyDataKey: Key, Sendable {
         typealias Value = DiagnosticDataWrapper
     }
 
-    struct DiagnosticDataWrapper: CustomStringConvertible {
+    struct DiagnosticDataWrapper: Sendable, CustomStringConvertible {
         let underlying: DiagnosticData
 
         public init (_ underlying: DiagnosticData) {

Sources/Basics/SendableTimeInterval.swift

@@ -11,106 +11,9 @@
 //===----------------------------------------------------------------------===//
 
 import enum Dispatch.DispatchTimeInterval
-import struct Foundation.TimeInterval
 
-public enum SendableTimeInterval: Equatable, Sendable {
-    case seconds(Int)
-    case milliseconds(Int)
-    case microseconds(Int)
-    case nanoseconds(Int)
-    case never
+extension DispatchTimeInterval: @unchecked Sendable {}
 
-    public func timeInterval() -> TimeInterval? {
-        switch self {
-        case .seconds(let value):
-            return Double(value)
-        case .milliseconds(let value):
-            return Double(value) / 1000
-        case .microseconds(let value):
-            return Double(value) / 1_000_000
-        case .nanoseconds(let value):
-            return Double(value) / 1_000_000_000
-        default:
-            return nil
-        }
-    }
-
-    public func nanoseconds() -> Int? {
-        switch self {
-        case .seconds(let value):
-            return value.multipliedReportingOverflow(by: 1_000_000_000).partialValue
-        case .milliseconds(let value):
-            return value.multipliedReportingOverflow(by: 1_000_000).partialValue
-        case .microseconds(let value):
-            return value.multipliedReportingOverflow(by: 1000).partialValue
-        case .nanoseconds(let value):
-            return value
-        default:
-            return nil
-        }
-    }
-
-    public func milliseconds() -> Int? {
-        switch self {
-        case .seconds(let value):
-            return value.multipliedReportingOverflow(by: 1000).partialValue
-        case .milliseconds(let value):
-            return value
-        case .microseconds(let value):
-            return Int(Double(value) / 1000)
-        case .nanoseconds(let value):
-            return Int(Double(value) / 1_000_000)
-        default:
-            return nil
-        }
-    }
-
-    public func seconds() -> Int? {
-        switch self {
-        case .seconds(let value):
-            return value
-        case .milliseconds(let value):
-            return Int(Double(value) / 1000)
-        case .microseconds(let value):
-            return Int(Double(value) / 1_000_000)
-        case .nanoseconds(let value):
-            return Int(Double(value) / 1_000_000_000)
-        default:
-            return nil
-        }
-    }
-
-    public var descriptionInSeconds: String {
-        switch self {
-        case .seconds(let value):
-            return "\(value)s"
-        case .milliseconds(let value):
-            return String(format: "%.2f", Double(value)/Double(1000)) + "s"
-        case .microseconds(let value):
-            return String(format: "%.2f", Double(value)/Double(1_000_000)) + "s"
-        case .nanoseconds(let value):
-            return String(format: "%.2f", Double(value)/Double(1_000_000_000)) + "s"
-        case .never:
-            return "n/a"
-        }
-    }
-}
-
-public extension SendableTimeInterval {
-    init(_ interval: DispatchTimeInterval) {
-        switch interval {
-        case let .seconds(s):
-            self = .seconds(s)
-        case let .milliseconds(ms):
-            self = .milliseconds(ms)
-        case let .microseconds(us):
-            self = .microseconds(us)
-        case let .nanoseconds(ns):
-            self = .nanoseconds(ns)
-        case .never:
-            self = .never
-        @unknown default:
-            fatalError("unknown case of `DispatchTimeInterval`")
-        }
-    }
-}
+/// This typealias hides `DispatchTimeInterval` as an implementation detail until we can use `Swift.Duration`, as the
+/// latter requires macOS 13.
+public typealias SendableTimeInterval = DispatchTimeInterval

Sources/CoreCommands/SwiftToolObservabilityHandler.swift

@@ -57,7 +57,7 @@ public struct SwiftToolObservabilityHandler: ObservabilityHandlerProvider {
         self.outputHandler.wait(timeout: timeout)
     }
 
-    struct OutputHandler: DiagnosticsHandler {
+    struct OutputHandler {
         private let logLevel: Diagnostic.Severity
         internal let outputStream: ThreadSafeOutputByteStream
         private let writer: InteractiveWriter
@@ -159,6 +159,9 @@ public struct SwiftToolObservabilityHandler: ObservabilityHandlerProvider {
     }
 }
 
+extension SwiftToolObservabilityHandler.OutputHandler: @unchecked Sendable {}
+extension SwiftToolObservabilityHandler.OutputHandler: DiagnosticsHandler {}
+
 /// This type is used to write on the underlying stream.
 ///
 /// If underlying stream is a not tty, the string will be written in without any

Sources/SPMBuildCore/PluginInvocation.swift

@@ -1009,7 +1009,7 @@ extension ObservabilityMetadata {
     }
 }
 
-public struct FileLocation: Equatable, CustomStringConvertible {
+public struct FileLocation: Equatable, CustomStringConvertible, Sendable {
     public let file: AbsolutePath
     public let line: Int?