Conversation
delegateClassesToInstrument and ignoredClassPrefixes were not documented at all, so the only discoverable behaviour was the default, which searches every class loaded in the process. Documents both, and adds a section on what that search costs and how passing the delegate classes explicitly skips it.
|
Did you reproduce the bug by any chance? |
|
@williazz yeah, I measured it. In a test process with ~35k classes loaded, walking every class's method list takes ~285 ms (3 runs: 285/282/290) and finds about 19 matching classes. Two caveats on my numbers: I timed the enumeration and method walk in isolation rather than the shipped Passing Longer term I think the answer is discovering delegate classes on demand instead of scanning everything up front. I'll open an issue for that so the performance side is tracked on its own rather than buried in a docs PR. |
|
Sharing how I measured, since my number came from timing the scan in isolation rather than the shipped code path. If you want the number for your own app, this measures the real var classCount: UInt32 = 0
_ = objc_copyClassList(&classCount)
let t0 = ProcessInfo.processInfo.systemUptime
let instrumentation = URLSessionInstrumentation(configuration: URLSessionInstrumentationConfiguration())
let ms = (ProcessInfo.processInfo.systemUptime - t0) * 1000
print("classes: \(classCount), URLSessionInstrumentation init: \(ms) ms")One warning from my own mistake: don't time a second And this is the harness behind the ~285 ms, so the caveat is visible rather than just asserted — it reimplements what let selectors = [
#selector(URLSessionDataDelegate.urlSession(_:dataTask:didReceive:)),
#selector(URLSessionDataDelegate.urlSession(_:dataTask:didReceive:completionHandler:)),
#selector(URLSessionDataDelegate.urlSession(_:task:didCompleteWithError:)),
#selector(URLSessionTaskDelegate.urlSession(_:task:didFinishCollecting:))
]
let t0 = ProcessInfo.processInfo.systemUptime
let classes = InstrumentationUtils.objc_getClassList()
let listed = ProcessInfo.processInfo.systemUptime
DispatchQueue.concurrentPerform(iterations: classes.count) { i in
var methodCount: UInt32 = 0
guard let methodList = class_copyMethodList(classes[i], &methodCount) else { return }
defer { free(methodList) }
for j in 0 ..< Int(methodCount) where selectors.contains(method_getName(methodList[j])) { break }
}
let done = ProcessInfo.processInfo.systemUptime
print("getClassList \((listed - t0) * 1000) ms, method walk \((done - listed) * 1000) ms")On my machine, 35,417 classes: |
|
|
||
| `delegateClassesToInstrument: [AnyClass]?`: The session delegate classes to instrument. When this is `nil`, the instrumentation discovers them by examining **every class loaded in the process** at initialization, which is the default. Passing your delegate classes explicitly skips that search — see [Initialization cost](#initialization-cost) below. | ||
|
|
||
| `ignoredClassPrefixes: [String]?`: Class name prefixes to leave out of that search. |
There was a problem hiding this comment.
Can we remove this claim for now? injectInNSURLClasses() calls objc_getClassList() directly, and ignoredClassPrefixes is never read after configuration, so setting it does not exclude classes or reduce the initialization scan.
There was a problem hiding this comment.
You're right, removed. Confirmed it's assigned in the initializer and never read anywhere — the search uses the hardcoded excludeList instead, so setting ignoredClassPrefixes has no effect at all. I shouldn't have documented it without checking it was wired up.
Worth deciding separately whether to wire it up or deprecate it, since right now it's public API that silently does nothing. Happy to open an issue for that if useful.
The option is stored but never read, so documenting it as excluding classes from the delegate search would be describing behaviour that does not exist.
| ) | ||
| ``` | ||
|
|
||
| Only the classes you list are instrumented, so a delegate you leave out is not, and requests made through it are not captured. |
There was a problem hiding this comment.
Small wording fix: completion-handler requests are still captured even when their session's delegate is omitted. I verified this with a local HTTP request: both span creation and completion ran. Would it be clearer to say that callbacks on unlisted delegates are not instrumented?
There was a problem hiding this comment.
@aranhave You're right, reworded. The completion-handler path is swizzled on URLSession itself (injectIntoNSURLSessionCreateTaskWithParameterMethods), not through the delegate, so those requests are captured regardless of what's in the list. One thing I noticed while checking - if you omit a delegate you actually use, the span still starts at task creation but nothing ends it, since didCompleteWithError is only swizzled on listed classes - so I added a line warning about that too.
Completion-handler and async/await requests are instrumented on URLSession itself, not through the session delegate, so they are captured whether or not their delegate class is listed. The previous wording claimed all requests through an omitted delegate are lost. Also warn about the other direction: a delegate-driven request through an omitted class still starts a span at task creation, but the callback that ends it is only swizzled on listed classes, so that span is never ended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1172 +/- ##
===========================================
+ Coverage 67.89% 79.45% +11.56%
===========================================
Files 344 95 -249
Lines 15169 7239 -7930
===========================================
- Hits 10299 5752 -4547
+ Misses 4870 1487 -3383 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Docs only.
Creating
URLSessionInstrumentationsearches every class loaded in the process to find session delegates. There's already a way to skip that — passdelegateClassesToInstrument— but neither that option norignoredClassPrefixesappears in the README, so the only discoverable behaviour is the expensive default.This documents both options and adds a short section on what the search costs and how to avoid it.
The reason I went looking: on #895 @williazz reports the instrumentation blocking app launch by ~500 ms across devices and simulators, and says the only workaround they found was deferring initialization, which then drops early requests. The escape hatch they needed already exists in the API, it just isn't written down anywhere.
I've tried to be straight about the trade-off rather than just recommending it — if you pass an explicit list, a delegate you leave out isn't instrumented and its requests aren't captured. And I noted that deferring initialization is not an equivalent workaround, for the reason williazz hit.
No code changes. The snippet compiles (checked against the current initializer).
🤖 Generated with Claude Code