Skip to content

Commit f95c908

Browse files
ahoppenahoppen
authored
Set url.full attribute on spans logged for HTTP requests (#906)
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.
1 parent 69c6cf0 commit f95c908

7 files changed

Lines changed: 186 additions & 25 deletions

File tree

‎Sources/AsyncHTTPClient/AsyncAwait/HTTPClient+tracing.swift‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ extension HTTPClient {
2727
}
2828

2929
return try await tracer.withSpan(request.method.rawValue, ofKind: .client) { span in
30-
let keys = self.configuration.tracing.attributeKeys
31-
span.attributes[keys.requestMethod] = request.method.rawValue
32-
// TODO: set more attributes on the span
30+
TracingSupport.handleRequestTracingAttributes(span, request, configuration: self.tracing)
3331
let response = try await body()
3432

3533
// set response span attributes

‎Sources/AsyncHTTPClient/DeconstructedURL.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import struct FoundationEssentials.URL
1818
import struct Foundation.URL
1919
#endif
2020

21-
struct DeconstructedURL {
21+
@usableFromInline
22+
struct DeconstructedURL: Sendable {
2223
var scheme: Scheme
2324
var connectionTarget: ConnectionTarget
2425
var uri: String
@@ -42,6 +43,7 @@ extension DeconstructedURL {
4243
try self.init(url: url)
4344
}
4445

46+
@usableFromInline
4547
init(url: URL) throws {
4648
guard let schemeString = url.scheme else {
4749
throw HTTPClientError.emptyScheme

‎Sources/AsyncHTTPClient/HTTPClient.swift‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1143,7 +1143,7 @@ public final class HTTPClient: Sendable {
11431143
}
11441144

11451145
/// Span attribute keys that the HTTPClient should set automatically.
1146-
/// This struct allows the configuration of the attribute names (keys) which will be used for the apropriate values.
1146+
/// This struct allows the configuration of the attribute names (keys) which will be used for the appropriate values.
11471147
@usableFromInline
11481148
package struct AttributeKeys: Sendable {
11491149
@usableFromInline package var requestMethod: String = "http.request.method"
@@ -1154,6 +1154,14 @@ public final class HTTPClient: Sendable {
11541154

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

1157+
@usableFromInline package var serverAddress: String = "server.address"
1158+
@usableFromInline package var serverPort: String = "server.port"
1159+
1160+
@usableFromInline package var urlScheme: String = "url.scheme"
1161+
@usableFromInline package var urlPath: String = "url.path"
1162+
@usableFromInline package var urlQuery: String = "url.query"
1163+
@usableFromInline package var fullUrl: String = "url.full"
1164+
11571165
@usableFromInline package init() {}
11581166
}
11591167
}

‎Sources/AsyncHTTPClient/HTTPHandler.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ extension HTTPClient {
238238
public var tlsConfiguration: TLSConfiguration?
239239

240240
/// Parsed, validated and deconstructed URL.
241+
@usableFromInline
241242
let deconstructedURL: DeconstructedURL
242243

243244
/// Create HTTP request.

‎Sources/AsyncHTTPClient/RequestBag+Tracing.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ extension RequestBag.LoopBoundState {
3434
"Unexpected active span when starting new request span! Was: \(String(describing: self.activeSpan))"
3535
)
3636
self.activeSpan = tracer.startSpan("\(request.method)", ofKind: .client)
37+
if let activeSpan {
38+
TracingSupport.handleRequestTracingAttributes(activeSpan, request, configuration: tracing)
39+
}
3740
}
3841

3942
/// Fails the active overall span given some internal error, e.g. timeout, pool shutdown etc.

‎Sources/AsyncHTTPClient/TracingSupport.swift‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,91 @@ import NIOHTTP1
1919
import NIOSSL
2020
import Tracing
2121

22+
#if canImport(FoundationEssentials)
23+
import FoundationEssentials
24+
#else
25+
import Foundation
26+
#endif
27+
2228
// MARK: - Centralized span attribute handling
2329

2430
@usableFromInline
2531
struct TracingSupport {
32+
@inlinable
33+
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
34+
static func handleRequestTracingAttributes(
35+
_ span: Span,
36+
_ request: HTTPClientRequest,
37+
configuration: HTTPClient.TracingConfiguration
38+
) {
39+
let requestBodySize: Int? =
40+
switch request.body?.mode {
41+
case .asyncSequence(.known(let length), _), .sequence(.known(let length), _, _):
42+
Int(length)
43+
case .byteBuffer(let byteBuffer):
44+
byteBuffer.readableBytes
45+
case .asyncSequence(.unknown, _), .sequence(.unknown, _, _), nil:
46+
nil
47+
#if UnstableHTTPAPIsSupport
48+
case .httpClientRequestBody(.known(let length), startUpload: _):
49+
Int(length)
50+
case .httpClientRequestBody(.unknown, startUpload: _):
51+
nil
52+
#endif
53+
}
54+
let url = URL(string: request.url)
55+
let deconstructedURL: DeconstructedURL? =
56+
if let url {
57+
try? DeconstructedURL(url: url)
58+
} else {
59+
nil
60+
}
61+
handleRequestTracingAttributes(
62+
span,
63+
requestMethod: request.method.rawValue,
64+
url: url,
65+
deconstructedURL: deconstructedURL,
66+
requestBodySize: requestBodySize,
67+
configuration: configuration
68+
)
69+
}
70+
71+
@inlinable
72+
static func handleRequestTracingAttributes(
73+
_ span: Span,
74+
_ request: HTTPClient.Request,
75+
configuration: HTTPClient.TracingConfiguration
76+
) {
77+
handleRequestTracingAttributes(
78+
span,
79+
requestMethod: request.method.rawValue,
80+
url: request.url,
81+
deconstructedURL: request.deconstructedURL,
82+
requestBodySize: request.body?.contentLength.map { Int($0) },
83+
configuration: configuration
84+
)
85+
}
86+
87+
@usableFromInline
88+
static func handleRequestTracingAttributes(
89+
_ span: Span,
90+
requestMethod: String,
91+
url: URL?,
92+
deconstructedURL: DeconstructedURL?,
93+
requestBodySize: Int?,
94+
configuration: HTTPClient.TracingConfiguration
95+
) {
96+
let keys = configuration.attributeKeys
97+
span.attributes[keys.requestMethod] = requestMethod
98+
span.attributes[keys.urlScheme] = deconstructedURL?.scheme.rawValue
99+
span.attributes[keys.serverAddress] = deconstructedURL?.connectionTarget.host
100+
span.attributes[keys.serverPort] = deconstructedURL?.connectionTarget.port
101+
span.attributes[keys.requestBodySize] = requestBodySize
102+
span.attributes[keys.urlPath] = url?.path
103+
span.attributes[keys.fullUrl] = url?.stringWithUserAndPasswordStripped
104+
span.attributes[keys.urlQuery] = url?.query
105+
}
106+
26107
@inlinable
27108
static func handleResponseStatusCode(
28109
_ span: Span,
@@ -51,3 +132,26 @@ struct HTTPHeadersInjector: Injector, @unchecked Sendable {
51132
// MARK: - Errors
52133

53134
internal struct HTTPRequestCancellationError: Error {}
135+
136+
extension URL {
137+
/// Returns the absolute string of `url` with any embedded credentials (username and password) removed to avoid logging secrets.
138+
fileprivate var stringWithUserAndPasswordStripped: String? {
139+
if #available(macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *) {
140+
guard self.user() != nil || self.password() != nil else {
141+
return self.absoluteString
142+
}
143+
} else {
144+
guard self.user != nil || self.password != nil else {
145+
return self.absoluteString
146+
}
147+
}
148+
guard var components = URLComponents(url: self, resolvingAgainstBaseURL: true) else {
149+
// Should never happen because URL should be well-formed. If it is not, be defensive and avoid logging the URL instead of logging
150+
// username + password.
151+
return nil
152+
}
153+
components.user = nil
154+
components.password = nil
155+
return components.string
156+
}
157+
}

‎Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift‎

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,19 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
7070
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
7171
return
7272
}
73-
guard let span = tracer.finishedSpans.first else {
74-
XCTFail("No span was recorded!")
75-
return
76-
}
73+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
7774

7875
XCTAssertEqual(span.operationName, "GET")
76+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
77+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlScheme), "http")
78+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
79+
XCTAssertNotNil(span.attributes.get(client.tracing.attributeKeys.serverAddress))
80+
XCTAssertEqual(
81+
span.attributes.get(client.tracing.attributeKeys.serverPort),
82+
SpanAttribute.int64(Int64(self.defaultHTTPBin.port))
83+
)
84+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
85+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
7986
}
8087

8188
func testTrace_post_sync() throws {
@@ -86,12 +93,14 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
8693
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
8794
return
8895
}
89-
guard let span = tracer.finishedSpans.first else {
90-
XCTFail("No span was recorded!")
91-
return
92-
}
96+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
9397

9498
XCTAssertEqual(span.operationName, "POST")
99+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "POST")
100+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlScheme), "http")
101+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
102+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
103+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
95104
}
96105

