From 8bd4ad2da864e92e9ae5242240706281f933c68e Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 31 Jan 2023 08:33:12 +0900 Subject: [PATCH 1/6] mark types Sendable --- Sources/Instrumentation/Instrument.swift | 21 ++++++++++++++++--- Sources/Tracing/Span.swift | 20 ++++++++++++++++-- .../DynamicTracepointTracerTests.swift | 7 +++++++ Tests/TracingTests/TestTracer.swift | 7 +++++++ Tests/TracingTests/TracedLockTests.swift | 6 ++++++ 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/Sources/Instrumentation/Instrument.swift b/Sources/Instrumentation/Instrument.swift index 924f0044..12244ea1 100644 --- a/Sources/Instrumentation/Instrument.swift +++ b/Sources/Instrumentation/Instrument.swift @@ -14,10 +14,21 @@ import InstrumentationBaggage +/// Typealias used to simplify Support of old Swift versions which do not have `Sendable` defined. +#if swift(>=5.6.0) +@preconcurrency public protocol _SwiftInstrumentationSendable: Sendable {} +#else +public protocol _SwiftInstrumentationSendable {} +#endif + /// Conforming types are used to extract values from a specific `Carrier`. -public protocol Extractor { +public protocol Extractor: _SwiftInstrumentationSendable { /// The carrier to extract values from. + #if swift(>=5.6.0) + associatedtype Carrier: Sendable + #else associatedtype Carrier + #endif /// Extract the value for the given key from the `Carrier`. /// @@ -28,9 +39,13 @@ public protocol Extractor { } /// Conforming types are used to inject values into a specific `Carrier`. -public protocol Injector { +public protocol Injector: _SwiftInstrumentationSendable { /// The carrier to inject values into. + #if swift(>=5.6.0) + associatedtype Carrier: Sendable + #else associatedtype Carrier + #endif /// Inject the given value for the given key into the given `Carrier`. /// @@ -43,7 +58,7 @@ public protocol Injector { /// Conforming types are usually cross-cutting tools like tracers. They are agnostic of what specific `Carrier` is used /// to propagate metadata across boundaries, but instead just specify what values to use for which keys. -public protocol Instrument { +public protocol Instrument: _SwiftInstrumentationSendable { /// Extract values from a `Carrier` by using the given extractor and inject them into the given `Baggage`. /// It's quite common for `Instrument`s to come up with new values if they weren't passed along in the given `Carrier`. /// diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index 5316b224..7587013a 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -22,7 +22,7 @@ import Dispatch /// Creating a `Span` is delegated to a ``Tracer`` and end users should never create them directly. /// /// - SeeAlso: For more details refer to the [OpenTelemetry Specification: Span](https://github.com/open-telemetry/opentelemetry-specification/blob/v0.7.0/specification/trace/api.md#span) which this type is compatible with. -public protocol Span: AnyObject { +public protocol Span: AnyObject, _SwiftTracingSendableSpan { /// The read-only `Baggage` of this `Span`, set when starting this `Span`. var baggage: Baggage { get } @@ -650,10 +650,26 @@ public struct SpanLink { /// Create a new `SpanLink`. /// - Parameters: - /// - context: The `Baggage` identifying the targeted ``Span``. + /// - baggage: The `Baggage` identifying the targeted ``Span``. /// - attributes: ``SpanAttributes`` that further describe the link. Defaults to no attributes. public init(baggage: Baggage, attributes: SpanAttributes = [:]) { self.baggage = baggage self.attributes = attributes } } + +#if compiler(>=5.6) +@preconcurrency public protocol _SwiftTracingSendableSpan: Sendable {} +#else +public protocol _SwiftTracingSendableSpan {} +#endif + +#if compiler(>=5.6) +extension SpanAttributes: Sendable {} +extension SpanAttribute: @unchecked Sendable {} +extension SpanStatus: Sendable {} +extension SpanEvent: @unchecked Sendable {} // unchecked because of DispatchWallTime +extension SpanKind: Sendable {} +extension SpanStatus.Code: Sendable {} +extension SpanLink: Sendable {} +#endif diff --git a/Tests/TracingTests/DynamicTracepointTracerTests.swift b/Tests/TracingTests/DynamicTracepointTracerTests.swift index 34b3b73a..e54a0838 100644 --- a/Tests/TracingTests/DynamicTracepointTracerTests.swift +++ b/Tests/TracingTests/DynamicTracepointTracerTests.swift @@ -104,6 +104,7 @@ final class DynamicTracepointTracerTests: XCTestCase { } } +/// Only intended to be used in single-threaded testing. final class DynamicTracepointTestTracer: Tracer { private(set) var activeTracepoints: Set = [] @@ -227,6 +228,7 @@ final class DynamicTracepointTestTracer: Tracer { } extension DynamicTracepointTestTracer { + /// Only intended to be used in single-threaded testing. final class TracepointSpan: Tracing.Span { private let operationName: String private let kind: SpanKind @@ -288,3 +290,8 @@ extension DynamicTracepointTestTracer { } } } + +#if compiler(>=5.6.0) +extension DynamicTracepointTestTracer: @unchecked Sendable {} +extension DynamicTracepointTestTracer.TracepointSpan: @unchecked Sendable {} +#endif diff --git a/Tests/TracingTests/TestTracer.swift b/Tests/TracingTests/TestTracer.swift index e9ec3a36..10a7d940 100644 --- a/Tests/TracingTests/TestTracer.swift +++ b/Tests/TracingTests/TestTracer.swift @@ -18,6 +18,7 @@ import Instrumentation import InstrumentationBaggage import Tracing +/// Only intended to be used in single-threaded testing. final class TestTracer: Tracer { private(set) var spans = [TestSpan]() var onEndSpan: (Span) -> Void = { _ in } @@ -93,6 +94,7 @@ extension Baggage { } } +/// Only intended to be used in single-threaded testing. final class TestSpan: Span { private let operationName: String private let kind: SpanKind @@ -156,3 +158,8 @@ final class TestSpan: Span { self.onEnd(self) } } + +#if compiler(>=5.6.0) +extension TestTracer: @unchecked Sendable {} +extension TestSpan: @unchecked Sendable {} +#endif diff --git a/Tests/TracingTests/TracedLockTests.swift b/Tests/TracingTests/TracedLockTests.swift index 81ad3d98..b487dfec 100644 --- a/Tests/TracingTests/TracedLockTests.swift +++ b/Tests/TracingTests/TracedLockTests.swift @@ -58,6 +58,7 @@ enum TaskIDKey: BaggageKey { // ==== ------------------------------------------------------------------------ // MARK: PrintLn Tracer +/// Only intended to be used in single-threaded testing. private final class TracedLockPrintlnTracer: Tracer { func startSpan( _ operationName: String, @@ -158,3 +159,8 @@ private final class TracedLockPrintlnTracer: Tracer { } } } + +#if compiler(>=5.6.0) +extension TracedLockPrintlnTracer: Sendable {} +extension TracedLockPrintlnTracer.TracedLockPrintlnSpan: @unchecked Sendable {} +#endif From 438a7b67d6a14659ded905d00dc94988f3435b02 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 31 Jan 2023 21:07:04 +0900 Subject: [PATCH 2/6] not sendable carrier --- Sources/Instrumentation/Instrument.swift | 8 -------- Sources/Tracing/Tracer.swift | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/Sources/Instrumentation/Instrument.swift b/Sources/Instrumentation/Instrument.swift index 12244ea1..e6d33ef7 100644 --- a/Sources/Instrumentation/Instrument.swift +++ b/Sources/Instrumentation/Instrument.swift @@ -24,11 +24,7 @@ public protocol _SwiftInstrumentationSendable {} /// Conforming types are used to extract values from a specific `Carrier`. public protocol Extractor: _SwiftInstrumentationSendable { /// The carrier to extract values from. - #if swift(>=5.6.0) associatedtype Carrier: Sendable - #else - associatedtype Carrier - #endif /// Extract the value for the given key from the `Carrier`. /// @@ -41,11 +37,7 @@ public protocol Extractor: _SwiftInstrumentationSendable { /// Conforming types are used to inject values into a specific `Carrier`. public protocol Injector: _SwiftInstrumentationSendable { /// The carrier to inject values into. - #if swift(>=5.6.0) associatedtype Carrier: Sendable - #else - associatedtype Carrier - #endif /// Inject the given value for the given key into the given `Carrier`. /// diff --git a/Sources/Tracing/Tracer.swift b/Sources/Tracing/Tracer.swift index f0b3cb47..e0b7eb2f 100644 --- a/Sources/Tracing/Tracer.swift +++ b/Sources/Tracing/Tracer.swift @@ -29,7 +29,7 @@ public protocol Tracer: Instrument { /// - operationName: The name of the operation being traced. This may be a handler function, database call, ... /// - baggage: The `Baggage` providing information on where to start the new ``Span``. /// - kind: The ``SpanKind`` of the new ``Span``. - /// - time: The `DispatchTime` at which to start the new ``Span``. + /// - time: The time at which to start the new ``Span``. /// - function: The function name in which the span was started /// - fileID: The `fileID` where the span was started. /// - line: The file line where the span was started. From 8bbbfa301922b6ddf92f981f722a3d07d581f35b Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 31 Jan 2023 21:11:42 +0900 Subject: [PATCH 3/6] avoid unchecked Sendable --- Sources/Tracing/Span.swift | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index 7587013a..7b9e7fcb 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -12,7 +12,7 @@ // //===----------------------------------------------------------------------===// -import Dispatch +@preconcurrency import struct Dispatch.DispatchWallTime @_exported import InstrumentationBaggage /// A `Span` represents an interval from the start of an operation to its end, along with additional metadata included @@ -233,8 +233,13 @@ public enum SpanAttribute: Equatable { case string(String) case stringArray([String]) + #if swift(>=5.6) + case stringConvertible(CustomStringConvertible & Sendable) + case stringConvertibleArray([CustomStringConvertible & Sendable]) + #else case stringConvertible(CustomStringConvertible) case stringConvertibleArray([CustomStringConvertible]) + #endif #if swift(>=5.2) // older swifts get confused and can't resolve if we mean the `case int(Int64)` or any of those overloads @@ -666,9 +671,9 @@ public protocol _SwiftTracingSendableSpan {} #if compiler(>=5.6) extension SpanAttributes: Sendable {} -extension SpanAttribute: @unchecked Sendable {} +extension SpanAttribute: Sendable {} // @unchecked because some payloads are CustomStringConvertible extension SpanStatus: Sendable {} -extension SpanEvent: @unchecked Sendable {} // unchecked because of DispatchWallTime +extension SpanEvent: Sendable {} extension SpanKind: Sendable {} extension SpanStatus.Code: Sendable {} extension SpanLink: Sendable {} From 9a928dfa0b1f27056876c812716713595014b09d Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 31 Jan 2023 21:13:47 +0900 Subject: [PATCH 4/6] dont need to require sendable on carrier --- Sources/Instrumentation/Instrument.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Instrumentation/Instrument.swift b/Sources/Instrumentation/Instrument.swift index e6d33ef7..7224b7e1 100644 --- a/Sources/Instrumentation/Instrument.swift +++ b/Sources/Instrumentation/Instrument.swift @@ -24,7 +24,7 @@ public protocol _SwiftInstrumentationSendable {} /// Conforming types are used to extract values from a specific `Carrier`. public protocol Extractor: _SwiftInstrumentationSendable { /// The carrier to extract values from. - associatedtype Carrier: Sendable + associatedtype Carrier /// Extract the value for the given key from the `Carrier`. /// @@ -37,7 +37,7 @@ public protocol Extractor: _SwiftInstrumentationSendable { /// Conforming types are used to inject values into a specific `Carrier`. public protocol Injector: _SwiftInstrumentationSendable { /// The carrier to inject values into. - associatedtype Carrier: Sendable + associatedtype Carrier /// Inject the given value for the given key into the given `Carrier`. /// From 0a947e1b28892c2fd04152f66da4e91261927289 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 31 Jan 2023 21:15:25 +0900 Subject: [PATCH 5/6] surround @preconcurrency with swift conditionals --- Sources/Tracing/Span.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index 7b9e7fcb..3f118b2f 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -12,7 +12,11 @@ // //===----------------------------------------------------------------------===// +#if swift(>=5.6.0) @preconcurrency import struct Dispatch.DispatchWallTime +#else +import struct Dispatch.DispatchWallTime +#endif @_exported import InstrumentationBaggage /// A `Span` represents an interval from the start of an operation to its end, along with additional metadata included From 53cf9ff9bd4a05210e9a0d8f56e625f581d9a2c9 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 31 Jan 2023 21:36:32 +0900 Subject: [PATCH 6/6] avoid error on 5.6 when sendable convertible used --- Sources/Tracing/Span.swift | 13 ++++++++++--- .../TracingTests/DynamicTracepointTracerTests.swift | 4 ++-- Tests/TracingTests/SpanTests.swift | 8 +++++++- Tests/TracingTests/TestTracer.swift | 4 ++-- Tests/TracingTests/TracedLockTests.swift | 2 +- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index 3f118b2f..ad15b2c3 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -392,11 +392,18 @@ extension Array: SpanAttributeConvertible where Element: SpanAttributeConvertibl return .boolArray(value) } else if let value = self as? [String] { return .stringArray(value) - } else if let value = self as? [CustomStringConvertible] { + } + #if swift(>=5.6.0) + if let value = self as? [CustomStringConvertible & Sendable] { + return .stringConvertibleArray(value) + } + #else + if let value = self as? [CustomStringConvertible] { return .stringConvertibleArray(value) - } else { - fatalError("Not supported SpanAttribute array type: \(type(of: self))") } + #endif + + fatalError("Not supported SpanAttribute array type: \(type(of: self))") } } diff --git a/Tests/TracingTests/DynamicTracepointTracerTests.swift b/Tests/TracingTests/DynamicTracepointTracerTests.swift index e54a0838..0738b8f1 100644 --- a/Tests/TracingTests/DynamicTracepointTracerTests.swift +++ b/Tests/TracingTests/DynamicTracepointTracerTests.swift @@ -292,6 +292,6 @@ extension DynamicTracepointTestTracer { } #if compiler(>=5.6.0) -extension DynamicTracepointTestTracer: @unchecked Sendable {} -extension DynamicTracepointTestTracer.TracepointSpan: @unchecked Sendable {} +extension DynamicTracepointTestTracer: @unchecked Sendable {} // only intended for single threaded testing +extension DynamicTracepointTestTracer.TracepointSpan: @unchecked Sendable {} // only intended for single threaded testing #endif diff --git a/Tests/TracingTests/SpanTests.swift b/Tests/TracingTests/SpanTests.swift index 3e79d8e0..25aa30e8 100644 --- a/Tests/TracingTests/SpanTests.swift +++ b/Tests/TracingTests/SpanTests.swift @@ -244,7 +244,7 @@ public struct HTTPAttributes: SpanAttributeNamespace { } } -public struct CustomAttributeValue: Equatable, CustomStringConvertible, SpanAttributeConvertible { +public struct CustomAttributeValue: Equatable, _CustomAttributeValueSendable, CustomStringConvertible, SpanAttributeConvertible { public func toSpanAttribute() -> SpanAttribute { .stringConvertible(self) } @@ -255,6 +255,12 @@ public struct CustomAttributeValue: Equatable, CustomStringConvertible, SpanAttr } #endif +#if swift(>=5.6.0) +typealias _CustomAttributeValueSendable = Sendable +#else +typealias _CustomAttributeValueSendable = Any +#endif + private struct TestBaggageContextKey: BaggageKey { typealias Value = String } diff --git a/Tests/TracingTests/TestTracer.swift b/Tests/TracingTests/TestTracer.swift index 10a7d940..397809f4 100644 --- a/Tests/TracingTests/TestTracer.swift +++ b/Tests/TracingTests/TestTracer.swift @@ -160,6 +160,6 @@ final class TestSpan: Span { } #if compiler(>=5.6.0) -extension TestTracer: @unchecked Sendable {} -extension TestSpan: @unchecked Sendable {} +extension TestTracer: @unchecked Sendable {} // only intended for single threaded testing +extension TestSpan: @unchecked Sendable {} // only intended for single threaded testing #endif diff --git a/Tests/TracingTests/TracedLockTests.swift b/Tests/TracingTests/TracedLockTests.swift index b487dfec..e7caee5a 100644 --- a/Tests/TracingTests/TracedLockTests.swift +++ b/Tests/TracingTests/TracedLockTests.swift @@ -162,5 +162,5 @@ private final class TracedLockPrintlnTracer: Tracer { #if compiler(>=5.6.0) extension TracedLockPrintlnTracer: Sendable {} -extension TracedLockPrintlnTracer.TracedLockPrintlnSpan: @unchecked Sendable {} +extension TracedLockPrintlnTracer.TracedLockPrintlnSpan: @unchecked Sendable {} // only intended for single threaded testing #endif