Merge 807fce2 into 25d925a

getsentry · Jan 4, 2024 · 6f034a1 · 6f034a1
2 parents 25d925a + 807fce2
commit 6f034a1
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@
 - Use correct rendered frames timestamp for TTID/TTFD and app start (#3531)
 
 - Missing transactions when not calling `reportFullyDisplayed` (#3477)
+- Don't override `sentry-trace` and `baggage` headers (#3540)
 
 ## 8.17.2
 

diff --git a/Sources/Sentry/SentryBaggage.m b/Sources/Sentry/SentryBaggage.m
@@ -34,11 +34,6 @@ - (instancetype)initWithTraceId:(SentryId *)traceId
     return self;
 }
 
-- (NSString *)toHTTPHeader
-{
-    return [self toHTTPHeaderWithOriginalBaggage:nil];
-}
-
 - (NSString *)toHTTPHeaderWithOriginalBaggage:(NSDictionary *_Nullable)originalBaggage
 {
     NSMutableDictionary *information

diff --git a/Sources/Sentry/SentryNetworkTracker.m b/Sources/Sentry/SentryNetworkTracker.m
@@ -239,19 +239,23 @@ - (void)addBaggageHeader:(SentryBaggage *)baggage
     NSString *baggageHeader = @"";
 
     if (baggage != nil) {
-        baggageHeader =
-            [baggage toHTTPHeaderWithOriginalBaggage:
-                         [SentrySerialization
-                             decodeBaggage:sessionTask.currentRequest
-                                               .allHTTPHeaderFields[SENTRY_BAGGAGE_HEADER]]];
+        NSDictionary *originalBaggage = [SentrySerialization
+            decodeBaggage:sessionTask.currentRequest.allHTTPHeaderFields[SENTRY_BAGGAGE_HEADER]];
+
+        if (originalBaggage[@"sentry-trace_id"] == nil) {
+            baggageHeader = [baggage toHTTPHeaderWithOriginalBaggage:originalBaggage];
+        }
     }
 
     // First we check if the current request is mutable, so we could easily add a new
     // header. Otherwise we try to change the current request for a new one with the extra
     // header.
     if ([sessionTask.currentRequest isKindOfClass:[NSMutableURLRequest class]]) {
         NSMutableURLRequest *currentRequest = (NSMutableURLRequest *)sessionTask.currentRequest;
-        [currentRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER];
+
+        if ([currentRequest valueForHTTPHeaderField:SENTRY_TRACE_HEADER] == nil) {
+            [currentRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER];
+        }
 
         if (baggageHeader.length > 0) {
             [currentRequest setValue:baggageHeader forHTTPHeaderField:SENTRY_BAGGAGE_HEADER];
@@ -265,7 +269,9 @@ - (void)addBaggageHeader:(SentryBaggage *)baggage
         if ([sessionTask respondsToSelector:setCurrentRequestSelector]) {
             NSMutableURLRequest *newRequest = [sessionTask.currentRequest mutableCopy];
 
-            [newRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER];
+            if ([newRequest valueForHTTPHeaderField:SENTRY_TRACE_HEADER] == nil) {
+                [newRequest setValue:traceHeader.value forHTTPHeaderField:SENTRY_TRACE_HEADER];
+            }
 
             if (baggageHeader.length > 0) {
                 [newRequest setValue:baggageHeader forHTTPHeaderField:SENTRY_BAGGAGE_HEADER];

diff --git a/Sources/Sentry/include/SentryBaggage.h b/Sources/Sentry/include/SentryBaggage.h
@@ -63,7 +63,6 @@ static NSString *const SENTRY_BAGGAGE_HEADER = @"baggage";
                      sampleRate:(nullable NSString *)sampleRate
                         sampled:(nullable NSString *)sampled;
 
-- (NSString *)toHTTPHeader;
 - (NSString *)toHTTPHeaderWithOriginalBaggage:(NSDictionary *_Nullable)originalBaggage;
 
 @end

diff --git a/...s/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTests.swift b/...s/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTests.swift
@@ -164,7 +164,7 @@ class SentryNetworkTrackerIntegrationTests: XCTestCase {
             self.assertNetworkError(error)
             let response = String(data: data ?? Data(), encoding: .utf8) ?? ""
 
-            let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader()
+            let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil)
             XCTAssertEqual(expectedBaggageHeader, response)
 
             expect.fulfill()

diff --git a/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift
@@ -565,9 +565,21 @@ class SentryNetworkTrackerTests: XCTestCase {
         let transaction = startTransaction() as! SentryTracer
         sut.urlSessionTaskResume(task)
 
-        let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader()
+        let expectedBaggageHeader = transaction.traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader)
     }
+
+    func testDontOverrideBaggageHeader() {
+        let sut = fixture.getSut()
+        let task = createDataTask {
+            var request = $0
+            request.setValue("sentry-trace_id=something", forHTTPHeaderField: "baggage")
+            return request
+        }
+        sut.urlSessionTaskResume(task)
+
+        XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", "sentry-trace_id=something")
+    }
 
     func testTraceHeader() {
         let sut = fixture.getSut()
@@ -580,6 +592,18 @@ class SentryNetworkTrackerTests: XCTestCase {
         let expectedTraceHeader = networkSpan.toTraceHeader().value()
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader)
     }
+
+    func testDontOverrideTraceHeader() {
+        let sut = fixture.getSut()
+        let task = createDataTask {
+            var request = $0
+            request.setValue("test", forHTTPHeaderField: "sentry-trace")
+            return request
+        }
+        sut.urlSessionTaskResume(task)
+
+        XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", "test")
+    }
 
     func testDefaultHeadersWhenDisabled() {
         let sut = fixture.getSut()
@@ -591,7 +615,7 @@ class SentryNetworkTrackerTests: XCTestCase {
 
         let expectedTraceHeader = SentrySDK.currentHub().scope.propagationContext.traceHeader.value()
         let traceContext = SentryTraceContext(trace: SentrySDK.currentHub().scope.propagationContext.traceId, options: self.fixture.options, userSegment: self.fixture.scope.userObject?.segment)
-        let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader()
+        let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader)
     }
@@ -603,7 +627,7 @@ class SentryNetworkTrackerTests: XCTestCase {
 
         let expectedTraceHeader = SentrySDK.currentHub().scope.propagationContext.traceHeader.value()
         let traceContext = SentryTraceContext(trace: SentrySDK.currentHub().scope.propagationContext.traceId, options: self.fixture.options, userSegment: self.fixture.scope.userObject?.segment)
-        let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader()
+        let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader(withOriginalBaggage: nil)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader)
     }
@@ -897,9 +921,12 @@ class SentryNetworkTrackerTests: XCTestCase {
         XCTAssertEqual(duration, expectedDuration)
     }
 
-    func createDataTask(method: String = "GET") -> URLSessionDataTaskMock {
+    func createDataTask(method: String = "GET", modifyRequest: ((URLRequest) -> (URLRequest))? = nil) -> URLSessionDataTaskMock {
         var request = URLRequest(url: SentryNetworkTrackerTests.fullUrl)
         request.httpMethod = method
+        if let modifyRequest = modifyRequest {
+           request = modifyRequest(request)
+        }
         return URLSessionDataTaskMock(request: request)
     }
 

diff --git a/Tests/SentryTests/Transaction/SentryBaggageTests.swift b/Tests/SentryTests/Transaction/SentryBaggageTests.swift
@@ -3,20 +3,14 @@ import Sentry
 import XCTest
 
 class SentryBaggageTests: XCTestCase {
-    func test_baggageToHeader() {
-        let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: "release name", environment: "teste", transaction: "transaction", userSegment: "test user", sampleRate: "0.49", sampled: "true").toHTTPHeader()
-
-        XCTAssertEqual(header, "sentry-environment=teste,sentry-public_key=publicKey,sentry-release=release%20name,sentry-sample_rate=0.49,sentry-sampled=true,sentry-trace_id=00000000000000000000000000000000,sentry-transaction=transaction,sentry-user_segment=test%20user")
-    }
-
     func test_baggageToHeader_AppendToOriginal() {
         let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: "release name", environment: "teste", transaction: "transaction", userSegment: "test user", sampleRate: "0.49", sampled: "true").toHTTPHeader(withOriginalBaggage: ["a": "a", "sentry-trace_id": "to-be-overwritten"])
 
         XCTAssertEqual(header, "a=a,sentry-environment=teste,sentry-public_key=publicKey,sentry-release=release%20name,sentry-sample_rate=0.49,sentry-sampled=true,sentry-trace_id=00000000000000000000000000000000,sentry-transaction=transaction,sentry-user_segment=test%20user")
     }
 
     func test_baggageToHeader_onlyTrace_ignoreNils() {
-        let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: nil, environment: nil, transaction: nil, userSegment: nil, sampleRate: nil, sampled: nil).toHTTPHeader()
+        let header = SentryBaggage(trace: SentryId.empty, publicKey: "publicKey", releaseName: nil, environment: nil, transaction: nil, userSegment: nil, sampleRate: nil, sampled: nil).toHTTPHeader(withOriginalBaggage: nil)
 
         XCTAssertEqual(header, "sentry-public_key=publicKey,sentry-trace_id=00000000000000000000000000000000")
     }