97106
func testTrace_post_sync_404_error() throws {
@@ -102,10 +111,7 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
102111
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
103112
return
104113
}
105-
guard let span = tracer.finishedSpans.first else {
106-
XCTFail("No span was recorded!")
107-
return
108-
}
114+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
109115

110116
XCTAssertEqual(span.operationName, "POST")
111117
XCTAssertTrue(span.errors.isEmpty, "Should have recorded error")
@@ -121,12 +127,16 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
121127
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
122128
return
123129
}
124-
guard let span = tracer.finishedSpans.first else {
125-
XCTFail("No span was recorded!")
126-
return
127-
}
130+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
128131

129132
XCTAssertEqual(span.operationName, "GET")
133+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
134+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlScheme), "http")
135+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
136+
XCTAssertNotNil(span.attributes.get(client.tracing.attributeKeys.serverAddress))
137+
XCTAssertNotNil(span.attributes.get(client.tracing.attributeKeys.serverPort))
138+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(url)")
139+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), nil)
130140
}
131141

132142
func testTrace_execute_async_404_error() async throws {
@@ -138,13 +148,48 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
138148
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
139149
return
140150
}
141-
guard let span = tracer.finishedSpans.first else {
142-
XCTFail("No span was recorded!")
143-
return
144-
}
151+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
145152

