Repository: apple/swift-distributed-tracing (swift)

Issue:
Span.recordError should take optional SpanAttributes
[The OTel specification states](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#record-exception) that Span’s `recordException` (in our case `recordError`) should take an additional argument to optionally specify `SpanAttributes`. These additional attributes (in case of OTel) are then merged into the resulting event’s attributes.

> If RecordException is provided, the method MUST accept an optional parameter to provide any additional event attributes (this SHOULD be done in the same way as for the AddEvent method). If attributes with the same name would be generated by the method already, the additional attributes take precedence.

Pull request:
**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

resolves https://github.com/apple/swift-distributed-tracing/issues/45

Patch:
diff --git a/Sources/Tracing/NoOpTracer.swift b/Sources/Tracing/NoOpTracer.swift
index a85ce65a..8d18a874 100644
--- a/Sources/Tracing/NoOpTracer.swift
+++ b/Sources/Tracing/NoOpTracer.swift
@@ -71,7 +71,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 7efdf678..ff271a78 100644
--- a/Sources/Tracing/Span.swift
+++ b/Sources/Tracing/Span.swift
@@ -59,7 +59,8 @@ public protocol Span: AnyObject, _SwiftTracingSendableSpan {
     ///
     /// - Parameters:
     ///   - error: The error to be recorded.
-    func recordError(_ error: Error)
+    ///   - attributes: Additional attributes describing the error.
+    func recordError(_ error: Error, attributes: SpanAttributes)
 
     /// The attributes describing this `Span`.
     var attributes: SpanAttributes { get set }
@@ -114,6 +115,16 @@ 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: [:])
+    }
+}
+
 // ==== ----------------------------------------------------------------------------------------------------------------
 // MARK: Span Event
 


Test patch:
diff --git a/Tests/TracingTests/DynamicTracepointTracerTests.swift b/Tests/TracingTests/DynamicTracepointTracerTests.swift
index 0a488c0a..1291531b 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 70f5cbe3..1479e89e 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,
@@ -103,6 +103,8 @@ final class TestSpan: Span {
     private let startTime: DispatchWallTime
     private(set) var endTime: DispatchWallTime?
 
+    private(set) var recordedErrors: [(Error, SpanAttributes)] = []
+
     var operationName: String
     let baggage: Baggage
 
@@ -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 aeb5c3fe..3a5deddb 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 360df370..1e6bca48 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 987f3e76..fa65cefc 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).