Merge pull request #142 from slashmo/feature/span-class-requirement

Require Span types to be classes

Author: slashmo

Package.swift

@@ -7,14 +7,14 @@ let package = Package(
         .library(name: "Instrumentation", targets: ["Instrumentation"]),
         .library(name: "Tracing", targets: ["Tracing"]),
         .library(name: "NIOInstrumentation", targets: ["NIOInstrumentation"]),
-        .library(name: "OpenTelemetryInstrumentationSupport", targets: ["OpenTelemetryInstrumentationSupport"])
+        .library(name: "OpenTelemetryInstrumentationSupport", targets: ["OpenTelemetryInstrumentationSupport"]),
     ],
     dependencies: [
         .package(
             url: "https://github.com/slashmo/gsoc-swift-baggage-context.git",
             from: "0.3.0"
         ),
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.17.0")
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.17.0"),
     ],
     targets: [
         // ==== --------------------------------------------------------------------------------------------------------

Sources/Tracing/NoOpTracer.swift

@@ -45,16 +45,16 @@ public struct NoOpTracer: Tracer {
         Extractor: ExtractorProtocol,
         Carrier == Extractor.Carrier {}
 
-    public struct NoOpSpan: Span {
+    public final class NoOpSpan: Span {
         public var context: BaggageContext {
             return .init()
         }
 
-        public mutating func setStatus(_ status: SpanStatus) {}
+        public func setStatus(_ status: SpanStatus) {}
 
-        public mutating func addLink(_ link: SpanLink) {}
+        public func addLink(_ link: SpanLink) {}
 
-        public mutating func addEvent(_ event: SpanEvent) {}
+        public func addEvent(_ event: SpanEvent) {}
 
         public func recordError(_ error: Error) {}
 
@@ -69,7 +69,7 @@ public struct NoOpTracer: Tracer {
 
         public let isRecording = false
 
-        public mutating func end(at timestamp: Timestamp) {
+        public func end(at timestamp: Timestamp) {
             // ignore
         }
     }

Sources/Tracing/Span.swift

@@ -18,23 +18,23 @@ import Baggage
 /// where `tracer` conforms to `Tracer`.
 ///
 /// - SeeAlso: [OpenTelemetry Specification: Span](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#span).
-public protocol Span {
+public protocol Span: AnyObject {
     /// The read-only `BaggageContext` of this `Span`, set when starting this `Span`.
     var context: BaggageContext { get }
 
     /// Set the status.
     /// - Parameter status: The status of this `Span`.
-    mutating func setStatus(_ status: SpanStatus)
+    func setStatus(_ status: SpanStatus)
 
     /// Add a `SpanEvent` in place.
     /// - Parameter event: The `SpanEvent` to add to this `Span`.
-    mutating func addEvent(_ event: SpanEvent)
+    func addEvent(_ event: SpanEvent)
 
     /// Record an error of the given type described by the the given message.
     ///
     /// - Parameters:
     ///   - error: The error to be recorded.
-    mutating func recordError(_ error: Error)
+    func recordError(_ error: Error)
 
     /// The attributes describing this `Span`.
     var attributes: SpanAttributes { get set }
@@ -44,32 +44,23 @@ public protocol Span {
 
     /// Add a `SpanLink` in place.
     /// - Parameter link: The `SpanLink` to add to this `Span`.
-    mutating func addLink(_ link: SpanLink)
+    func addLink(_ link: SpanLink)
 
     /// End this `Span` at the given timestamp.
     /// - Parameter timestamp: The `Timestamp` at which the span ended.
-    mutating func end(at timestamp: Timestamp)
+    func end(at timestamp: Timestamp)
 }
 
 extension Span {
-    /// Create a copy of this `Span` with the given event added to the existing set of events.
-    /// - Parameter event: The new `SpanEvent` to be added to the returned copy.
-    /// - Returns: A copy of this `Span` with the given event added to the existing set of events.
-    public func addingEvent(_ event: SpanEvent) -> Self {
-        var copy = self
-        copy.addEvent(event)
-        return copy
-    }
-
     /// End this `Span` at the current timestamp.
-    public mutating func end() {
+    public func end() {
         self.end(at: .now())
     }
 
     /// Adds a `SpanLink` between this `Span` and the given `Span`.
     /// - Parameter other: The `Span` to link to.
     /// - Parameter attributes: The `SpanAttributes` describing this link. Defaults to no attributes.
-    public mutating func addLink(_ other: Span, attributes: SpanAttributes = [:]) {
+    public func addLink(_ other: Span, attributes: SpanAttributes = [:]) {
         self.addLink(SpanLink(context: other.context, attributes: attributes))
     }
 }

Tests/TracingTests/SpanTests+XCTest.swift

@@ -25,7 +25,6 @@ extension SpanTests {
    @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings")
    static var allTests : [(String, (SpanTests) -> () throws -> Void)] {
       return [
-                ("testAddingEventCreatesCopy", testAddingEventCreatesCopy),
                 ("testSpanEventIsExpressibleByStringLiteral", testSpanEventIsExpressibleByStringLiteral),
                 ("testSpanAttributeIsExpressibleByStringLiteral", testSpanAttributeIsExpressibleByStringLiteral),
                 ("testSpanAttributeIsExpressibleByStringInterpolation", testSpanAttributeIsExpressibleByStringInterpolation),

Tests/TracingTests/SpanTests.swift

@@ -17,22 +17,6 @@ import Tracing
 import XCTest
 
 final class SpanTests: XCTestCase {
-    func testAddingEventCreatesCopy() {
-        let span = TestSpan(
-            operationName: "test",
-            startTimestamp: .now(),
-            context: BaggageContext(),
-            kind: .internal,
-            onEnd: { _ in }
-        )
-        XCTAssert(span.events.isEmpty)
-
-        let copiedSpan = span.addingEvent("test-event")
-        XCTAssertEqual(copiedSpan.events[0].name, "test-event")
-
-        XCTAssert(span.events.isEmpty)
-    }
-
     func testSpanEventIsExpressibleByStringLiteral() {
         let event: SpanEvent = "test"
 
@@ -172,7 +156,7 @@ final class SpanTests: XCTestCase {
             onEnd: { _ in }
         )
         let childContext = BaggageContext()
-        var child = TestSpan(
+        let child = TestSpan(
             operationName: "server",
             startTimestamp: .now(),
             context: childContext,

Tests/TracingTests/TracedLockTests.swift

@@ -88,7 +88,7 @@ private final class TracedLockPrintlnTracer: Tracer {
         Extractor: ExtractorProtocol,
         Carrier == Extractor.Carrier {}
 
-    struct TracedLockPrintlnSpan: Span {
+    final class TracedLockPrintlnSpan: Span {
         private let operationName: String
         private let kind: SpanKind
 
@@ -129,22 +129,22 @@ private final class TracedLockPrintlnTracer: Tracer {
             print("  span [\(self.operationName): \(self.context[TaskIDKey.self] ?? "no-name")] @ \(self.startTimestamp): start")
         }
 
-        mutating func setStatus(_ status: SpanStatus) {
+        func setStatus(_ status: SpanStatus) {
             self.status = status
             self.isRecording = true
         }
 
-        mutating func addLink(_ link: SpanLink) {
+        func addLink(_ link: SpanLink) {
             self.links.append(link)
         }
 
-        mutating func addEvent(_ event: SpanEvent) {
+        func addEvent(_ event: SpanEvent) {
             self.events.append(event)
         }
 
         func recordError(_ error: Error) {}
 
-        mutating func end(at timestamp: Timestamp) {
+        func end(at timestamp: Timestamp) {
             self.endTimestamp = timestamp
             print("     span [\(self.operationName): \(self.context[TaskIDKey.self] ?? "no-name")] @ \(timestamp): end")
         }

Tests/TracingTests/TracerTests.swift

@@ -109,7 +109,7 @@ extension TestTracer {
 
 // MARK: - TestSpan
 
-struct TestSpan: Span {
+final class TestSpan: Span {
     private let operationName: String
     private let kind: SpanKind
 
@@ -152,22 +152,22 @@ struct TestSpan: Span {
         self.kind = kind
     }
 
-    mutating func setStatus(_ status: SpanStatus) {
+    func setStatus(_ status: SpanStatus) {
         self.status = status
         self.isRecording = true
     }
 
-    mutating func addLink(_ link: SpanLink) {
+    func addLink(_ link: SpanLink) {
         self.links.append(link)
     }
 
-    mutating func addEvent(_ event: SpanEvent) {
+    func addEvent(_ event: SpanEvent) {
         self.events.append(event)
     }
 
     func recordError(_ error: Error) {}
 
-    mutating func end(at timestamp: Timestamp) {
+    func end(at timestamp: Timestamp) {
         self.endTimestamp = timestamp
         self.onEnd(self)
     }
@@ -218,7 +218,7 @@ struct FakeHTTPServer {
         var context = BaggageContext()
         self.instrument.extract(request.headers, into: &context, using: HTTPHeadersExtractor())
 
-        var span = tracer.startSpan(named: "GET \(request.path)", context: context)
+        let span = tracer.startSpan(named: "GET \(request.path)", context: context)
 
         let response = self.catchAllHandler(span.context, request, self.client)
         span.attributes["http.status"] = .int(response.status)