Add spanCustomization callback to Trace URLSession tracking

[mariusc83]

What and why?

Adds a spanCustomization callback to Trace.Configuration.URLSessionTracking, allowing users to customize spans for intercepted network requests — adding custom tags or overriding the operation name based on request/response content.

This addresses a long-standing feature request (#1649, 7 reactions, 2+ years) for parity with the Android SDK's TracedRequestListener. The key use case is tagging GraphQL requests with operation name/type, but it applies to any scenario where span metadata should depend on the request or response.

How?

• Adds Trace.Configuration.SpanCustomization typealias: (URLRequest, OTSpan, URLResponse?, Error?) -> Void
• Adds spanCustomization property to Trace.Configuration.URLSessionTracking
• The callback is invoked in TracingURLSessionHandler.interceptionDidComplete after default tags are set and before span.finish(), so users can add custom tags or override the operation name
• Mirrors the existing RUM.ResourceAttributesProvider pattern for API consistency

Usage:

Trace.enable(with: .init(
    urlSessionTracking: .init(
        firstPartyHostsTracing: .trace(hosts: ["api.example.com"]),
        spanCustomization: { request, span, response, error in
            if let body = request.httpBody,
               let json = try? JSONSerialization.jsonObject(with: body) as? [String: Any],
               let opName = json["operationName"] as? String {
                span.setTag(key: "graphql.operation.name", value: opName)
                span.setOperationName("graphql.\(opName)")
            }
        }
    )
))

Review checklist

• Feature or bugfix MUST have appropriate tests (unit, integration)
• Make sure each commit and the PR mention the Issue number or JIRA reference
• Add CHANGELOG entry for user facing changes
• Add Objective-C interface for public APIs - see our guidelines (internal)
• Run make api-surface when adding new APIs

Closes #1649

[simaoseica-dd]

Great addition. Thank you.
Left some comments. Also, please add an entry for this new feature on the changelog 👌

[simaoseica-dd]

Can you merge this test with the first one testGivenSpanCustomization_whenInterceptionCompletes_itCallsCustomizationWithAllParameters? They seem to have the same setup.

[mariusc83]

Done in 805d67e — merged the custom-tags-coexist assertions into testGivenSpanCustomization_whenInterceptionCompletes_itCallsCustomizationWithAllParameters and replaced the third test with an error-only test (per the other comment).

[arroz]

This is passing the request to a user provided closure. As the name implies, this is unsafe, since the user may touch the request at the same time the SDK is instrumenting it on a different thread. The API signature should be rethought to avoid the need of passing the request. As the name implies, this is unsafe, which in Swift language means it can lead to unpredictable state not detected by runtime, silently corrupting user data.

[mariusc83]

Thanks for the flag — I investigated this in detail.

TL;DR: The concern is valid in general but does not apply here. The callback follows the same established pattern as RUM.ResourceAttributesProvider, which already ships and passes interception.request.unsafeOriginal in the exact same way (URLSessionRUMResourcesHandler.swift:140).

Why it is safe in this context:

1. The callback runs after the request completes — it is called inside interceptionDidComplete, after all default tags are set and just before span.finish(). There is no concurrent SDK mutation of the request at this point.

2. URLRequest is a Swift value type (struct with COW). When passed as the closure parameter, the user receives a copy. Mutating it in the callback cannot affect the SDK internal state or the original network request.

3. The unsafeOriginal naming is about reading headers concurrently (see #1638, #1767) — specifically, URLRequest.allHTTPHeaderFields can crash when accessed concurrently. That risk exists during request interception, not in the completion callback where the request is already done.

Android SDK parity: The Android equivalent (TracedRequestListener.onRequestIntercepted) also passes the request to the callback. The newer NetworkTracedRequestListener wraps it in an immutable HttpRequestInfo interface, but the OkHttp-specific version passes the original okhttp3.Request directly (which is immutable by design in OkHttp). Both invoke the callback post-completion, same as here.

If we want to be extra safe, we could copy the relevant fields into a dedicated immutable struct (like Android HttpRequestInfo), but that would diverge from the existing RUM.ResourceAttributesProvider pattern and limit what users can inspect (e.g., httpBody for GraphQL operation extraction — the primary use case from #1649).

[arroz]

I would reach to the person who wrote the original code with the "unsafeOriginal" to obtain more context on this. However:

1. The fact we shipped something doesn't mean it's correct. It should not be relied upon as an absolute measure for correction.
2. URLRequests are reusable, so nothing guarantees the user is not holding to a request. We cannot be sure the request is not being mutated (see below).
3. "URLRequest is a Swift value type (struct with COW)" in theory. In practice URLRequest is a toll-free bridge type to NSURLRequest and NSURLMutableRequest. A little research shows allHTTPHeaderFields maps (or mapped on some recent OS version) to a bridged NSMutableDictionary (https://discussions.unity.com/t/upgraded-to-0-7-1-allhttpheaderfields-crash/322436). We can also find additional context by @ncreated here (Crash accessing URLSessionTask.currentRequest and URLRequest.allHTTPHeaderFields kean/Pulse#268). This means the actual implementation of URLRequest may not follow any of the Swift value type guarantees, and indeed evidence shows it doesn't. This has likely been fixed in the most recent OS release (26.*) since I tried to concurrently access this dictionary in a sample project and it didn't crash, but that is not good enough since we support OS releases all the way to iOS 12 right now. Which means for years to come we cannot assume this bug in the Apple frameworks was fixed.
4. It's irrelevant what Android is doing since the system frameworks are different between platforms and the bugs from one don't exist on the other.

Given all this, I am not in favor of shipping a callback like this without a very careful consideration of the impact. My suggestions:

1. Does the use case this applies to really demands the URLRequest to be available on the callback? Can we pass certain fields of the request instead of the whole request?
2. If it's determined we need the URLRequest, investigate in which OS versions this causes problems (like creating sample code that causes the crash and run it on the several simulators) and add a very visible warning in the documentation stating the problem, the OS versions affected, and asking users to mind this issue so they don't cause crashes when using this.

Overall, we should avoid shipping a feature that may cause crashes even if they are caused by a bug in the OS in a situation where we do know about the bug. If we have to, it needs to be thoroughly documented.

[mariusc83]

This makes sense, but I have one question here: Shouldn't the fact that we only delegate the request object to a lambda after the request was completed (basically the interceptor doesn't touch it anymore) be enough for avoiding concurrency access issues ? I am asking as a non iOS expert. On top of that if we want to be 100% correct, using the ImmutableRequest copy from the original request as @ncreated proposes here: #1767 could cover the problem I think. What do you guys think ?

[mariusc83]

After a discussion with @ncreated I have a better picture on how that COW approach works for the UrlRequest instance in iOS and if I understood correctly it is only copying the pointer and not the whole object. Meaning that yes you could experience issues but shouldn't the 1st argument in the LLM investigation: The callback runs after the request completes — it is called inside interceptionDidComplete, after all default tags are set and just before span.finish(). There is no concurrent SDK mutation of the request at this point. cover that issue already ? How likely it is that the system still touches the request after that ?

[mariusc83]

Addressed in f0f1bdf — the SpanCustomization callback now receives ImmutableRequest instead of the raw URLRequest via unsafeOriginal. This avoids the thread-safety issues with URLRequest's bridged NSMutableDictionary internals on iOS 12–25.

ImmutableRequest captures .url, .httpMethod, and .ddOriginHeaderValue at interception time into plain let properties, so the callback gets a true value-type snapshot with no shared mutable state.

[ncreated]

☝️ Great insights, folks 👍. I agree that exposing an immutable copy of URLRequest makes the most sense here. That said, I’m not convinced that using ImmutableRequest as-is is the right approach, for a few reasons:

1. This type only exposes three useful properties: url, httpMethod, and ddOriginHeaderValue. None of them seem especially useful for the spanCustomization callback use case we’re trying to enable, especially for GraphQL requests. I think the most valuable data for this callback is URLRequest.httpBody, which users can combine with url and httpMethod to decode the request and set span tags, similar to the example in the PR description.
2. ImmutableRequest still exposes unsafeOriginal, so users can run into the same issues as if we exposed URLRequest directly. The only difference is that it’s slightly less accessible.
3. ImmutableRequest is an internal SDK type (defined in DatadogInternal), and we should avoid exposing it in a public interface because that would effectively make it part of the public API surface, with all the usual backwards-compatibility constraints that come with that.

My recommendation would be:

• leave ImmutableRequest untouched
• define a separate InterceptedRequest (or similarly named) type in DatadogTrace with only the properties needed for this functionality, likely url, httpMethod, and httpBody

I think this would give us better interface segregation, more clarity for end users, and hopefully better thread safety.

[arroz]

I agree exposing ImmutableRequest is not a good solution, since it exposes an entity that shouldn't be exposed, and does not solve the initial problem.

The problem here is not COW: the URLRequest in this situation should be safe to pass around according to how Swift is designed, and it would be if all its implementation had been done in Swift. COW is just an implementation detail that does not really matter in this context*. The problem here is the backing implementation is still done in ObjectiveC, and because the Swift type is bridged, Apple overlooked concurrency in some situations, making URLRequest behaviour inconsistent with how Swift works.

To solve this: I am not 100% against exposing the URLRequest, especially because the crasher was likely already solved by Apple on modern systems versions (needs confirmation). What I'm saying is, if we do choose to expose it, since we know the problem exists, we should document it thoroughly, including understanding which versions of the OS are affected by this, so people can take appropriate precautions. Otherwise we are exposing an API that we know might create crashers or other data corruption (not by our fault or our SDK users fault, but still, the problem exists).

• There is a myth about COW that should be clarified: COW does not happen automatically, in Swift all value types are copied when passed around. It just so happens Swift stdlib collections are really very small structs that hold a reference type, aka a class, with the real collection content, and implement COW manually. This implementation relies on isKnownUniquelyReferenced and must handle concurrent access internally if needed. This is all very well tested and established, and it works. Again, the problem here is the type we are interacting it is really a (poorly) bridged ObjectiveC type, and not a real Swift type, even if the bridging feature makes it seem so.

[mariusc83]

Done in a546f9f. Created a new Trace.Configuration.InterceptedRequest struct in DatadogTrace with exactly the three properties you recommended: url, httpMethod, and httpBody.

Key points:

• No unsafeOriginal exposure — users can't reach the underlying URLRequest
• httpBody is backed by immutable NSData, safe for concurrent reads (unlike allHTTPHeaderFields)
• ImmutableRequest is unchanged and stays internal to DatadogInternal
• The conversion happens in an internal init(from: ImmutableRequest) on InterceptedRequest, invisible in the public API

Also added three new tests: nil body, GraphQL decoding end-to-end, and a concurrency stress test (5 tasks completing simultaneously on a concurrent queue) to validate thread safety of the callback.

[mariusc83]

Addressed in a546f9f — introduced Trace.Configuration.InterceptedRequest with url, httpMethod, and httpBody. No unsafeOriginal is exposed. httpBody is NSData-backed (immutable), so the thread-safety concern specific to the bridged NSMutableDictionary in allHTTPHeaderFields does not apply.

[arroz]

I would reach to the person who wrote the original code with the "unsafeOriginal" to obtain more context on this. However:

1. The fact we shipped something doesn't mean it's correct. It should not be relied upon as an absolute measure for correction.
2. URLRequests are reusable, so nothing guarantees the user is not holding to a request. We cannot be sure the request is not being mutated (see below).
3. "URLRequest is a Swift value type (struct with COW)" in theory. In practice URLRequest is a toll-free bridge type to NSURLRequest and NSURLMutableRequest. A little research shows allHTTPHeaderFields maps (or mapped on some recent OS version) to a bridged NSMutableDictionary (https://discussions.unity.com/t/upgraded-to-0-7-1-allhttpheaderfields-crash/322436). We can also find additional context by @ncreated here (Crash accessing URLSessionTask.currentRequest and URLRequest.allHTTPHeaderFields kean/Pulse#268). This means the actual implementation of URLRequest may not follow any of the Swift value type guarantees, and indeed evidence shows it doesn't. This has likely been fixed in the most recent OS release (26.*) since I tried to concurrently access this dictionary in a sample project and it didn't crash, but that is not good enough since we support OS releases all the way to iOS 12 right now. Which means for years to come we cannot assume this bug in the Apple frameworks was fixed.
4. It's irrelevant what Android is doing since the system frameworks are different between platforms and the bugs from one don't exist on the other.

Given all this, I am not in favor of shipping a callback like this without a very careful consideration of the impact. My suggestions:

1. Does the use case this applies to really demands the URLRequest to be available on the callback? Can we pass certain fields of the request instead of the whole request?
2. If it's determined we need the URLRequest, investigate in which OS versions this causes problems (like creating sample code that causes the crash and run it on the several simulators) and add a very visible warning in the documentation stating the problem, the OS versions affected, and asking users to mind this issue so they don't cause crashes when using this.

Overall, we should avoid shipping a feature that may cause crashes even if they are caused by a bug in the OS in a situation where we do know about the bug. If we have to, it needs to be thoroughly documented.

[ncreated]

Thanks for addressing the API concern. I left few minor comments, other than this - it looks good to me.

[ncreated]

nit/ Seems to be leftover from personal setup:

Suggested change

.rum-analysis/

[ncreated]

nit/ This comment seems to be agent's own reflection on this work, rather than sth useful in this context.

Suggested change

	// httpBody is Data? — safe to access; the thread-safety concern in ImmutableRequest
	// applies only to allHTTPHeaderFields (bridged NSMutableDictionary).
	self.httpBody = request.unsafeOriginal.httpBody
	self.httpBody = request.unsafeOriginal.httpBody

[simaoseica-dd]

LGTM. Thank you for the contribution