From a4643e6baeafd34a8cdc2dbab54ff9030c2e8493 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 21 Nov 2025 13:05:36 -0800 Subject: [PATCH 1/2] fix: fix url session instrumentation for a missing case --- Package.resolved | 4 +- .../URLSessionInstrumentation.swift | 4 +- .../URLSessionInstrumentationTests.swift | 38 +++++++++++++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/Package.resolved b/Package.resolved index 96116a88..fb42981f 100644 --- a/Package.resolved +++ b/Package.resolved @@ -122,8 +122,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-protobuf.git", "state" : { - "revision" : "c6fe6442e6a64250495669325044052e113e990c", - "version" : "1.32.0" + "revision" : "c169a5744230951031770e27e475ff6eefe51f9d", + "version" : "1.33.3" } }, { diff --git a/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift b/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift index 5f5af94b..092750b2 100644 --- a/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift +++ b/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift @@ -87,7 +87,9 @@ public class URLSessionInstrumentation { URLSessionDataDelegate.urlSession(_:dataTask:didBecome:) as (URLSessionDataDelegate) -> ( (URLSession, URLSessionDataTask, URLSessionStreamTask) -> Void - )?) + )?), + #selector( + URLSessionTaskDelegate.urlSession(_:task:didFinishCollecting:)) ] let classes = configuration.delegateClassesToInstrument diff --git a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift index 6bb07a4e..1112d66e 100644 --- a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift +++ b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift @@ -53,6 +53,17 @@ class URLSessionInstrumentationTests: XCTestCase { } } + /// A minimal delegate that only implements didFinishCollecting. + /// This tests that delegate classes are discovered even when they only implement + /// urlSession(_:task:didFinishCollecting:) and no other delegate methods. + class MinimalMetricsDelegate: NSObject, URLSessionTaskDelegate { + var didFinishCollectingCalled = false + + func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics) { + didFinishCollectingCalled = true + } + } + static var requestCopy: URLRequest! static var responseCopy: HTTPURLResponse! @@ -915,15 +926,15 @@ class URLSessionInstrumentationTests: XCTestCase { public func testAsyncAwaitUploadMethodsAreNotInstrumented() async throws { let url = URL(string: "http://localhost:33333/success")! let request = URLRequest(url: url) - + // Test upload(for:from:) method let (data, response) = try await URLSession.shared.upload(for: request, from: Data()) - + guard let httpResponse = response as? HTTPURLResponse else { XCTFail("Response should be HTTPURLResponse") return } - + XCTAssertEqual(httpResponse.statusCode, 200, "Request should succeed") XCTAssertNotNil(data, "Should receive response data") @@ -931,4 +942,25 @@ class URLSessionInstrumentationTests: XCTestCase { XCTAssertTrue(URLSessionInstrumentationTests.checker.createdRequestCalled, "createdRequest should be called") XCTAssertTrue(URLSessionInstrumentationTests.checker.receivedResponseCalled, "receivedResponse should be called") } + + /// Tests that delegate classes are discovered and swizzled even when they only implement + /// urlSession(_:task:didFinishCollecting:) and no other delegate methods. + /// This is a regression test for a bug where the selector for didFinishCollecting was missing + /// from the delegate discovery mechanism. + @available(macOS 10.15, iOS 13.0, tvOS 13.0, *) + public func testDelegateWithOnlyDidFinishCollectingIsDiscovered() async throws { + let request = URLRequest(url: URL(string: "http://localhost:33333/success")!) + + let delegate = MinimalMetricsDelegate() + let session = URLSession(configuration: URLSessionConfiguration.default, delegate: delegate, delegateQueue: nil) + + _ = try await session.data(for: request) + + // Verify the user's delegate was called (proves swizzling forwarded the call) + XCTAssertTrue(delegate.didFinishCollectingCalled, "Delegate should receive metrics callback") + + // Verify instrumentation worked (span was created and completed) + XCTAssertTrue(URLSessionInstrumentationTests.checker.createdRequestCalled, "Instrumentation should capture the request") + XCTAssertTrue(URLSessionInstrumentationTests.checker.receivedResponseCalled, "Instrumentation should capture the response") + } } From b753d9c2985d9701e08a592addab762fae77a618 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 24 Nov 2025 17:16:08 -0800 Subject: [PATCH 2/2] fix a minor nil-safety issue --- Sources/Instrumentation/NetworkStatus/NetworkStatus.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Instrumentation/NetworkStatus/NetworkStatus.swift b/Sources/Instrumentation/NetworkStatus/NetworkStatus.swift index a471b778..e4b9c102 100644 --- a/Sources/Instrumentation/NetworkStatus/NetworkStatus.swift +++ b/Sources/Instrumentation/NetworkStatus/NetworkStatus.swift @@ -27,7 +27,7 @@ case .cellular: if #available(iOS 13.0, *) { if let serviceId = networkInfo.dataServiceIdentifier, let value = networkInfo.serviceCurrentRadioAccessTechnology?[serviceId] { - return ("cell", simpleConnectionName(connectionType: value), networkInfo.serviceSubscriberCellularProviders?[networkInfo.dataServiceIdentifier!]) + return ("cell", simpleConnectionName(connectionType: value), networkInfo.serviceSubscriberCellularProviders?[serviceId]) } } else { if let radioType = networkInfo.currentRadioAccessTechnology {