Skip to content

Set url.full attribute on spans logged for HTTP requests #906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ahoppen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Set url.full attribute on spans logged for HTTP requests #906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+tracing.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, keys: tracing.attributeKeys)
let response = try await body()

// set response span attributes
Expand Down
4 changes: 3 additions & 1 deletion Sources/AsyncHTTPClient/HTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,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"
Expand All @@ -1139,6 +1139,8 @@ public final class HTTPClient: Sendable {

@usableFromInline package var httpFlavor: String = "http.flavor"

@usableFromInline package var fullUrl: String = "url.full"

@usableFromInline package init() {}
}
}
Expand Down
3 changes: 3 additions & 0 deletions Sources/AsyncHTTPClient/RequestBag+Tracing.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, keys: tracing.attributeKeys)
}
}

/// Fails the active overall span given some internal error, e.g. timeout, pool shutdown etc.
Expand Down
64 changes: 64 additions & 0 deletions Sources/AsyncHTTPClient/TracingSupport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,74 @@ 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,
keys: HTTPClient.TracingConfiguration.AttributeKeys
) {
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
}
handleRequestTracingAttributes(
span,
requestMethod: request.method.rawValue,
url: request.url,
requestBodySize: requestBodySize,
keys: keys
)
}

static func handleRequestTracingAttributes(
_ span: Span,
_ request: HTTPClient.Request,
keys: HTTPClient.TracingConfiguration.AttributeKeys
) {
handleRequestTracingAttributes(
span,
requestMethod: request.method.rawValue,
url: request.url.description,
Copy link
Copy Markdown
Collaborator

@FranzBusch FranzBusch May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to be careful with that. The full URL can include PII.

Copy link
Copy Markdown
Author

@ahoppen ahoppen May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. What do you think of the following: Only log only the host name by default. I wouldn’t consider that as PII and it might already give a hint as to what's being requested. We then add a configuration option to TracingConfiguration that allows users to opt into recording the entire URL in traces.

Copy link
Copy Markdown
Collaborator

@FranzBusch FranzBusch May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think I would recommend doing this

  1. Emit server.address, server.port, url.scheme, HTTP method, status by default.
  2. Emit url.path by default but document that callers should configure redaction if they hit user-scoped paths.
  3. Make url.full and query string opt-in.

requestBodySize: request.body?.contentLength.map { Int.init($0) },
keys: keys
)
}

@inlinable
static func handleRequestTracingAttributes(
_ span: Span,
requestMethod: String,
url: String,
requestBodySize: Int?,
keys: HTTPClient.TracingConfiguration.AttributeKeys
) {
span.attributes[keys.requestMethod] = requestMethod
span.attributes[keys.fullUrl] = url
span.attributes[keys.requestBodySize] = requestBodySize
}

@inlinable
static func handleResponseStatusCode(
_ span: Span,
Expand Down
30 changes: 30 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
}

XCTAssertEqual(span.operationName, "GET")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
}

func testTrace_post_sync() throws {
Expand All @@ -92,6 +95,9 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
}

XCTAssertEqual(span.operationName, "POST")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "POST")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
}

func testTrace_post_sync_404_error() throws {
Expand Down Expand Up @@ -127,6 +133,9 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
}

XCTAssertEqual(span.operationName, "GET")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
}

func testTrace_execute_async_404_error() async throws {
Expand All @@ -147,4 +156,25 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
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
}
guard let span = tracer.finishedSpans.first else {
XCTFail("No span was recorded!")
return
}

XCTAssertEqual(span.operationName, "GET")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), 4)
}
}
Loading