Skip to content

Commit 9c494de

Browse files
authored
added delegate instrumentation exclude list (#897)
* added delegate instrumentation exclude list NSURLSessionInstrumentation fix. "__NSCFURLPRoxySessionConnection" was double instrumented creating two spans for the same network request. preventing it from being instrumented as a delegate prevented the duplication. also fixed replaced explicit set (to implicit) per swiftlint * the NSURLSession on watchos is capable of instrumenting URLSession.shared. reflected in tests
1 parent 4f33acb commit 9c494de

File tree

2 files changed

+42
-20
lines changed

2 files changed

+42
-20
lines changed

Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public class URLSessionInstrumentation {
3131

3232
private var _configuration: URLSessionInstrumentationConfiguration
3333
public var configuration: URLSessionInstrumentationConfiguration {
34-
get { configurationQueue.sync { _configuration } }
34+
configurationQueue.sync { _configuration }
3535
}
3636

3737
private let queue = DispatchQueue(
@@ -41,6 +41,10 @@ public class URLSessionInstrumentation {
4141

4242
static var instrumentedKey = "io.opentelemetry.instrumentedCall"
4343

44+
static let excludeList: [String] = [
45+
"__NSCFURLProxySessionConnection"
46+
]
47+
4448
static let AVTaskClassList: [AnyClass] = [
4549
"__NSCFBackgroundAVAggregateAssetDownloadTask",
4650
"__NSCFBackgroundAVAssetDownloadTask",
@@ -94,16 +98,25 @@ public class URLSessionInstrumentation {
9498
}
9599
defer { free(methodList) }
96100

101+
var foundClasses: [AnyClass] = []
97102
for j in 0 ..< selectorsCount {
98103
for i in 0 ..< Int(methodCount)
99104
where method_getName(methodList[i]) == selectors[j] {
100105
selectorFound = true
101-
injectIntoDelegateClass(cls: theClass)
106+
foundClasses.append(theClass)
102107
}
103108
if selectorFound {
104109
break
105110
}
106111
}
112+
113+
foundClasses.removeAll { cls in
114+
Self.excludeList.contains(NSStringFromClass(cls))
115+
}
116+
117+
foundClasses.forEach { cls in
118+
injectIntoDelegateClass(cls: cls)
119+
}
107120
}
108121

109122
if #available(OSX 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) {
@@ -205,53 +218,53 @@ public class URLSessionInstrumentation {
205218
private func injectIntoNSURLSessionCreateTaskWithParameterMethods() {
206219
typealias UploadWithDataIMP = @convention(c) (URLSession, Selector, URLRequest, Data?) -> URLSessionTask
207220
typealias UploadWithFileIMP = @convention(c) (URLSession, Selector, URLRequest, URL) -> URLSessionTask
208-
221+
209222
let cls = URLSession.self
210-
223+
211224
// MARK: Swizzle `uploadTask(with:from:)`
212225
if let method = class_getInstanceMethod(cls, #selector(URLSession.uploadTask(with:from:))) {
213226
let originalIMP = method_getImplementation(method)
214227
let imp = unsafeBitCast(originalIMP, to: UploadWithDataIMP.self)
215-
228+
216229
let block: @convention(block) (URLSession, URLRequest, Data?) -> URLSessionTask = { [weak self] session, request, data in
217230
guard let instrumentation = self else {
218231
return imp(session, #selector(URLSession.uploadTask(with:from:)), request, data)
219232
}
220-
233+
221234
let sessionTaskId = UUID().uuidString
222235
let instrumentedRequest = URLSessionLogger.processAndLogRequest(
223236
request,
224237
sessionTaskId: sessionTaskId,
225238
instrumentation: instrumentation,
226239
shouldInjectHeaders: true
227240
)
228-
241+
229242
let task = imp(session, #selector(URLSession.uploadTask(with:from:)), instrumentedRequest ?? request, data)
230243
instrumentation.setIdKey(value: sessionTaskId, for: task)
231244
return task
232245
}
233246
let swizzledIMP = imp_implementationWithBlock(block)
234247
method_setImplementation(method, swizzledIMP)
235248
}
236-
249+
237250
// MARK: Swizzle `uploadTask(with:fromFile:)`
238251
if let method = class_getInstanceMethod(cls, #selector(URLSession.uploadTask(with:fromFile:))) {
239252
let originalIMP = method_getImplementation(method)
240253
let imp = unsafeBitCast(originalIMP, to: UploadWithFileIMP.self)
241-
254+
242255
let block: @convention(block) (URLSession, URLRequest, URL) -> URLSessionTask = { [weak self] session, request, fileURL in
243256
guard let instrumentation = self else {
244257
return imp(session, #selector(URLSession.uploadTask(with:fromFile:)), request, fileURL)
245258
}
246-
259+
247260
let sessionTaskId = UUID().uuidString
248261
let instrumentedRequest = URLSessionLogger.processAndLogRequest(
249262
request,
250263
sessionTaskId: sessionTaskId,
251264
instrumentation: instrumentation,
252265
shouldInjectHeaders: true
253266
)
254-
267+
255268
let task = imp(session, #selector(URLSession.uploadTask(with:fromFile:)), instrumentedRequest ?? request, fileURL)
256269
instrumentation.setIdKey(value: sessionTaskId, for: task)
257270
return task
@@ -756,17 +769,17 @@ public class URLSessionInstrumentation {
756769
// Check if we can determine if this is an async/await call
757770
// For iOS 15/macOS 12, we can't use Task.basePriority, so we check other indicators
758771
var isAsyncContext = false
759-
772+
760773
if #available(OSX 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) {
761774
isAsyncContext = Task.basePriority != nil
762775
} else {
763776
// For iOS 15/macOS 12, check if the task has no delegate and no session delegate
764777
// This is a heuristic that works for async/await methods
765-
isAsyncContext = task.delegate == nil &&
778+
isAsyncContext = task.delegate == nil &&
766779
(task.value(forKey: "session") as? URLSession)?.delegate == nil &&
767780
task.state != .running
768781
}
769-
782+
770783
if isAsyncContext {
771784
// This is likely an async/await call
772785
let instrumentedRequest = URLSessionLogger.processAndLogRequest(request,
@@ -777,11 +790,11 @@ public class URLSessionInstrumentation {
777790
task.setValue(instrumentedRequest, forKey: "currentRequest")
778791
}
779792
self.setIdKey(value: taskId, for: task)
780-
793+
781794
// For async/await methods, we need to ensure the delegate is set
782795
// to capture the completion, but only if there's no existing delegate
783796
// AND no session delegate (session delegates are called for async/await too)
784-
if task.delegate == nil,
797+
if task.delegate == nil,
785798
task.state != .running,
786799
(task.value(forKey: "session") as? URLSession)?.delegate == nil {
787800
task.delegate = AsyncTaskDelegate(instrumentation: self, sessionTaskId: taskId)
@@ -844,20 +857,20 @@ class FakeDelegate: NSObject, URLSessionTaskDelegate {
844857
class AsyncTaskDelegate: NSObject, URLSessionTaskDelegate {
845858
private weak var instrumentation: URLSessionInstrumentation?
846859
private let sessionTaskId: String
847-
860+
848861
init(instrumentation: URLSessionInstrumentation, sessionTaskId: String) {
849862
self.instrumentation = instrumentation
850863
self.sessionTaskId = sessionTaskId
851864
super.init()
852865
}
853-
866+
854867
func urlSession(_ session: URLSession, task: URLSessionTask,
855868
didCompleteWithError error: Error?) {
856869
guard let instrumentation = instrumentation else { return }
857-
870+
858871
// Get the task ID that was set when the task was created
859872
let taskId = sessionTaskId
860-
873+
861874
if let error = error {
862875
let status = (task.response as? HTTPURLResponse)?.statusCode ?? 0
863876
URLSessionLogger.logError(error, dataOrFile: nil, statusCode: status,

Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,12 @@ class URLSessionInstrumentationTests: XCTestCase {
370370
task.resume()
371371
URLSessionInstrumentationTests.semaphore.wait()
372372

373+
374+
#if os(watchOS)
375+
XCTAssertEqual(1, URLSessionInstrumentationTests.instrumentation.startedRequestSpans.count)
376+
#else
373377
XCTAssertEqual(0, URLSessionInstrumentationTests.instrumentation.startedRequestSpans.count)
378+
#endif
374379
XCTAssertTrue(URLSessionInstrumentationTests.checker.createdRequestCalled)
375380
XCTAssertNotNil(URLSessionInstrumentationTests.requestCopy?.allHTTPHeaderFields?[W3CTraceContextPropagator.traceparent])
376381
}
@@ -605,7 +610,11 @@ class URLSessionInstrumentationTests: XCTestCase {
605610

606611
_ = try await URLSession.shared.data(for: request)
607612

613+
#if os(watchOS)
614+
XCTAssertEqual(1, URLSessionInstrumentationTests.instrumentation.startedRequestSpans.count)
615+
#else
608616
XCTAssertEqual(0, URLSessionInstrumentationTests.instrumentation.startedRequestSpans.count)
617+
#endif
609618
XCTAssertTrue(URLSessionInstrumentationTests.checker.createdRequestCalled)
610619
XCTAssertNotNil(URLSessionInstrumentationTests.requestCopy?.allHTTPHeaderFields?[W3CTraceContextPropagator.traceparent])
611620
}

0 commit comments

Comments
 (0)