From 7a0d2f2be388180f4f8aadf89f6becdcb6f80316 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 22 May 2026 08:16:50 +0200 Subject: [PATCH] Set `url.full` attribute on spans logged for HTTP requests We previously only set the request method on these spans, which made it hard to identify which exact HTTP request was causing this span to be emitted. Also record the URL of the HTTP request to add more information to these spans. While at it, also record the request body size because we already had an attribute key configured for it. --- .../AsyncAwait/HTTPClient+tracing.swift | 4 +- .../AsyncHTTPClient/DeconstructedURL.swift | 4 +- Sources/AsyncHTTPClient/HTTPClient.swift | 10 ++- Sources/AsyncHTTPClient/HTTPHandler.swift | 1 + .../AsyncHTTPClient/RequestBag+Tracing.swift | 3 + Sources/AsyncHTTPClient/TracingSupport.swift | 75 +++++++++++++++++++ .../HTTPClientTracingTests.swift | 65 +++++++++++----- 7 files changed, 137 insertions(+), 25 deletions(-) diff --git a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+tracing.swift b/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+tracing.swift index 0be737619..28b6e9cf4 100644 --- a/Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+tracing.swift +++ b/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 diff --git a/Sources/AsyncHTTPClient/DeconstructedURL.swift b/Sources/AsyncHTTPClient/DeconstructedURL.swift index 96a16d87d..bfba474a1 100644 --- a/Sources/AsyncHTTPClient/DeconstructedURL.swift +++ b/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 diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 863874001..cc8792497 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -1143,7 +1143,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" @@ -1154,6 +1154,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() {} } } diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 4c4cc6f9a..3dd08cf99 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/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. diff --git a/Sources/AsyncHTTPClient/RequestBag+Tracing.swift b/Sources/AsyncHTTPClient/RequestBag+Tracing.swift index 5551459d1..de961076d 100644 --- a/Sources/AsyncHTTPClient/RequestBag+Tracing.swift +++ b/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. diff --git a/Sources/AsyncHTTPClient/TracingSupport.swift b/Sources/AsyncHTTPClient/TracingSupport.swift index feb564ffb..c132d2b9f 100644 --- a/Sources/AsyncHTTPClient/TracingSupport.swift +++ b/Sources/AsyncHTTPClient/TracingSupport.swift @@ -19,10 +19,85 @@ 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), _), .sequence(.known(let length), _, _): + Int(length) + case .byteBuffer(let byteBuffer): + byteBuffer.readableBytes + case .asyncSequence(.unknown, _), .sequence(.unknown, _, _), 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 + span.attributes[keys.urlPath] = url?.path + span.attributes[keys.fullUrl] = url?.absoluteString + span.attributes[keys.urlQuery] = url?.query + } + @inlinable static func handleResponseStatusCode( _ span: Span, diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift index 7e8d09d1e..3f145ce1e 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift @@ -70,12 +70,19 @@ 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)) + ) + XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)") + XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil) } func testTrace_post_sync() throws { @@ -86,12 +93,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") + XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)") + XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil) } func testTrace_post_sync_404_error() throws { @@ -102,10 +111,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 +127,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)) + XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)") + XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil) } func testTrace_execute_async_404_error() async throws { @@ -138,13 +148,28 @@ 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) + } }