From 6eccbcc7c59b6d96a23807f667fc2375ffe4cb4d 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 | 20 +++- Sources/AsyncHTTPClient/HTTPHandler.swift | 1 + .../AsyncHTTPClient/RequestBag+Tracing.swift | 3 + Sources/AsyncHTTPClient/TracingSupport.swift | 85 ++++++++++++++ .../HTTPClientTracingTests.swift | 110 ++++++++++++++---- 7 files changed, 199 insertions(+), 28 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 dbc40984f..819897947 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/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() {} } } 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..feedb7294 100644 --- a/Sources/AsyncHTTPClient/TracingSupport.swift +++ b/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, diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift index 7e8d09d1e..31c192e64 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift +++ b/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") + } }