Skip to content

Commit c32bd1c

Browse files
committed
Remove the need for #if TracingSupport on every trace call-site
1 parent b1fa205 commit c32bd1c

File tree

3 files changed

+51
-23
lines changed

3 files changed

+51
-23
lines changed

Sources/AsyncHTTPClient/HTTPHandler.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,9 @@ extension HTTPClient {
929929
public let logger: Logger // We are okay to store the logger here because a Task is for only one request.
930930

931931
#if TracingSupport
932-
public let tracer: (any Tracer)? // We are okay to store the tracer here because a Task is for only one request.
932+
let tracer: (any Tracer)? // We are okay to store the tracer here because a Task is for only one request.
933+
#else
934+
let tracer: TracingSupportDisabledTracer? // We are okay to store the tracer here because a Task is for only one request.
933935
#endif
934936

935937
let promise: EventLoopPromise<Response>
@@ -965,9 +967,7 @@ extension HTTPClient {
965967
self.eventLoop = eventLoop
966968
self.promise = eventLoop.makePromise()
967969
self.logger = logger
968-
#if TracingSupport
969970
self.tracer = nil
970-
#endif // TracingSupport
971971
self.makeOrGetFileIOThreadPool = makeOrGetFileIOThreadPool
972972
self.state = NIOLockedValueBox(State(isCancelled: false, taskDelegate: nil))
973973
}

Sources/AsyncHTTPClient/RequestBag+Tracing.swift

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,60 @@
1212
//
1313
//===----------------------------------------------------------------------===//
1414

15-
#if TracingSupport
16-
1715
import Logging
1816
import NIOConcurrencyHelpers
1917
import NIOCore
2018
import NIOHTTP1
2119
import NIOSSL
2220

21+
#if TracingSupport
2322
import Tracing
23+
#endif
24+
25+
#if TracingSupport
26+
typealias HTTPClientTracingSupportTracerType = Tracer
27+
#else
28+
enum TracingSupportDisabledTracer {}
29+
typealias HTTPClientTracingSupportTracerType = TracingSupportDisabledTracer
30+
#endif
31+
32+
protocol _TracingSupportOperations {
33+
associatedtype TracerType
34+
35+
/// Starts the "overall" Span that encompases the beginning of a request until receipt of the head part of the response.
36+
mutating func startRequestSpan(tracer: TracerType?)
37+
38+
/// Fails the active overall span given some internal error, e.g. timeout, pool shutdown etc.
39+
/// This is not to be used for failing a span given a failure status coded HTTPResponse.
40+
mutating func failRequestSpan(error: any Error)
41+
42+
/// Ends the active overall span upon receipt of the response head.
43+
///
44+
/// If the status code is in error range, this will automatically fail the span.
45+
mutating func endRequestSpan(response: HTTPResponseHead)
46+
}
47+
48+
extension RequestBag.LoopBoundState: _TracingSupportOperations { }
49+
50+
#if !TracingSupport
51+
/// Operations used to start/end spans at apropriate times from the Request lifecycle.
52+
extension RequestBag.LoopBoundState {
53+
typealias TracerType = HTTPClientTracingSupportTracerType
54+
55+
@inlinable
56+
mutating func startRequestSpan(tracer: TracerType?) {}
57+
58+
@inlinable
59+
mutating func failRequestSpan(error: any Error) {}
60+
61+
@inlinable
62+
mutating func endRequestSpan(response: HTTPResponseHead) {}
63+
}
64+
65+
#else // TracingSupport
2466

2567
extension RequestBag.LoopBoundState {
68+
typealias TracerType = Tracer
2669

2770
mutating func startRequestSpan(tracer: (any Tracer)?) {
2871
guard let tracer else {
@@ -33,7 +76,7 @@ extension RequestBag.LoopBoundState {
3376
self.activeRequestSpan = tracer.startSpan("\(request.method)")
3477
}
3578

36-
// TODO: should be able to record the reason for the failure, e.g. timeout, cancellation, server error etc
79+
// TODO: should be able to record the reason for the failure, e.g. timeout, cancellation etc.
3780
mutating func failRequestSpan(error: any Error) {
3881
guard let span = activeRequestSpan else {
3982
return
@@ -49,7 +92,7 @@ extension RequestBag.LoopBoundState {
4992
return
5093
}
5194

52-
span.attributes["http.response.status_code"] = SpanAttribute.int(Int64(response.status.code))
95+
span.attributes["http.response.status_code"] = SpanAttribute.int64(Int64(response.status.code))
5396
if response.status.code >= 400 {
5497
span.setStatus(.init(code: .error))
5598
}

Sources/AsyncHTTPClient/RequestBag.swift

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,9 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
7171
self.task.logger
7272
}
7373

74-
#if TracingSupport
75-
var tracer: (any Tracer)? {
74+
var tracer: HTTPClientTracingSupportTracerType? {
7675
self.task.tracer
7776
}
78-
#endif
7977

8078
let connectionDeadline: NIODeadline
8179

@@ -132,24 +130,18 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
132130

133131
private func willExecuteRequest0(_ executor: HTTPRequestExecutor) {
134132
// Immediately start a span for the "whole" request
135-
#if TracingSupport
136133
self.loopBoundState.value.startRequestSpan(tracer: self.tracer)
137-
#endif
138134

139135
let action = self.loopBoundState.value.state.willExecuteRequest(executor)
140136
switch action {
141137
case .cancelExecuter(let executor):
142138
executor.cancelRequest(self)
143-
#if TracingSupport
144139
self.loopBoundState.value.failRequestSpan(error: CancellationError()) // TODO: something better?
145-
#endif
146140
case .failTaskAndCancelExecutor(let error, let executor):
147141
self.delegate.didReceiveError(task: self.task, error)
148142
self.task.failInternal(with: error)
149143
executor.cancelRequest(self)
150-
#if TracingSupport
151144
self.loopBoundState.value.failRequestSpan(error: error)
152-
#endif
153145
case .none:
154146
break
155147
}
@@ -252,9 +244,7 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
252244
self.task.eventLoop.assertInEventLoop()
253245

254246
self.delegate.didReceiveError(task: self.task, error)
255-
#if TracingSupport
256247
self.loopBoundState.value.failRequestSpan(error: error)
257-
#endif // TracingSupport
258248
self.task.promise.fail(error)
259249
}
260250

@@ -408,10 +398,7 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
408398
self.executeFailAction0(action)
409399

410400
self.loopBoundState.value.redirectTask?.fail(reason: error)
411-
412-
#if TracingSupport
413401
self.loopBoundState.value.failRequestSpan(error: error)
414-
#endif // TracingSupport
415402
}
416403

417404
private func executeFailAction0(_ action: RequestBag<Delegate>.StateMachine.FailAction) {
@@ -422,9 +409,7 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
422409
self.failTask0(error)
423410
case .cancelExecutor(let executor):
424411
executor.cancelRequest(self)
425-
#if TracingSupport
426412
self.loopBoundState.value.failRequestSpan(error: CancellationError()) // TODO: is this right?
427-
#endif
428413
case .none:
429414
break
430415
}

0 commit comments

Comments
 (0)