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") + } }