From b3fb5e8fd18b9b7b2a93ee36d98ab659cf0d1813 Mon Sep 17 00:00:00 2001 From: Batuhan Saka Date: Fri, 29 Nov 2024 17:07:45 +0100 Subject: [PATCH 1/5] add request context to baggage provider --- .../URLSessionInstrumentationConfiguration.swift | 11 +++++++---- .../URLSession/URLSessionLogger.swift | 10 +++++----- .../URLSessionInstrumentationTests.swift | 16 ++++++++-------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift b/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift index 2636244d..6950e3a7 100644 --- a/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift +++ b/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift @@ -25,7 +25,7 @@ public struct URLSessionInstrumentationConfiguration { receivedResponse: ((URLResponse, DataOrFile?, Span) -> Void)? = nil, receivedError: ((Error, DataOrFile?, HTTPStatus, Span) -> Void)? = nil, delegateClassesToInstrument: [AnyClass]? = nil, - defaultBaggageProvider: (() -> (Baggage)?)? = nil) + baggageProvider: ((URLRequest, Span?) -> (Baggage)?)? = nil) { self.shouldRecordPayload = shouldRecordPayload self.shouldInstrument = shouldInstrument @@ -37,7 +37,7 @@ public struct URLSessionInstrumentationConfiguration { self.receivedResponse = receivedResponse self.receivedError = receivedError self.delegateClassesToInstrument = delegateClassesToInstrument - self.defaultBaggageProvider = defaultBaggageProvider + self.baggageProvider = baggageProvider } // Instrumentation Callbacks @@ -76,13 +76,16 @@ public struct URLSessionInstrumentationConfiguration { /// The array of URLSession delegate classes that will be instrumented by the library, will autodetect if nil is passed. public var delegateClassesToInstrument: [AnyClass]? - /// Implement this callback to provide a default baggage instance for all instrumented requests. + /// Implement this callback to provide a custom baggage instance for all instrumented requests. /// The provided baggage is merged with active baggage (if any) to create a combined baggage. /// The combined baggage is then injected into request headers using the configured `TextMapBaggagePropagator`. /// This allows consistent propagation across requests, regardless of the active context. + /// + /// The callback provides access to the URLRequest and Span, allowing you to enrich the baggage + /// content based on request details or span context. /// /// Note: The injected baggage depends on the propagator in use (e.g., W3C or custom). /// Returns: A `Baggage` instance or `nil` if no default baggage is needed. - public let defaultBaggageProvider: (() -> (Baggage)?)? + public let baggageProvider: ((URLRequest, Span?) -> (Baggage)?)? } diff --git a/Sources/Instrumentation/URLSession/URLSessionLogger.swift b/Sources/Instrumentation/URLSession/URLSessionLogger.swift index c41fc34c..4eef4041 100644 --- a/Sources/Instrumentation/URLSession/URLSessionLogger.swift +++ b/Sources/Instrumentation/URLSession/URLSessionLogger.swift @@ -161,10 +161,10 @@ class URLSessionLogger { objc_setAssociatedObject(instrumentedRequest, URLSessionInstrumentation.instrumentedKey, true, .OBJC_ASSOCIATION_COPY_NONATOMIC) let propagators = OpenTelemetry.instance.propagators - let defaultBaggage = instrumentation.configuration.defaultBaggageProvider?() + let customBaggage = instrumentation.configuration.baggageProvider?(request, span) var traceHeaders = tracePropagationHTTPHeaders(span: span, - defaultBaggage: defaultBaggage, + customBaggage: customBaggage, textMapPropagator: propagators.textMapPropagator, textMapBaggagePropagator: propagators.textMapBaggagePropagator) @@ -175,7 +175,7 @@ class URLSessionLogger { return instrumentedRequest } - private static func tracePropagationHTTPHeaders(span: Span?, defaultBaggage: Baggage?, textMapPropagator: TextMapPropagator, textMapBaggagePropagator: TextMapBaggagePropagator) -> [String: String] { + private static func tracePropagationHTTPHeaders(span: Span?, customBaggage: Baggage?, textMapPropagator: TextMapPropagator, textMapBaggagePropagator: TextMapBaggagePropagator) -> [String: String] { var headers = [String: String]() struct HeaderSetter: Setter { @@ -195,8 +195,8 @@ class URLSessionLogger { activeBaggage.getEntries().forEach { baggageBuilder.put(key: $0.key, value: $0.value, metadata: $0.metadata) } } - if let defaultBaggage { - defaultBaggage.getEntries().forEach { baggageBuilder.put(key: $0.key, value: $0.value, metadata: $0.metadata) } + if let customBaggage { + customBaggage.getEntries().forEach { baggageBuilder.put(key: $0.key, value: $0.value, metadata: $0.metadata) } } let combinedBaggage = baggageBuilder.build() diff --git a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift index d941df53..33b52bbf 100644 --- a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift +++ b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift @@ -40,7 +40,7 @@ class URLSessionInstrumentationTests: XCTestCase { static var responseCopy: HTTPURLResponse! static var activeBaggage: Baggage! - static var defaultBaggage: Baggage! + static var customBaggage: Baggage! static var config = URLSessionInstrumentationConfiguration(shouldRecordPayload: nil, shouldInstrument: { req in @@ -80,8 +80,8 @@ class URLSessionInstrumentationTests: XCTestCase { receivedError: { _, _, _, _ in URLSessionInstrumentationTests.checker.receivedErrorCalled = true }, - defaultBaggageProvider: { - defaultBaggage + baggageProvider: { _, _ in + customBaggage }) static var checker = Check() @@ -95,7 +95,7 @@ class URLSessionInstrumentationTests: XCTestCase { OpenTelemetry.registerPropagators(textPropagators: [W3CTraceContextPropagator()], baggagePropagator: W3CBaggagePropagator()) OpenTelemetry.registerTracerProvider(tracerProvider: TracerProviderSdk()) - defaultBaggage = DefaultBaggageManager.instance.baggageBuilder() + customBaggage = DefaultBaggageManager.instance.baggageBuilder() .put(key: EntryKey(name: "bar")!, value: EntryValue(string: "baz")!, metadata: nil) .build() @@ -120,7 +120,7 @@ class URLSessionInstrumentationTests: XCTestCase { override class func tearDown() { server.stop() - defaultBaggage = nil + customBaggage = nil OpenTelemetry.instance.contextProvider.removeContextForBaggage(activeBaggage) } @@ -278,7 +278,7 @@ class URLSessionInstrumentationTests: XCTestCase { XCTAssertEqual("HTTP GET", span.name) } - public func testShouldInstrumentRequest_PropagateCombinedActiveAndDefaultBaggages() throws { + public func testShouldInstrumentRequest_PropagateCombinedActiveAndCustomBaggages() throws { let request1 = URLRequest(url: URL(string: "http://defaultName.com")!) let request2 = URLRequest(url: URL(string: "http://dontinstrument.com")!) @@ -301,7 +301,7 @@ class URLSessionInstrumentationTests: XCTestCase { // foo=bar propagated through active baggage defined in `setUp` XCTAssertTrue(baggageHeaderValue.contains("foo=bar")) - // bar=baz propagated through default baggage provided in `URLSessionInstrumentationConfiguration` + // bar=baz propagated through custom baggage provided in `URLSessionInstrumentationConfiguration` XCTAssertTrue(baggageHeaderValue.contains("bar=baz")) XCTAssertEqual(1, URLSessionLogger.runningSpans.count) @@ -311,7 +311,7 @@ class URLSessionInstrumentationTests: XCTestCase { } public func testShouldInstrumentRequest_PropagateOnlyActiveBaggage() throws { - Self.defaultBaggage = nil + Self.customBaggage = nil let request1 = URLRequest(url: URL(string: "http://defaultName.com")!) From b2b4fc45c339c9caae010716999a589e0ab2300e Mon Sep 17 00:00:00 2001 From: Batuhan Saka Date: Fri, 29 Nov 2024 17:23:25 +0100 Subject: [PATCH 2/5] update doc for `baggageProvider` callback --- .../URLSessionInstrumentationConfiguration.swift | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift b/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift index 6950e3a7..331997ba 100644 --- a/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift +++ b/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift @@ -76,16 +76,15 @@ public struct URLSessionInstrumentationConfiguration { /// The array of URLSession delegate classes that will be instrumented by the library, will autodetect if nil is passed. public var delegateClassesToInstrument: [AnyClass]? - /// Implement this callback to provide a custom baggage instance for all instrumented requests. - /// The provided baggage is merged with active baggage (if any) to create a combined baggage. - /// The combined baggage is then injected into request headers using the configured `TextMapBaggagePropagator`. - /// This allows consistent propagation across requests, regardless of the active context. + /// Provides a baggage instance for instrumented requests that is merged with active baggage (if any). + /// The callback can be used to define static baggage for all requests or create dynamic baggage + /// based on the provided URLRequest and Span parameters. /// - /// The callback provides access to the URLRequest and Span, allowing you to enrich the baggage - /// content based on request details or span context. + /// The resulting baggage is injected into request headers using the configured `TextMapBaggagePropagator`, + /// ensuring consistent propagation across requests, regardless of the active context. /// /// Note: The injected baggage depends on the propagator in use (e.g., W3C or custom). - /// Returns: A `Baggage` instance or `nil` if no default baggage is needed. + /// Returns: A `Baggage` instance or `nil` if no baggage is needed. public let baggageProvider: ((URLRequest, Span?) -> (Baggage)?)? } From 55fc822d8a6f764dce2956b60a77179ea0dcbdc3 Mon Sep 17 00:00:00 2001 From: Batuhan Saka Date: Fri, 29 Nov 2024 18:24:53 +0100 Subject: [PATCH 3/5] update README.md for `URLSessionInstrumentation` pkg --- Sources/Instrumentation/URLSession/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Instrumentation/URLSession/README.md b/Sources/Instrumentation/URLSession/README.md index 063ef1ae..2c3cf913 100644 --- a/Sources/Instrumentation/URLSession/README.md +++ b/Sources/Instrumentation/URLSession/README.md @@ -28,4 +28,5 @@ This behaviour can be modified or augmented by using the optional callbacks defi `receivedError: ((Error, DataOrFile?, HTTPStatus, Span) -> Void)?` - Called after an error is received, it allows to add extra information to the Span +`baggageProvider: ((URLRequest, Span) -> (Baggage)?)?`: Provides baggage instance for instrumented requests that is merged with active baggage. The callback receives URLRequest and Span parameters to create dynamic baggage based on request context. The resulting baggage is injected into request headers using the configured propagator. From 9e7e79133248c8fc77b742b7a7a64dc5e67194c1 Mon Sep 17 00:00:00 2001 From: Batuhan Saka Date: Fri, 20 Dec 2024 10:05:35 +0100 Subject: [PATCH 4/5] mark `URLRequest` as inout parameter --- .../URLSession/URLSessionInstrumentationConfiguration.swift | 4 ++-- Sources/Instrumentation/URLSession/URLSessionLogger.swift | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift b/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift index 331997ba..424191a1 100644 --- a/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift +++ b/Sources/Instrumentation/URLSession/URLSessionInstrumentationConfiguration.swift @@ -25,7 +25,7 @@ public struct URLSessionInstrumentationConfiguration { receivedResponse: ((URLResponse, DataOrFile?, Span) -> Void)? = nil, receivedError: ((Error, DataOrFile?, HTTPStatus, Span) -> Void)? = nil, delegateClassesToInstrument: [AnyClass]? = nil, - baggageProvider: ((URLRequest, Span?) -> (Baggage)?)? = nil) + baggageProvider: ((inout URLRequest, Span?) -> (Baggage)?)? = nil) { self.shouldRecordPayload = shouldRecordPayload self.shouldInstrument = shouldInstrument @@ -85,6 +85,6 @@ public struct URLSessionInstrumentationConfiguration { /// /// Note: The injected baggage depends on the propagator in use (e.g., W3C or custom). /// Returns: A `Baggage` instance or `nil` if no baggage is needed. - public let baggageProvider: ((URLRequest, Span?) -> (Baggage)?)? + public let baggageProvider: ((inout URLRequest, Span?) -> (Baggage)?)? } diff --git a/Sources/Instrumentation/URLSession/URLSessionLogger.swift b/Sources/Instrumentation/URLSession/URLSessionLogger.swift index 4eef4041..92a6b48c 100644 --- a/Sources/Instrumentation/URLSession/URLSessionLogger.swift +++ b/Sources/Instrumentation/URLSession/URLSessionLogger.swift @@ -157,12 +157,12 @@ class URLSessionLogger { return nil } instrumentation.configuration.injectCustomHeaders?(&request, span) + let customBaggage = instrumentation.configuration.baggageProvider?(&request, span) + var instrumentedRequest = request objc_setAssociatedObject(instrumentedRequest, URLSessionInstrumentation.instrumentedKey, true, .OBJC_ASSOCIATION_COPY_NONATOMIC) let propagators = OpenTelemetry.instance.propagators - let customBaggage = instrumentation.configuration.baggageProvider?(request, span) - var traceHeaders = tracePropagationHTTPHeaders(span: span, customBaggage: customBaggage, textMapPropagator: propagators.textMapPropagator, From 0e5f7ff36438be714101764e7f01174772397c6a Mon Sep 17 00:00:00 2001 From: Batuhan Saka Date: Fri, 20 Dec 2024 10:07:18 +0100 Subject: [PATCH 5/5] update Instrumentation/URLSession/README.md --- Sources/Instrumentation/URLSession/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Instrumentation/URLSession/README.md b/Sources/Instrumentation/URLSession/README.md index 2c3cf913..7bca6998 100644 --- a/Sources/Instrumentation/URLSession/README.md +++ b/Sources/Instrumentation/URLSession/README.md @@ -28,5 +28,5 @@ This behaviour can be modified or augmented by using the optional callbacks defi `receivedError: ((Error, DataOrFile?, HTTPStatus, Span) -> Void)?` - Called after an error is received, it allows to add extra information to the Span -`baggageProvider: ((URLRequest, Span) -> (Baggage)?)?`: Provides baggage instance for instrumented requests that is merged with active baggage. The callback receives URLRequest and Span parameters to create dynamic baggage based on request context. The resulting baggage is injected into request headers using the configured propagator. +`baggageProvider: ((inout URLRequest, Span) -> (Baggage)?)?`: Provides baggage instance for instrumented requests that is merged with active baggage. The callback receives URLRequest and Span parameters to create dynamic baggage based on request context. The resulting baggage is injected into request headers using the configured propagator.