adopt sendable (#109)

motivation: adjust to swift 5.6

changes:
* define sendable shims for protocols and structs that may be used in async context
* refactor Gauge to include rather than inherit Recorder
* adjust tests
* add a test to make sure no warning are emitted

Sources/CoreMetrics/Metrics.swift

@@ -12,7 +12,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-// MARK: User API
+// MARK: - User API
+
+// MARK: - Counter
 
 extension Counter {
     /// Create a new `Counter`.
@@ -39,9 +41,9 @@ extension Counter {
 /// This is the user-facing Counter API.
 ///
 /// Its behavior depends on the `CounterHandler` implementation.
-public class Counter {
+public final class Counter {
     @usableFromInline
-    var handler: CounterHandler
+    let handler: CounterHandler
     public let label: String
     public let dimensions: [(String, String)]
 
@@ -90,6 +92,8 @@ extension Counter: CustomStringConvertible {
     }
 }
 
+// MARK: - FloatingPointCounter
+
 extension FloatingPointCounter {
     /// Create a new `FloatingPointCounter`.
     ///
@@ -116,9 +120,9 @@ extension FloatingPointCounter {
 /// This is the user-facing FloatingPointCounter API.
 ///
 /// Its behavior depends on the `FloatingCounterHandler` implementation.
-public class FloatingPointCounter {
+public final class FloatingPointCounter {
     @usableFromInline
-    var handler: FloatingPointCounterHandler
+    let handler: FloatingPointCounterHandler
     public let label: String
     public let dimensions: [(String, String)]
 
@@ -167,13 +171,15 @@ extension FloatingPointCounter: CustomStringConvertible {
     }
 }
 
-public extension Recorder {
+// MARK: - Recorder
+
+extension Recorder {
     /// Create a new `Recorder`.
     ///
     /// - parameters:
     ///     - label: The label for the `Recorder`.
     ///     - dimensions: The dimensions for the `Recorder`.
-    convenience init(label: String, dimensions: [(String, String)] = [], aggregate: Bool = true) {
+    public convenience init(label: String, dimensions: [(String, String)] = [], aggregate: Bool = true) {
         let handler = MetricsSystem.factory.makeRecorder(label: label, dimensions: dimensions, aggregate: aggregate)
         self.init(label: label, dimensions: dimensions, aggregate: aggregate, handler: handler)
     }
@@ -181,7 +187,7 @@ public extension Recorder {
     /// Signal the underlying metrics library that this recorder will never be updated again.
     /// In response the library MAY decide to eagerly release any resources held by this `Recorder`.
     @inlinable
-    func destroy() {
+    public func destroy() {
         MetricsSystem.factory.destroyRecorder(self.handler)
     }
 }
@@ -193,7 +199,7 @@ public extension Recorder {
 /// Its behavior depends on the `RecorderHandler` implementation.
 public class Recorder {
     @usableFromInline
-    var handler: RecorderHandler
+    let handler: RecorderHandler
     public let label: String
     public let dimensions: [(String, String)]
     public let aggregate: Bool
@@ -247,10 +253,12 @@ extension Recorder: CustomStringConvertible {
     }
 }
 
+// MARK: - Gauge
+
 /// A gauge is a metric that represents a single numerical value that can arbitrarily go up and down.
 /// Gauges are typically used for measured values like temperatures or current memory usage, but also "counts" that can go up and down, like the number of active threads.
 /// Gauges are modeled as `Recorder` with a sample size of 1 and that does not perform any aggregation.
-public class Gauge: Recorder {
+public final class Gauge: Recorder {
     /// Create a new `Gauge`.
     ///
     /// - parameters:
@@ -261,6 +269,8 @@ public class Gauge: Recorder {
     }
 }
 
+// MARK: - Timer
+
 public struct TimeUnit: Equatable {
     private enum Code: Equatable {
         case nanoseconds
@@ -328,9 +338,9 @@ public extension Timer {
 /// This is the user-facing Timer API.
 ///
 /// Its behavior depends on the `TimerHandler` implementation.
-public class Timer {
+public final class Timer {
     @usableFromInline
-    var handler: TimerHandler
+    let handler: TimerHandler
     public let label: String
     public let dimensions: [(String, String)]
 
@@ -451,6 +461,8 @@ extension Timer: CustomStringConvertible {
     }
 }
 
+// MARK: - MetricsSystem
+
 /// The `MetricsSystem` is a global facility where the default metrics backend implementation (`MetricsFactory`) can be
 /// configured. `MetricsSystem` is set up just once in a given program to set up the desired metrics backend
 /// implementation.
@@ -459,7 +471,7 @@ public enum MetricsSystem {
 
     /// `bootstrap` is an one-time configuration function which globally selects the desired metrics backend
     /// implementation. `bootstrap` can be called at maximum once in any given program, calling it more than once will
-    /// lead to undefined behaviour, most likely a crash.
+    /// lead to undefined behavior, most likely a crash.
     ///
     /// - parameters:
     ///     - factory: A factory that given an identifier produces instances of metrics handlers such as `CounterHandler`, `RecorderHandler` and `TimerHandler`.
@@ -514,7 +526,9 @@ public enum MetricsSystem {
     }
 }
 
-// MARK: Library SPI, intended to be implemented by backend libraries
+// MARK: - Library SPI, intended to be implemented by backend libraries
+
+// MARK: - MetricsFactory
 
 /// The `MetricsFactory` is the bridge between the `MetricsSystem` and the metrics backend implementation.
 /// `MetricsFactory`'s role is to initialize concrete implementations of the various metric types:
@@ -601,7 +615,7 @@ public protocol MetricsFactory {
 }
 
 /// Wraps a CounterHandler, adding support for incrementing by floating point values by storing an accumulated floating point value and recording increments to the underlying CounterHandler after crossing integer boundaries.
-internal class AccumulatingRoundingFloatingPointCounter: FloatingPointCounterHandler {
+internal final class AccumulatingRoundingFloatingPointCounter: FloatingPointCounterHandler {
     private let lock = Lock()
     private let counterHandler: CounterHandler
     internal var fraction: Double = 0
@@ -689,6 +703,8 @@ extension MetricsFactory {
     }
 }
 
+// MARK: - Backend Handlers
+
 /// A `CounterHandler` represents a backend implementation of a `Counter`.
 ///
 /// This type is an implementation detail and should not be used directly, unless implementing your own metrics backend.
@@ -700,7 +716,7 @@ extension MetricsFactory {
 /// as expected regardless of the selected `CounterHandler` implementation.
 ///
 /// - The `CounterHandler` must be a `class`.
-public protocol CounterHandler: AnyObject {
+public protocol CounterHandler: AnyObject, _SwiftMetricsSendableProtocol {
     /// Increment the counter.
     ///
     /// - parameters:
@@ -722,7 +738,7 @@ public protocol CounterHandler: AnyObject {
 /// as expected regardless of the selected `FloatingPointCounterHandler` implementation.
 ///
 /// - The `FloatingPointCounterHandler` must be a `class`.
-public protocol FloatingPointCounterHandler: AnyObject {
+public protocol FloatingPointCounterHandler: AnyObject, _SwiftMetricsSendableProtocol {
     /// Increment the counter.
     ///
     /// - parameters:
@@ -744,7 +760,7 @@ public protocol FloatingPointCounterHandler: AnyObject {
 /// as expected regardless of the selected `RecorderHandler` implementation.
 ///
 /// - The `RecorderHandler` must be a `class`.
-public protocol RecorderHandler: AnyObject {
+public protocol RecorderHandler: AnyObject, _SwiftMetricsSendableProtocol {
     /// Record a value.
     ///
     /// - parameters:
@@ -768,7 +784,7 @@ public protocol RecorderHandler: AnyObject {
 /// as expected regardless of the selected `TimerHandler` implementation.
 ///
 /// - The `TimerHandler` must be a `class`.
-public protocol TimerHandler: AnyObject {
+public protocol TimerHandler: AnyObject, _SwiftMetricsSendableProtocol {
     /// Record a duration in nanoseconds.
     ///
     /// - parameters:
@@ -788,7 +804,7 @@ extension TimerHandler {
     }
 }
 
-// MARK: Predefined Metrics Handlers
+// MARK: - Predefined Metrics Handlers
 
 /// A pseudo-metrics handler that can be used to send messages to multiple other metrics handlers.
 public final class MultiplexMetricsHandler: MetricsFactory {
@@ -837,7 +853,7 @@ public final class MultiplexMetricsHandler: MetricsFactory {
         }
     }
 
-    private class MuxCounter: CounterHandler {
+    private final class MuxCounter: CounterHandler {
         let counters: [CounterHandler]
         public init(factories: [MetricsFactory], label: String, dimensions: [(String, String)]) {
             self.counters = factories.map { $0.makeCounter(label: label, dimensions: dimensions) }
@@ -852,7 +868,7 @@ public final class MultiplexMetricsHandler: MetricsFactory {
         }
     }
 
-    private class MuxFloatingPointCounter: FloatingPointCounterHandler {
+    private final class MuxFloatingPointCounter: FloatingPointCounterHandler {
         let counters: [FloatingPointCounterHandler]
         public init(factories: [MetricsFactory], label: String, dimensions: [(String, String)]) {
             self.counters = factories.map { $0.makeFloatingPointCounter(label: label, dimensions: dimensions) }
@@ -867,7 +883,7 @@ public final class MultiplexMetricsHandler: MetricsFactory {
         }
     }
 
-    private class MuxRecorder: RecorderHandler {
+    private final class MuxRecorder: RecorderHandler {
         let recorders: [RecorderHandler]
         public init(factories: [MetricsFactory], label: String, dimensions: [(String, String)], aggregate: Bool) {
             self.recorders = factories.map { $0.makeRecorder(label: label, dimensions: dimensions, aggregate: aggregate) }
@@ -882,7 +898,7 @@ public final class MultiplexMetricsHandler: MetricsFactory {
         }
     }
 
-    private class MuxTimer: TimerHandler {
+    private final class MuxTimer: TimerHandler {
         let timers: [TimerHandler]
         public init(factories: [MetricsFactory], label: String, dimensions: [(String, String)]) {
             self.timers = factories.map { $0.makeTimer(label: label, dimensions: dimensions) }
@@ -928,3 +944,22 @@ public final class NOOPMetricsHandler: MetricsFactory, CounterHandler, FloatingP
     public func record(_: Double) {}
     public func recordNanoseconds(_: Int64) {}
 }
+
+// MARK: - Sendable support helpers
+
+#if compiler(>=5.6)
+extension MetricsSystem: Sendable {}
+extension Counter: Sendable {}
+extension FloatingPointCounter: Sendable {}
+// must be @unchecked since Gauge inherits Recorder :(
+extension Recorder: @unchecked Sendable {}
+extension Timer: Sendable {}
+// ideally we would not be using @unchecked here, but concurrency-safety checks do not recognize locks
+extension AccumulatingRoundingFloatingPointCounter: @unchecked Sendable {}
+#endif
+
+#if compiler(>=5.6)
+@preconcurrency public protocol _SwiftMetricsSendableProtocol: Sendable {}
+#else
+public protocol _SwiftMetricsSendableProtocol {}
+#endif

Sources/Metrics/Metrics.swift

@@ -16,7 +16,7 @@
 @_exported import class CoreMetrics.Timer
 import Foundation
 
-public extension Timer {
+extension Timer {
     /// Convenience for measuring duration of a closure.
     ///
     /// - parameters:
@@ -24,7 +24,7 @@ public extension Timer {
     ///     - dimensions: The dimensions for the Timer.
     ///     - body: Closure to run & record.
     @inlinable
-    static func measure<T>(label: String, dimensions: [(String, String)] = [], body: @escaping () throws -> T) rethrows -> T {
+    public static func measure<T>(label: String, dimensions: [(String, String)] = [], body: @escaping () throws -> T) rethrows -> T {
         let timer = Timer(label: label, dimensions: dimensions)
         let start = DispatchTime.now().uptimeNanoseconds
         defer {
@@ -39,18 +39,18 @@ public extension Timer {
     /// - parameters:
     ///   - since: Start of the interval as `DispatchTime`.
     ///   - end: End of the interval, defaulting to `.now()`.
-    func recordInterval(since: DispatchTime, end: DispatchTime = .now()) {
+    public func recordInterval(since: DispatchTime, end: DispatchTime = .now()) {
         self.recordNanoseconds(end.uptimeNanoseconds - since.uptimeNanoseconds)
     }
 }
 
-public extension Timer {
+extension Timer {
     /// Convenience for recording a duration based on TimeInterval.
     ///
     /// - parameters:
     ///     - duration: The duration to record.
     @inlinable
-    func record(_ duration: TimeInterval) {
+    public func record(_ duration: TimeInterval) {
         self.recordSeconds(duration)
     }
 
@@ -59,7 +59,7 @@ public extension Timer {
     /// - parameters:
     ///     - duration: The duration to record.
     @inlinable
-    func record(_ duration: DispatchTimeInterval) {
+    public func record(_ duration: DispatchTimeInterval) {
         switch duration {
         case .nanoseconds(let value):
             self.recordNanoseconds(value)

Sources/MetricsTestKit/TestMetrics.swift

@@ -139,14 +139,10 @@ extension TestMetrics.FullKey: Hashable {
     }
 }
 
-// ==== ----------------------------------------------------------------------------------------------------------------
+// MARK: - Assertions
-
-// MARK: Assertions
 
 extension TestMetrics {
-    // ==== ------------------------------------------------------------------------------------------------------------
+    // MARK: - Counter
-
-    // MARK: Counter
 
     public func expectCounter(_ metric: Counter) throws -> TestCounter {
         guard let counter = metric.handler as? TestCounter else {
@@ -168,9 +164,7 @@ extension TestMetrics {
         return testCounter
     }
 
-    // ==== ------------------------------------------------------------------------------------------------------------
+    // MARK: - Gauge
-
-    // MARK: Gauge
 
     public func expectGauge(_ metric: Gauge) throws -> TestRecorder {
         return try self.expectRecorder(metric)
@@ -180,9 +174,7 @@ extension TestMetrics {
         return try self.expectRecorder(label, dimensions)
     }
 
-    // ==== ------------------------------------------------------------------------------------------------------------
+    // MARK: - Recorder
-
-    // MARK: Recorder
 
     public func expectRecorder(_ metric: Recorder) throws -> TestRecorder {
         guard let recorder = metric.handler as? TestRecorder else {
@@ -204,9 +196,7 @@ extension TestMetrics {
         return testRecorder
     }
 
-    // ==== ------------------------------------------------------------------------------------------------------------
+    // MARK: - Timer
-
-    // MARK: Timer
 
     public func expectTimer(_ metric: CoreMetrics.Timer) throws -> TestTimer {
         guard let timer = metric.handler as? TestTimer else {
@@ -229,9 +219,7 @@ extension TestMetrics {
     }
 }
 
-// ==== ----------------------------------------------------------------------------------------------------------------
+// MARK: - Metric type implementations
-
-// MARK: Metric type implementations
 
 public protocol TestMetric {
     associatedtype Value
@@ -418,11 +406,27 @@ extension NSLock {
     }
 }
 
-// ==== ----------------------------------------------------------------------------------------------------------------
+// MARK: - Errors
 
-// MARK: Errors
+#if compiler(>=5.6)
+public enum TestMetricsError: Error {
+    case missingMetric(label: String, dimensions: [(String, String)])
+    case illegalMetricType(metric: Sendable, expected: String)
+}
 
+#else
 public enum TestMetricsError: Error {
     case missingMetric(label: String, dimensions: [(String, String)])
     case illegalMetricType(metric: Any, expected: String)
 }
+#endif
+
+// MARK: - Sendable support
+
+#if compiler(>=5.6)
+// ideally we would not be using @unchecked here, but concurrency-safety checks do not recognize locks
+extension TestMetrics: @unchecked Sendable {}
+extension TestCounter: @unchecked Sendable {}
+extension TestRecorder: @unchecked Sendable {}
+extension TestTimer: @unchecked Sendable {}
+#endif

Tests/MetricsTests/CoreMetricsTests.swift

@@ -384,7 +384,7 @@ class MetricsTests: XCTestCase {
     }
 
     func testCustomFactory() {
-        class CustomHandler: CounterHandler {
+        final class CustomHandler: CounterHandler {
             func increment<DataType>(by: DataType) where DataType: BinaryInteger {}
             func reset() {}
         }

Tests/MetricsTests/TestMetrics.swift

@@ -40,8 +40,8 @@ internal final class TestMetrics: MetricsFactory {
     }
 
     private func make<Item>(label: String, dimensions: [(String, String)], registry: inout [String: Item], maker: (String, [(String, String)]) -> Item) -> Item {
-        return self.lock.withLock {
         let item = maker(label, dimensions)
+        return self.lock.withLock {
             registry[label] = item
             return item
         }
@@ -49,21 +49,27 @@ internal final class TestMetrics: MetricsFactory {
 
     func destroyCounter(_ handler: CounterHandler) {
         if let testCounter = handler as? TestCounter {
+            self.lock.withLock { () -> Void in
                 self.counters.removeValue(forKey: testCounter.label)
             }
         }
+    }
 
     func destroyRecorder(_ handler: RecorderHandler) {
         if let testRecorder = handler as? TestRecorder {
+            self.lock.withLock { () -> Void in
                 self.recorders.removeValue(forKey: testRecorder.label)
             }
         }
+    }
 
     func destroyTimer(_ handler: TimerHandler) {
         if let testTimer = handler as? TestTimer {
+            self.lock.withLock { () -> Void in
                 self.timers.removeValue(forKey: testTimer.label)
             }
         }
+    }
 }
 
 internal class TestCounter: CounterHandler, Equatable {
@@ -176,8 +182,8 @@ internal class TestTimer: TimerHandler, Equatable {
     }
 }
 
-private extension NSLock {
+extension NSLock {
-    func withLock<T>(_ body: () -> T) -> T {
+    fileprivate func withLock<T>(_ body: () -> T) -> T {
         self.lock()
         defer {
             self.unlock()
@@ -185,3 +191,12 @@ private extension NSLock {
         return body()
     }
 }
+
+// MARK: - Sendable support
+
+#if compiler(>=5.6)
+// ideally we would not be using @unchecked here, but concurrency-safety checks do not recognize locks
+extension TestCounter: @unchecked Sendable {}
+extension TestRecorder: @unchecked Sendable {}
+extension TestTimer: @unchecked Sendable {}
+#endif

Tests/MetricsTests/TestSendable.swift

@@ -0,0 +1,101 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift Metrics API open source project
+//
+// Copyright (c) 2022 Apple Inc. and the Swift Metrics API project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of Swift Metrics API project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+@testable import CoreMetrics
+import Dispatch
+import XCTest
+
+class SendableTest: XCTestCase {
+    #if compiler(>=5.6)
+    func testSendableMetrics() async {
+        // bootstrap with our test metrics
+        let metrics = TestMetrics()
+        MetricsSystem.bootstrapInternal(metrics)
+
+        do {
+            let name = "counter-\(NSUUID().uuidString)"
+            let value = Int.random(in: 0 ... 1000)
+            let counter = Counter(label: name)
+
+            let task = Task.detached { () -> [Int64] in
+                counter.increment(by: value)
+                let handler = counter.handler as! TestCounter
+                return handler.values.map { $0.1 }
+            }
+            let values = await task.value
+            XCTAssertEqual(values.count, 1, "expected number of entries to match")
+            XCTAssertEqual(values[0], Int64(value), "expected value to match")
+        }
+
+        do {
+            let name = "floating-point-counter-\(NSUUID().uuidString)"
+            let value = Double.random(in: 0 ... 0.9999)
+            let counter = FloatingPointCounter(label: name)
+
+            let task = Task.detached { () -> Double in
+                counter.increment(by: value)
+                let handler = counter.handler as! AccumulatingRoundingFloatingPointCounter
+                return handler.fraction
+            }
+            let fraction = await task.value
+            XCTAssertEqual(fraction, value)
+        }
+
+        do {
+            let name = "recorder-\(NSUUID().uuidString)"
+            let value = Double.random(in: -1000 ... 1000)
+            let recorder = Recorder(label: name)
+
+            let task = Task.detached { () -> [Double] in
+                recorder.record(value)
+                let handler = recorder.handler as! TestRecorder
+                return handler.values.map { $0.1 }
+            }
+            let values = await task.value
+            XCTAssertEqual(values.count, 1, "expected number of entries to match")
+            XCTAssertEqual(values[0], value, "expected value to match")
+        }
+
+        do {
+            let name = "gauge-\(NSUUID().uuidString)"
+            let value = Double.random(in: -1000 ... 1000)
+            let gauge = Gauge(label: name)
+
+            let task = Task.detached { () -> [Double] in
+                gauge.record(value)
+                let handler = gauge.handler as! TestRecorder
+                return handler.values.map { $0.1 }
+            }
+            let values = await task.value
+            XCTAssertEqual(values.count, 1, "expected number of entries to match")
+            XCTAssertEqual(values[0], value, "expected value to match")
+        }
+
+        do {
+            let name = "timer-\(NSUUID().uuidString)"
+            let value = Int64.random(in: 0 ... 1000)
+            let timer = Timer(label: name)
+
+            let task = Task.detached { () -> [Int64] in
+                timer.recordNanoseconds(value)
+                let handler = timer.handler as! TestTimer
+                return handler.values.map { $0.1 }
+            }
+            let values = await task.value
+            XCTAssertEqual(values.count, 1, "expected number of entries to match")
+            XCTAssertEqual(values[0], value, "expected value to match")
+        }
+    }
+    #endif
+}

docker/docker-compose.yaml

@@ -40,4 +40,4 @@ services:
 
   shell:
     <<: *common
-    entrypoint: /bin/bash
+    entrypoint: /bin/bash -l

scripts/soundness.sh

@@ -32,7 +32,7 @@ here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 
 function replace_acceptable_years() {
     # this needs to replace all acceptable forms with 'YEARS'
-    sed -e 's/2017-2019/YEARS/' -e 's/2018-2019/YEARS/' -e 's/2019/YEARS/' -e 's/2021/YEARS/'
+    sed -e 's/20[12][789012]-20[12][89012]/YEARS/' -e 's/20[12][89012]/YEARS/'
 }
 
 printf "=> Checking linux tests... "