Skip to content

Commit 7396854

Browse files
committed
[Tracing] fix availability issues
1 parent ce89513 commit 7396854

File tree

6 files changed

+106
-33
lines changed

6 files changed

+106
-33
lines changed

Sources/AsyncHTTPClient/HTTPClient.swift

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public final class HTTPClient: Sendable {
7777

7878
#if TracingSupport
7979
@_spi(Tracing)
80-
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) // for TaskLocal ServiceContext
80+
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
8181
public var tracer: (any Tracer)? {
8282
configuration.tracing.tracer
8383
}
@@ -768,16 +768,22 @@ public final class HTTPClient: Sendable {
768768
return nil
769769
case .shuttingDown, .shutDown:
770770
logger.debug("client is shutting down, failing request")
771-
let error = HTTPClientError.alreadyShutdown
772-
// #if TracingSupport
773-
// span?.recordError(error)
774-
// span?.end()
775-
// #endif
771+
#if TracingSupport
772+
if #available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) {
773+
return Task<Delegate.Response>.failedTask(
774+
eventLoop: taskEL,
775+
error: HTTPClientError.alreadyShutdown,
776+
logger: logger,
777+
tracer: tracer,
778+
makeOrGetFileIOThreadPool: self.makeOrGetFileIOThreadPool
779+
)
780+
}
781+
#endif // TracingSupport
782+
776783
return Task<Delegate.Response>.failedTask(
777784
eventLoop: taskEL,
778-
error: error,
785+
error: HTTPClientError.alreadyShutdown,
779786
logger: logger,
780-
tracer: tracer,
781787
makeOrGetFileIOThreadPool: self.makeOrGetFileIOThreadPool
782788
)
783789
}
@@ -802,12 +808,29 @@ public final class HTTPClient: Sendable {
802808
}
803809
}()
804810

811+
let task: HTTPClient.Task<Delegate.Response>
812+
#if TracingSupport
813+
if #available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) {
814+
task = Task<Delegate.Response>(
815+
eventLoop: taskEL,
816+
logger: logger,
817+
tracer: tracer,
818+
makeOrGetFileIOThreadPool: self.makeOrGetFileIOThreadPool
819+
)
820+
} else {
821+
task = Task<Delegate.Response>(
822+
eventLoop: taskEL,
823+
logger: logger,
824+
makeOrGetFileIOThreadPool: self.makeOrGetFileIOThreadPool
825+
)
826+
}
827+
#else
805828
let task = Task<Delegate.Response>(
806829
eventLoop: taskEL,
807830
logger: logger,
808-
tracer: tracer,
809831
makeOrGetFileIOThreadPool: self.makeOrGetFileIOThreadPool
810832
)
833+
#endif // TracingSupport
811834

