Set url.full attribute on spans logged for HTTP requests

Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+tracing.swift

@@ -27,9 +27,7 @@ extension HTTPClient {
         }
 
         return try await tracer.withSpan(request.method.rawValue, ofKind: .client) { span in
-            let keys = self.configuration.tracing.attributeKeys
-            span.attributes[keys.requestMethod] = request.method.rawValue
-            // TODO: set more attributes on the span
+            TracingSupport.handleRequestTracingAttributes(span, request, configuration: self.tracing)
             let response = try await body()
 
             // set response span attributes

Sources/AsyncHTTPClient/DeconstructedURL.swift

@@ -18,7 +18,8 @@ import struct FoundationEssentials.URL
 import struct Foundation.URL
 #endif
 
-struct DeconstructedURL {
+@usableFromInline
+struct DeconstructedURL: Sendable {
     var scheme: Scheme
     var connectionTarget: ConnectionTarget
     var uri: String
@@ -42,6 +43,7 @@ extension DeconstructedURL {
         try self.init(url: url)
     }
 
+    @usableFromInline
     init(url: URL) throws {
         guard let schemeString = url.scheme else {
             throw HTTPClientError.emptyScheme

Sources/AsyncHTTPClient/HTTPClient.swift

@@ -1118,6 +1118,16 @@ public final class HTTPClient: Sendable {
         @usableFromInline
         package var attributeKeys: AttributeKeys
 
+        /// Whether to record the full request URL (`url.full`) and query string (`url.query`) on emitted spans.
+        ///
+        /// Since the full request URL and query string may contain sensitive information, they are not recorded by default.
+        public var recordFullURL: Bool = false
+
+        /// Whether to record the request path (`url.path`) on emitted spans.
+        ///
+        /// Disable if your service includes sensitive information in the URL path that should not be recorded.
+        public var recordURLPath: Bool = true
+
         public init() {
             if #available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) {
                 self._tracer = InstrumentationSystem.tracer
@@ -1128,7 +1138,7 @@ public final class HTTPClient: Sendable {
         }
 
         /// Span attribute keys that the HTTPClient should set automatically.
-        /// This struct allows the configuration of the attribute names (keys) which will be used for the apropriate values.
+        /// This struct allows the configuration of the attribute names (keys) which will be used for the appropriate values.
         @usableFromInline
         package struct AttributeKeys: Sendable {
             @usableFromInline package var requestMethod: String = "http.request.method"
@@ -1139,6 +1149,14 @@ public final class HTTPClient: Sendable {
 
             @usableFromInline package var httpFlavor: String = "http.flavor"
 
+            @usableFromInline package var serverAddress: String = "server.address"
+            @usableFromInline package var serverPort: String = "server.port"
+
+            @usableFromInline package var urlScheme: String = "url.scheme"
+            @usableFromInline package var urlPath: String = "url.path"
+            @usableFromInline package var urlQuery: String = "url.query"
+            @usableFromInline package var fullUrl: String = "url.full"
+
             @usableFromInline package init() {}
         }
     }

Sources/AsyncHTTPClient/HTTPHandler.swift

@@ -238,6 +238,7 @@ extension HTTPClient {
         public var tlsConfiguration: TLSConfiguration?
 
         /// Parsed, validated and deconstructed URL.
+        @usableFromInline
         let deconstructedURL: DeconstructedURL
 
         /// Create HTTP request.

Sources/AsyncHTTPClient/RequestBag+Tracing.swift

@@ -34,6 +34,9 @@ extension RequestBag.LoopBoundState {
             "Unexpected active span when starting new request span! Was: \(String(describing: self.activeSpan))"
         )
         self.activeSpan = tracer.startSpan("\(request.method)", ofKind: .client)
+        if let activeSpan {
+            TracingSupport.handleRequestTracingAttributes(activeSpan, request, configuration: tracing)
+        }
     }
 
     /// Fails the active overall span given some internal error, e.g. timeout, pool shutdown etc.

Sources/AsyncHTTPClient/TracingSupport.swift

@@ -19,10 +19,95 @@ import NIOHTTP1
 import NIOSSL
 import Tracing
 
+#if canImport(FoundationEssentials)
+import FoundationEssentials
+#else
+import Foundation
+#endif
+
 // MARK: - Centralized span attribute handling
 
 @usableFromInline
 struct TracingSupport {
+    @inlinable
+    @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
+    static func handleRequestTracingAttributes(
+        _ span: Span,
+        _ request: HTTPClientRequest,
+        configuration: HTTPClient.TracingConfiguration
+    ) {
+        let requestBodySize: Int? =
+            switch request.body?.mode {
+            case .asyncSequence(.known(let length), _):
+                Int(length)
+            case .asyncSequence(.unknown, _):
+                nil
+            case .sequence(.known(let length), _, _):
+                Int(length)
+            case .sequence(.unknown, _, _):
+                nil
+            case .byteBuffer(let byteBuffer):
+                byteBuffer.readableBytes
+            case nil:
+                nil
+            }
+        let url = URL(string: request.url)
+        let deconstructedURL: DeconstructedURL? =
+            if let url {
+                try? DeconstructedURL(url: url)
+            } else {
+                nil
+            }
+        handleRequestTracingAttributes(
+            span,
+            requestMethod: request.method.rawValue,
+            url: url,
+            deconstructedURL: deconstructedURL,
+            requestBodySize: requestBodySize,
+            configuration: configuration
+        )
+    }
+
+    @inlinable
+    static func handleRequestTracingAttributes(
+        _ span: Span,
+        _ request: HTTPClient.Request,
+        configuration: HTTPClient.TracingConfiguration
+    ) {
+        handleRequestTracingAttributes(
+            span,
+            requestMethod: request.method.rawValue,
+            url: request.url,
+            deconstructedURL: request.deconstructedURL,
+            requestBodySize: request.body?.contentLength.map { Int($0) },
+            configuration: configuration
+        )
+    }
+
+    @usableFromInline
+    static func handleRequestTracingAttributes(
+        _ span: Span,
+        requestMethod: String,
+        url: URL?,
+        deconstructedURL: DeconstructedURL?,
+        requestBodySize: Int?,
+        configuration: HTTPClient.TracingConfiguration
+    ) {
+        let keys = configuration.attributeKeys
+        span.attributes[keys.requestMethod] = requestMethod
+        span.attributes[keys.urlScheme] = deconstructedURL?.scheme.rawValue
+        span.attributes[keys.serverAddress] = deconstructedURL?.connectionTarget.host
+        span.attributes[keys.serverPort] = deconstructedURL?.connectionTarget.port
+        span.attributes[keys.requestBodySize] = requestBodySize
+        if configuration.recordURLPath {
+            span.attributes[keys.urlPath] = url?.path
+        }
+        if configuration.recordFullURL {
+            span.attributes[keys.fullUrl] = url?.absoluteString
+            span.attributes[keys.urlQuery] = url?.query
+        }
+    }
+
     @inlinable
     static func handleResponseStatusCode(
         _ span: Span,

Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift

@@ -33,8 +33,8 @@ import Network
 import NIOTransportServices
 #endif
 
-private func makeTracedHTTPClient(tracer: InMemoryTracer) -> HTTPClient {
-    var config = HTTPClient.Configuration()
+private func makeTracedHTTPClient(tracer: InMemoryTracer, configuration: HTTPClient.Configuration) -> HTTPClient {
+    var config = configuration
     config.httpVersion = .automatic
     config.tracing.tracer = tracer
     return HTTPClient(
@@ -51,7 +51,7 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
     override func setUp() {
         super.setUp()
         self.tracer = InMemoryTracer()
-        self.client = makeTracedHTTPClient(tracer: tracer)
+        self.client = makeTracedHTTPClient(tracer: tracer, configuration: HTTPClient.Configuration())
     }
 
     override func tearDown() {
@@ -70,12 +70,22 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
             XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
             return
         }
-        guard let span = tracer.finishedSpans.first else {
-            XCTFail("No span was recorded!")
-            return
-        }
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
 
         XCTAssertEqual(span.operationName, "GET")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlScheme), "http")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
+        XCTAssertNotNil(span.attributes.get(client.tracing.attributeKeys.serverAddress))
+        XCTAssertEqual(
+            span.attributes.get(client.tracing.attributeKeys.serverPort),
+            SpanAttribute.int64(Int64(self.defaultHTTPBin.port))
+        )
+        XCTAssertNil(
+            span.attributes.get(client.tracing.attributeKeys.fullUrl),
+            "url.full must not be set unless tracing.recordFullURL is enabled"
+        )
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
     }
 
     func testTrace_post_sync() throws {
@@ -86,12 +96,14 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
             XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
             return
         }
-        guard let span = tracer.finishedSpans.first else {
-            XCTFail("No span was recorded!")
-            return
-        }
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
 
         XCTAssertEqual(span.operationName, "POST")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "POST")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlScheme), "http")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
+        XCTAssertNil(span.attributes.get(client.tracing.attributeKeys.fullUrl))
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
     }
 
     func testTrace_post_sync_404_error() throws {
@@ -102,10 +114,7 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
             XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
             return
         }
-        guard let span = tracer.finishedSpans.first else {
-            XCTFail("No span was recorded!")
-            return
-        }
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
 
         XCTAssertEqual(span.operationName, "POST")
         XCTAssertTrue(span.errors.isEmpty, "Should have recorded error")
@@ -121,12 +130,16 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
             XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
             return
         }
-        guard let span = tracer.finishedSpans.first else {
-            XCTFail("No span was recorded!")
-            return
-        }
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
 
         XCTAssertEqual(span.operationName, "GET")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlScheme), "http")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
+        XCTAssertNotNil(span.attributes.get(client.tracing.attributeKeys.serverAddress))
+        XCTAssertNotNil(span.attributes.get(client.tracing.attributeKeys.serverPort))
+        XCTAssertNil(span.attributes.get(client.tracing.attributeKeys.fullUrl))
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
     }
 
     func testTrace_execute_async_404_error() async throws {
@@ -138,13 +151,64 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
             XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
             return
         }
-        guard let span = tracer.finishedSpans.first else {
-            XCTFail("No span was recorded!")
-            return
-        }
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
 
         XCTAssertEqual(span.operationName, "GET")
         XCTAssertTrue(span.errors.isEmpty, "Should have recorded error")
         XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.responseStatusCode), 404)
     }
