From 387bf2f596914d976ec29967557e69249f8031ce Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Fri, 24 Feb 2023 20:23:32 +0900 Subject: [PATCH 1/3] Recording an error may carry attributes **Motivation:** It can be used to attach more information bout the error, like its stacktrace and similar information. This is also supported by the otel spec, so we're aligning closer with it here. **Modifications:** Change the recordError to accept optional span attributes **Result:** more powerful record error API --- Sources/Tracing/NoOpTracer.swift | 2 +- Sources/Tracing/Span.swift | 8 ++++- .../DynamicTracepointTracerTests.swift | 2 +- Tests/TracingTests/TestTracer.swift | 12 ++++--- Tests/TracingTests/TracedLockTests.swift | 2 +- Tests/TracingTests/TracerTests+XCTest.swift | 1 + Tests/TracingTests/TracerTests.swift | 31 +++++++++++++++++++ 7 files changed, 50 insertions(+), 8 deletions(-) diff --git a/Sources/Tracing/NoOpTracer.swift b/Sources/Tracing/NoOpTracer.swift index 97bfc35..5ac2992 100644 --- a/Sources/Tracing/NoOpTracer.swift +++ b/Sources/Tracing/NoOpTracer.swift @@ -60,7 +60,7 @@ public struct NoOpTracer: Tracer { public func addEvent(_ event: SpanEvent) {} - public func recordError(_ error: Error) {} + public func recordError(_ error: Error, attributes: SpanAttributes) {} public var attributes: SpanAttributes { get { diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index ad15b2c..0f68427 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -42,7 +42,7 @@ public protocol Span: AnyObject, _SwiftTracingSendableSpan { /// /// - Parameters: /// - error: The error to be recorded. - func recordError(_ error: Error) + func recordError(_ error: Error, attributes: SpanAttributes) /// The attributes describing this `Span`. var attributes: SpanAttributes { get set } @@ -97,6 +97,12 @@ extension Span { } } +extension Span { + public func recordError(_ error: Error) { + self.recordError(error, attributes: [:]) + } +} + // ==== ---------------------------------------------------------------------------------------------------------------- // MARK: Span Event diff --git a/Tests/TracingTests/DynamicTracepointTracerTests.swift b/Tests/TracingTests/DynamicTracepointTracerTests.swift index 0738b8f..515ca9e 100644 --- a/Tests/TracingTests/DynamicTracepointTracerTests.swift +++ b/Tests/TracingTests/DynamicTracepointTracerTests.swift @@ -276,7 +276,7 @@ extension DynamicTracepointTestTracer { // nothing } - func recordError(_ error: Error) { + func recordError(_ error: Error, attributes: SpanAttributes) { print("") } diff --git a/Tests/TracingTests/TestTracer.swift b/Tests/TracingTests/TestTracer.swift index 397809f..f11c715 100644 --- a/Tests/TracingTests/TestTracer.swift +++ b/Tests/TracingTests/TestTracer.swift @@ -21,7 +21,7 @@ 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 } + var onEndSpan: (TestSpan) -> Void = { _ in } func startSpan( _ operationName: String, @@ -104,6 +104,8 @@ final class TestSpan: Span { private let startTime: DispatchWallTime private(set) var endTime: DispatchWallTime? + private(set) var recordedErrors: [(Error, SpanAttributes)] = [] + let baggage: Baggage private(set) var events = [SpanEvent]() { @@ -122,14 +124,14 @@ final class TestSpan: Span { private(set) var isRecording = false - let onEnd: (Span) -> Void + let onEnd: (TestSpan) -> Void init( operationName: String, startTime: DispatchWallTime, baggage: Baggage, kind: SpanKind, - onEnd: @escaping (Span) -> Void + onEnd: @escaping (TestSpan) -> Void ) { self.operationName = operationName self.startTime = startTime @@ -151,7 +153,9 @@ final class TestSpan: Span { self.events.append(event) } - func recordError(_ error: Error) {} + func recordError(_ error: Error, attributes: SpanAttributes) { + self.recordedErrors.append((error, attributes)) + } func end(at time: DispatchWallTime) { self.endTime = time diff --git a/Tests/TracingTests/TracedLockTests.swift b/Tests/TracingTests/TracedLockTests.swift index e7caee5..fda0373 100644 --- a/Tests/TracingTests/TracedLockTests.swift +++ b/Tests/TracingTests/TracedLockTests.swift @@ -151,7 +151,7 @@ private final class TracedLockPrintlnTracer: Tracer { self.events.append(event) } - func recordError(_ error: Error) {} + func recordError(_ error: Error, attributes: SpanAttributes) {} func end(at time: DispatchWallTime) { self.endTime = time diff --git a/Tests/TracingTests/TracerTests+XCTest.swift b/Tests/TracingTests/TracerTests+XCTest.swift index 360df37..1e6bca4 100644 --- a/Tests/TracingTests/TracerTests+XCTest.swift +++ b/Tests/TracingTests/TracerTests+XCTest.swift @@ -35,6 +35,7 @@ extension TracerTests { ("testWithSpan_automaticBaggagePropagation_async", testWithSpan_automaticBaggagePropagation_async), ("testWithSpan_enterFromNonAsyncCode_passBaggage_asyncOperation", testWithSpan_enterFromNonAsyncCode_passBaggage_asyncOperation), ("testWithSpan_automaticBaggagePropagation_async_throws", testWithSpan_automaticBaggagePropagation_async_throws), + ("testWithSpan_recordErrorWithAttributes", testWithSpan_recordErrorWithAttributes), ] } } diff --git a/Tests/TracingTests/TracerTests.swift b/Tests/TracingTests/TracerTests.swift index 987f3e7..fa65cef 100644 --- a/Tests/TracingTests/TracerTests.swift +++ b/Tests/TracingTests/TracerTests.swift @@ -253,6 +253,37 @@ final class TracerTests: XCTestCase { #endif } + func testWithSpan_recordErrorWithAttributes() throws { + #if swift(>=5.5) && canImport(_Concurrency) + guard #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) else { + throw XCTSkip("Task locals are not supported on this platform.") + } + + let tracer = TestTracer() + InstrumentationSystem.bootstrapInternal(tracer) + defer { + InstrumentationSystem.bootstrapInternal(nil) + } + + var endedSpan: TestSpan? + tracer.onEndSpan = { span in endedSpan = span } + + let errorToThrow = ExampleSpanError() + let attrsForError: SpanAttributes = ["attr": "value"] + + tracer.withSpan("hello") { span in + span.recordError(errorToThrow, attributes: attrsForError) + } + + XCTAssertTrue(endedSpan != nil) + XCTAssertEqual(endedSpan!.recordedErrors.count, 1) + let error = endedSpan!.recordedErrors.first!.0 + XCTAssertEqual(error as! ExampleSpanError, errorToThrow) + let attrs = endedSpan!.recordedErrors.first!.1 + XCTAssertEqual(attrs, attrsForError) + #endif + } + #if swift(>=5.5) && canImport(_Concurrency) @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) /// Helper method to execute async operations until we can use async tests (currently incompatible with the generated LinuxMain file). From db2b5ea9f505a82413809c4cd9ae7c3cf9287861 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 7 Mar 2023 12:43:01 +0900 Subject: [PATCH 2/3] Update Sources/Tracing/Span.swift Co-authored-by: Moritz Lang --- Sources/Tracing/Span.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index 0f68427..941a2c4 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -42,6 +42,7 @@ public protocol Span: AnyObject, _SwiftTracingSendableSpan { /// /// - Parameters: /// - error: The error to be recorded. + /// - attributes: Additional attributes describing the error. func recordError(_ error: Error, attributes: SpanAttributes) /// The attributes describing this `Span`. From 6108afbfd733732953a438f3084e981b242741bc Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 7 Mar 2023 12:43:08 +0900 Subject: [PATCH 3/3] Update Sources/Tracing/Span.swift Co-authored-by: Moritz Lang --- Sources/Tracing/Span.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/Tracing/Span.swift b/Sources/Tracing/Span.swift index 941a2c4..ab3d2bd 100644 --- a/Sources/Tracing/Span.swift +++ b/Sources/Tracing/Span.swift @@ -99,6 +99,10 @@ extension Span { } extension Span { + /// Record a failure described by the given error. + /// + /// - Parameters: + /// - error: The error to be recorded. public func recordError(_ error: Error) { self.recordError(error, attributes: [:]) }