From c7e646b58eb68bd560f94a804d988cd279849b2c Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 24 Feb 2023 19:27:17 +0900 Subject: [PATCH 1/2] Allow for reading and mutating a span name, similar to otel spec **Motivation:** The feature frozen otel specification allows for renaming spans after they have been started. This often is too late to impact sampling decisions, however we still can use to to provide better names for spans as time goes on. **Modifications:** add an operationName to the Span protocol. A span has to be thread-safe already and we have mutating operations on it already so it does not change the complexity requirements on implementing a span. **Result:** Be closer to otel spec and more flexible in naming and renaming spans. Resolves https://github.com/apple/swift-distributed-tracing/issues/48 --- Sources/Tracing/NoOpTracer.swift | 15 +++++++++++++-- Sources/Tracing/Span.swift | 17 +++++++++++++++++ .../DynamicTracepointTracerTests.swift | 4 ++-- Tests/TracingTests/TestTracer.swift | 2 +- Tests/TracingTests/TracedLockTests.swift | 2 +- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/Sources/Tracing/NoOpTracer.swift b/Sources/Tracing/NoOpTracer.swift index 97bfc35e..a761d2be 100644 --- a/Sources/Tracing/NoOpTracer.swift +++ b/Sources/Tracing/NoOpTracer.swift @@ -29,7 +29,7 @@ public struct NoOpTracer: Tracer { file fileID: String, line: UInt ) -> Span { - NoOpSpan(baggage: baggage) + NoOpSpan(operationName: operationName, baggage: baggage) } public func forceFlush() {} @@ -50,7 +50,18 @@ public struct NoOpTracer: Tracer { public let baggage: Baggage public let isRecording = false - public init(baggage: Baggage) { + public let _operationName: String + public var operationName: String { + get { + self._operationName + } + set { + // ignore + } + } + + public init(operationName: String, baggage: Baggage) { + self._operationName = operationName self.baggage = baggage } diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index ad15b2c3..7efdf678 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -30,6 +30,23 @@ public protocol Span: AnyObject, _SwiftTracingSendableSpan { /// The read-only `Baggage` of this `Span`, set when starting this `Span`. var baggage: Baggage { get } + /// Returns the name of the operation this span represents. + /// + /// The name may be changed during the lifetime of a `Span`, this change + /// may or may not impact the sampling decision and actually emitting the span, + /// depending on how a backend decides to treat renames. + /// + /// This can still be useful when, for example, we want to immediately start + /// span when receiving a request but make it more precise as handling of the request proceeds. + /// For example, we can start a span immediately when a request is received in a server, + /// and update it to reflect the matched route, if it did match one: + /// + /// - 1) Start span with basic path (e.g. `operationName = request.head.uri` during `withSpan`) + /// - 2.1) "Route Not Found" -> Record error + /// - 2.2) "Route Found" -> Rename to route (`/users/1` becomes `/users/:userID`) + /// - 3) End span + var operationName: String { get set } + /// Set the status. /// - Parameter status: The status of this `Span`. func setStatus(_ status: SpanStatus) diff --git a/Tests/TracingTests/DynamicTracepointTracerTests.swift b/Tests/TracingTests/DynamicTracepointTracerTests.swift index 0738b8f1..0a488c0a 100644 --- a/Tests/TracingTests/DynamicTracepointTracerTests.swift +++ b/Tests/TracingTests/DynamicTracepointTracerTests.swift @@ -152,7 +152,7 @@ final class DynamicTracepointTestTracer: Tracer { { let tracepoint = TracepointID(function: function, fileID: fileID, line: line) guard self.shouldRecord(tracepoint: tracepoint) else { - return NoOpTracer.NoOpSpan(baggage: baggage) + return NoOpTracer.NoOpSpan(operationName: operationName, baggage: baggage) } let span = TracepointSpan( @@ -230,7 +230,6 @@ 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 private var status: SpanStatus? @@ -238,6 +237,7 @@ extension DynamicTracepointTestTracer { private let startTime: DispatchWallTime private(set) var endTime: DispatchWallTime? + public var operationName: String private(set) var baggage: Baggage private(set) var isRecording: Bool = false diff --git a/Tests/TracingTests/TestTracer.swift b/Tests/TracingTests/TestTracer.swift index 397809f4..70f5cbe3 100644 --- a/Tests/TracingTests/TestTracer.swift +++ b/Tests/TracingTests/TestTracer.swift @@ -96,7 +96,6 @@ extension Baggage { /// Only intended to be used in single-threaded testing. final class TestSpan: Span { - private let operationName: String private let kind: SpanKind private var status: SpanStatus? @@ -104,6 +103,7 @@ final class TestSpan: Span { private let startTime: DispatchWallTime private(set) var endTime: DispatchWallTime? + var operationName: String let baggage: Baggage private(set) var events = [SpanEvent]() { diff --git a/Tests/TracingTests/TracedLockTests.swift b/Tests/TracingTests/TracedLockTests.swift index e7caee5a..aeb5c3fe 100644 --- a/Tests/TracingTests/TracedLockTests.swift +++ b/Tests/TracingTests/TracedLockTests.swift @@ -98,7 +98,6 @@ private final class TracedLockPrintlnTracer: Tracer { Carrier == Extract.Carrier {} final class TracedLockPrintlnSpan: Span { - private let operationName: String private let kind: SpanKind private var status: SpanStatus? @@ -106,6 +105,7 @@ private final class TracedLockPrintlnTracer: Tracer { private let startTime: DispatchWallTime private(set) var endTime: DispatchWallTime? + var operationName: String let baggage: Baggage private var links = [SpanLink]() From b26e2df23c6f2e18303ea0ff8aa72ad7ac6510b4 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 27 Feb 2023 11:34:57 +0900 Subject: [PATCH 2/2] Update Sources/Tracing/NoOpTracer.swift --- Sources/Tracing/NoOpTracer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Tracing/NoOpTracer.swift b/Sources/Tracing/NoOpTracer.swift index a761d2be..a85ce65a 100644 --- a/Sources/Tracing/NoOpTracer.swift +++ b/Sources/Tracing/NoOpTracer.swift @@ -50,7 +50,7 @@ public struct NoOpTracer: Tracer { public let baggage: Baggage public let isRecording = false - public let _operationName: String + private let _operationName: String public var operationName: String { get { self._operationName