+
+    func testTrace_record_request_body_size_async() async throws {
+        let url = self.defaultHTTPBinURLPrefix + "echo-method"
+        var request = HTTPClientRequest(url: url)
+        request.body = .bytes(ByteBuffer(string: "test"))
+        let _ = try await client.execute(request, deadline: .distantFuture)
+
+        guard tracer.activeSpans.isEmpty else {
+            XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
+            return
+        }
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
+
+        XCTAssertEqual(span.operationName, "GET")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
+        XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), 4)
+    }
+
+    func testTrace_records_full_url_when_opted_in_async() async throws {
+        var config = HTTPClient.Configuration()
+        config.tracing.recordFullURL = true
+        let optedInClient = makeTracedHTTPClient(tracer: tracer, configuration: config)
+        defer { XCTAssertNoThrow(try optedInClient.syncShutdown()) }
+
+        let url = self.defaultHTTPBinURLPrefix + "echo-method?token=secret"
+        let request = HTTPClientRequest(url: url)
+        _ = try await optedInClient.execute(request, deadline: .distantFuture)
+
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
+
+        XCTAssertEqual(span.attributes.get(optedInClient.tracing.attributeKeys.fullUrl), "\(url)")
+        XCTAssertEqual(span.attributes.get(optedInClient.tracing.attributeKeys.urlQuery), "token=secret")
+        XCTAssertEqual(span.attributes.get(optedInClient.tracing.attributeKeys.urlPath), "/echo-method")
+    }
+
+    func testTrace_does_not_record_url_path_when_opted_out_async() async throws {
+        var config = HTTPClient.Configuration()
+        config.tracing.recordURLPath = false
+        let optedOutClient = makeTracedHTTPClient(tracer: tracer, configuration: config)
+        defer { XCTAssertNoThrow(try optedOutClient.syncShutdown()) }
+
+        let url = self.defaultHTTPBinURLPrefix + "echo-method"
+        let request = HTTPClientRequest(url: url)
+        _ = try await optedOutClient.execute(request, deadline: .distantFuture)
+
+        let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
+
+        XCTAssertNil(
+            span.attributes.get(optedOutClient.tracing.attributeKeys.urlPath),
+            "url.path must not be set when tracing.recordURLPath is disabled"
+        )
+        XCTAssertEqual(span.attributes.get(optedOutClient.tracing.attributeKeys.urlScheme), "http")
+    }
 }