146153
XCTAssertEqual(span.operationName, "GET")
147154
XCTAssertTrue(span.errors.isEmpty, "Should have recorded error")
148155
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.responseStatusCode), 404)
149156
}
157+
158+
func testTrace_record_request_body_size_async() async throws {
159+
let url = self.defaultHTTPBinURLPrefix + "echo-method"
160+
var request = HTTPClientRequest(url: url)
161+
request.body = .bytes(ByteBuffer(string: "test"))
162+
let _ = try await client.execute(request, deadline: .distantFuture)
163+
164+
guard tracer.activeSpans.isEmpty else {
165+
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
166+
return
167+
}
168+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
169+
170+
XCTAssertEqual(span.operationName, "GET")
171+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestMethod), "GET")
172+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
173+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.requestBodySize), 4)
174+
}
175+
176+
func testTrace_strips_credentials_from_full_url_async() async throws {
177+
let urlWithoutCredentials = self.defaultHTTPBinURLPrefix + "echo-method"
178+
let urlWithCredentials = urlWithoutCredentials.replacingOccurrences(
179+
of: "http://",
180+
with: "http://user:password@"
181+
)
182+
let request = HTTPClientRequest(url: urlWithCredentials)
183+
let _ = try await client.execute(request, deadline: .distantFuture)
184+
185+
guard tracer.activeSpans.isEmpty else {
186+
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
187+
return
188+
}
189+
let span = try XCTUnwrap(tracer.finishedSpans.first, "No span was recorded!")
190+
191+
XCTAssertEqual(span.operationName, "GET")
192+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.urlPath), "/echo-method")
193+
XCTAssertEqual(span.attributes.get(client.tracing.attributeKeys.fullUrl), "\(urlWithoutCredentials)")
194+
}
150195
}

0 commit comments

Comments
 (0)