812835
do {
813836
let requestBag = try RequestBag(
@@ -1109,8 +1132,16 @@ public final class HTTPClient: Sendable {
11091132
}
11101133
}
11111134

1135+
/// Configuration for tracing attributes set by the HTTPClient.
11121136
public var attributeKeys: AttributeKeys
11131137

1138+
1139+
public init() {
1140+
self._tracer = nil
1141+
self.attributeKeys = .init()
1142+
}
1143+
1144+
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
11141145
public init(
11151146
tracer: (any Tracer)? = InstrumentationSystem.tracer,
11161147
attributeKeys: AttributeKeys = .init()

Sources/AsyncHTTPClient/HTTPHandler.swift

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,15 @@ 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)? // Ok to store the tracer here because a Task is for only one request.
932+
let anyTracer: Optional<any Sendable> // Ok to store the tracer here because a Task is for only one request.
933+
934+
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
935+
public var tracer: (any Tracer)? {
936+
get {
937+
print("[swift][\(#fileID):\(#line)] _tracer = \(anyTracer)")
938+
return anyTracer as! (any Tracer)?
939+
}
940+
}
933941
#endif
934942

935943
let promise: EventLoopPromise<Response>
@@ -965,7 +973,7 @@ extension HTTPClient {
965973
self.eventLoop = eventLoop
966974
self.promise = eventLoop.makePromise()
967975
self.logger = logger
968-
self.tracer = nil
976+
self.anyTracer = nil
969977
self.makeOrGetFileIOThreadPool = makeOrGetFileIOThreadPool
970978
self.state = NIOLockedValueBox(State(isCancelled: false, taskDelegate: nil))
971979
}
@@ -986,6 +994,7 @@ extension HTTPClient {
986994
}
987995

988996
#if TracingSupport
997+
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
989998
init(
990999
eventLoop: EventLoop,
9911000
logger: Logger,
@@ -995,11 +1004,13 @@ extension HTTPClient {
9951004
self.eventLoop = eventLoop
9961005
self.promise = eventLoop.makePromise()
9971006
self.logger = logger
998-
self.tracer = tracer
1007+
print("[swift] set any tracer on TASK = \(tracer)")
1008+
self.anyTracer = tracer
9991009
self.makeOrGetFileIOThreadPool = makeOrGetFileIOThreadPool
10001010
self.state = NIOLockedValueBox(State(isCancelled: false, taskDelegate: nil))
10011011
}
10021012

1013+
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
10031014
static func failedTask(
10041015
eventLoop: EventLoop,
10051016
error: Error,

Sources/AsyncHTTPClient/RequestBag.swift

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,16 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
6969
self.task.logger
7070
}
7171

72-
let connectionDeadline: NIODeadline
73-
74-
var tracer: HTTPClientTracingSupportTracerType? {
72+
var anyTracer: (any Sendable)? {
73+
self.task.anyTracer
74+
}
75+
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
76+
var tracer: (any Tracer)? {
7577
self.task.tracer
7678
}
7779

80+
let connectionDeadline: NIODeadline
81+
7882
let requestOptions: RequestOptions
7983

8084
let requestHead: HTTPRequestHead
@@ -97,6 +101,8 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
97101
self.eventLoopPreference = eventLoopPreference
98102
self.task = task
99103

104+
assert(task.anyTracer != nil, "tracer was nil!")
105+
100106
let loopBoundState = LoopBoundState(
101107
request: request,
102108
state: StateMachine(redirectHandler: redirectHandler),
@@ -128,18 +134,19 @@ final class RequestBag<Delegate: HTTPClientResponseDelegate & Sendable>: Sendabl
128134

129135
private func willExecuteRequest0(_ executor: HTTPRequestExecutor) {
130136
// Immediately start a span for the "whole" request
131-
self.loopBoundState.value.startRequestSpan(tracer: self.tracer)
137+
print("[swift] WILL EXECUTE \(self.anyTracer)")
138+
self.loopBoundState.value.startRequestSpan(tracer: self.anyTracer)
132139

133140
let action = self.loopBoundState.value.state.willExecuteRequest(executor)
134141
switch action {
135142
case .cancelExecuter(let executor):
136143
executor.cancelRequest(self)
137-
self.loopBoundState.value.failRequestSpan(error: CancellationError())
144+
self.loopBoundState.value.failRequestSpanAsCancelled()
138145
case .failTaskAndCancelExecutor(let error, let executor):
139146
self.delegate.didReceiveError(task: self.task, error)
140147
self.task.failInternal(with: error)
141148
executor.cancelRequest(self)
142-
self.loopBoundState.value.failRequestSpan(error: CancellationError())
149+
self.loopBoundState.value.failRequestSpan(error: error)
143150
case .none:
144151
break
145152
}

Sources/AsyncHTTPClient/TracingSupport.swift

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,24 @@ struct HTTPHeadersInjector: Injector, @unchecked Sendable {
3434
}
3535
#endif // TracingSupport
3636

37-
#if TracingSupport
38-
typealias HTTPClientTracingSupportTracerType = any Tracer
39-
#else
40-
enum TracingSupportDisabledTracer {}
41-
typealias HTTPClientTracingSupportTracerType = TracingSupportDisabledTracer
42-
#endif
37+
// #if TracingSupport
38+
// @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
39+
// typealias HTTPClientTracingSupportTracerType = any Tracer
40+
// #else
41+
// enum TracingSupportDisabledTracer {}
42+
// typealias HTTPClientTracingSupportTracerType = TracingSupportDisabledTracer
43+
// #endif
4344

4445
protocol _TracingSupportOperations {
45-
associatedtype TracerType
46+
// associatedtype TracerType
4647

4748
/// Starts the "overall" Span that encompases the beginning of a request until receipt of the head part of the response.
48-
mutating func startRequestSpan(tracer: TracerType?)
49+
mutating func startRequestSpan(tracer: Any?)
4950

5051
/// Fails the active overall span given some internal error, e.g. timeout, pool shutdown etc.
5152
/// This is not to be used for failing a span given a failure status coded HTTPResponse.
5253
mutating func failRequestSpan(error: any Error)
54+
mutating func failRequestSpanAsCancelled() // because CancellationHandler availability...
5355

5456
/// Ends the active overall span upon receipt of the response head.
5557
///
@@ -65,7 +67,7 @@ extension RequestBag.LoopBoundState {
6567
typealias TracerType = HTTPClientTracingSupportTracerType
6668

6769
@inlinable
68-
mutating func startRequestSpan(tracer: TracerType?) {}
70+
mutating func startRequestSpan(tracer: Any?) {}
6971

7072
@inlinable
7173
mutating func failRequestSpan(error: any Error) {}
@@ -77,10 +79,14 @@ extension RequestBag.LoopBoundState {
7779
#else // TracingSupport
7880

7981
extension RequestBag.LoopBoundState {
80-
typealias TracerType = Tracer
81-
82-
mutating func startRequestSpan(tracer: (any Tracer)?) {
83-
guard let tracer else {
82+
// typealias TracerType = Tracer
83+
84+
mutating func startRequestSpan(tracer: Any?) {
85+
guard #available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *),
86+
let tracer = tracer as? (any Tracer)?,
87+
let tracer else {
88+
// print("[swift][\(#fileID):\(#line)] MISSING TRACER: \(tracer)")
89+
fatalError("[swift][\(#fileID):\(#line)] MISSING TRACER: \(tracer)")
8490
return
8591
}
8692

@@ -92,13 +98,23 @@ extension RequestBag.LoopBoundState {
9298
self.activeSpan?.attributes["loc"] = "\(#fileID):\(#line)"
9399
}
94100

95-
// TODO: should be able to record the reason for the failure, e.g. timeout, cancellation etc.
101+
mutating func failRequestSpanAsCancelled() {
102+
if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) {
103+
let error = CancellationError()
104+
failRequestSpan(error: error)
105+
} else {
106+
fatalError("Unexpected configuration; expected availability of CancellationError")
107+
}
108+
}
109+
96110
mutating func failRequestSpan(error: any Error) {
97111
guard let span = activeSpan else {
98112
return
99113
}
100114

101115
span.recordError(error)
116+
span.end()
117+
102118
self.activeSpan = nil
103119
}
104120

Tests/AsyncHTTPClientTests/HTTPClientBase.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class XCTestCaseHTTPClientTestsBaseClass: XCTestCase {
6161
}
6262
)
6363
backgroundLogger.logLevel = .trace
64-
var configuration = HTTPClient.Configuration().enableFastFailureModeForTesting()
64+
let configuration = HTTPClient.Configuration().enableFastFailureModeForTesting()
6565

6666
self.defaultClient = HTTPClient(
6767
eventLoopGroupProvider: .shared(self.clientGroup),

Tests/AsyncHTTPClientTests/HTTPClientTracingTests.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
8585
let url = self.defaultHTTPBinURLPrefix + "echo-method"
8686
let _ = try client.post(url: url).wait()
8787

88+
guard tracer.activeSpans.isEmpty else {
89+
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
90+
return
91+
}
8892
guard let span = tracer.finishedSpans.first else {
8993
XCTFail("No span was recorded!")
9094
return
@@ -98,6 +102,10 @@ final class HTTPClientTracingTests: XCTestCaseHTTPClientTestsBaseClass {
98102
let request = HTTPClientRequest(url: url)
99103
let _ = try await client.execute(request, deadline: .distantFuture)
100104

105+
guard tracer.activeSpans.isEmpty else {
106+
XCTFail("Still active spans which were not finished (\(tracer.activeSpans.count))! \(tracer.activeSpans)")
107+
return
108+
}
101109
guard let span = tracer.finishedSpans.first else {
102110
XCTFail("No span was recorded!")
103111
return

0 commit comments

Comments
 (0)