impr: Capture transactions on a bg thread (#3892)

Instead of capturing and serializing the transaction on the calling thread, the SDK now does this on a background thread. Fixes GH-3826
getsentry · Apr 29, 2024 · 38b36f5 · 38b36f5
1 parent c8dbe73
commit 38b36f5
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@
 
 - Don't transmit device boot time in envelopes enriched with crash data (#3912)
 
+### Improvements
+
+- Capture transactions on a background thread (#3892)
+
 ## 8.25.0-alpha.0
 
 ### Features

diff --git a/Samples/iOS-Swift/iOS-Swift/ViewControllers/TraceTestViewController.swift b/Samples/iOS-Swift/iOS-Swift/ViewControllers/TraceTestViewController.swift
@@ -68,15 +68,11 @@ class TraceTestViewController: UIViewController {
                 return
             }
 
-            guard let child = children.first(where: { $0.operation == "http.client" }) else {
-                UIAssert.fail("Did not found http request child")
+            guard let child = children.first(where: { $0.operation == "http.client" && $0.data["url"] as? String == "https://sentry-brand.storage.googleapis.com/sentry-logo-black.png" && $0.data["http.response.status_code"] as? String == "200" }) else {
+                UIAssert.fail("Did not find child span for HTTP for retrieving the sentry brand logo.")
                 return
             }
 
-            UIAssert.isEqual(child.data["url"] as? String, "https://sentry-brand.storage.googleapis.com/sentry-logo-black.png", "Could not read url data value")
-
-            UIAssert.isEqual(child.data["http.response.status_code"] as? String, "200", "Could not read status_code tag value")
-
             UIAssert.checkForViewControllerLifeCycle(span, viewController: "TraceTestViewController", stepsToCheck: self.lifeCycleSteps)
         }
     }

diff --git a/Sources/Sentry/SentryHub.m b/Sources/Sentry/SentryHub.m
@@ -40,6 +40,7 @@
 
 @property (nullable, nonatomic, strong) SentryClient *client;
 @property (nullable, nonatomic, strong) SentryScope *scope;
+@property (nonatomic) SentryDispatchQueueWrapper *dispatchQueue;
 @property (nonatomic, strong) SentryCrashWrapper *crashWrapper;
 @property (nonatomic, strong) NSMutableSet<NSString *> *installedIntegrationNames;
 @property (nonatomic) NSUInteger errorsBeforeSession;
@@ -57,14 +58,15 @@ - (instancetype)initWithClient:(nullable SentryClient *)client
     if (self = [super init]) {
         _client = client;
         _scope = scope;
+        _dispatchQueue = SentryDependencyContainer.sharedInstance.dispatchQueueWrapper;
         SentryStatsdClient *statsdClient = [[SentryStatsdClient alloc] initWithClient:client];
         SentryMetricsClient *metricsClient =
             [[SentryMetricsClient alloc] initWithClient:statsdClient];
         _metrics = [[SentryMetricsAPI alloc]
              initWithEnabled:client.options.enableMetrics
                       client:metricsClient
                  currentDate:SentryDependencyContainer.sharedInstance.dateProvider
-               dispatchQueue:SentryDependencyContainer.sharedInstance.dispatchQueueWrapper
+               dispatchQueue:_dispatchQueue
                       random:SentryDependencyContainer.sharedInstance.random
             beforeEmitMetric:client.options.beforeEmitMetric];
         [_metrics setDelegate:self];
@@ -85,9 +87,11 @@ - (instancetype)initWithClient:(nullable SentryClient *)client
 - (instancetype)initWithClient:(nullable SentryClient *)client
                       andScope:(nullable SentryScope *)scope
                andCrashWrapper:(SentryCrashWrapper *)crashWrapper
+              andDispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue
 {
     self = [self initWithClient:client andScope:scope];
     _crashWrapper = crashWrapper;
+    _dispatchQueue = dispatchQueue;
 
     return self;
 }
@@ -268,25 +272,31 @@ - (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope
     }
 }
 
-- (SentryId *)captureTransaction:(SentryTransaction *)transaction withScope:(SentryScope *)scope
+- (void)captureTransaction:(SentryTransaction *)transaction withScope:(SentryScope *)scope
 {
-    return [self captureTransaction:transaction withScope:scope additionalEnvelopeItems:@[]];
+    [self captureTransaction:transaction withScope:scope additionalEnvelopeItems:@[]];
 }
 
-- (SentryId *)captureTransaction:(SentryTransaction *)transaction
-                       withScope:(SentryScope *)scope
-         additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems
+- (void)captureTransaction:(SentryTransaction *)transaction
+                  withScope:(SentryScope *)scope
+    additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems
 {
     SentrySampleDecision decision = transaction.trace.sampled;
     if (decision != kSentrySampleDecisionYes) {
         [self.client recordLostEvent:kSentryDataCategoryTransaction
                               reason:kSentryDiscardReasonSampleRate];
-        return SentryId.empty;
+        return;
     }
 
-    return [self captureEvent:transaction
-                      withScope:scope
-        additionalEnvelopeItems:additionalEnvelopeItems];
+    // When a user calls finish on a transaction, which calls captureTransaction, the calling thread
+    // here could be the main thread, which we only want to block as long as required. Therefore, we
+    // capture the transaction on a background thread.
+    __weak SentryHub *weakSelf = self;
+    [self.dispatchQueue dispatchAsyncWithBlock:^{
+        [weakSelf captureEvent:transaction
+                          withScope:scope
+            additionalEnvelopeItems:additionalEnvelopeItems];
+    }];
 }
 
 - (SentryId *)captureEvent:(SentryEvent *)event

diff --git a/Sources/Sentry/include/SentryHub+Private.h b/Sources/Sentry/include/SentryHub+Private.h
@@ -54,11 +54,11 @@ SentryHub ()
     additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems
     NS_SWIFT_NAME(capture(event:scope:additionalEnvelopeItems:));
 
-- (SentryId *)captureTransaction:(SentryTransaction *)transaction withScope:(SentryScope *)scope;
+- (void)captureTransaction:(SentryTransaction *)transaction withScope:(SentryScope *)scope;
 
-- (SentryId *)captureTransaction:(SentryTransaction *)transaction
-                       withScope:(SentryScope *)scope
-         additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems;
+- (void)captureTransaction:(SentryTransaction *)transaction
+                  withScope:(SentryScope *)scope
+    additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems;
 
 - (void)captureEnvelope:(SentryEnvelope *)envelope;
 

diff --git a/Tests/SentryTests/Integrations/Session/SentrySessionGeneratorTests.swift b/Tests/SentryTests/Integrations/Session/SentrySessionGeneratorTests.swift
@@ -142,7 +142,7 @@ class SentrySessionGeneratorTests: NotificationCenterTestCase {
 
         sentryCrash = TestSentryCrashWrapper.sharedInstance()
         let client = SentrySDK.currentHub().getClient()
-        let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: self.sentryCrash)
+        let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: self.sentryCrash, andDispatchQueue: SentryDispatchQueueWrapper())
         SentrySDK.setCurrentHub(hub)
 
         crashIntegration = SentryCrashIntegration(crashAdapter: sentryCrash, andDispatchQueueWrapper: TestSentryDispatchQueueWrapper())

diff --git a/Tests/SentryTests/Integrations/Session/SentrySessionTrackerTests.swift b/Tests/SentryTests/Integrations/Session/SentrySessionTrackerTests.swift
@@ -34,7 +34,7 @@ class SentrySessionTrackerTests: XCTestCase {
         }
 
         func setNewHubToSDK() {
-            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: self.sentryCrash)
+            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: self.sentryCrash, andDispatchQueue: SentryDispatchQueueWrapper())
             SentrySDK.setCurrentHub(hub)
         }
     }

diff --git a/...yTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsIntegrationTests.swift b/...yTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsIntegrationTests.swift
@@ -22,7 +22,7 @@ class SentryWatchdogTerminationIntegrationTests: XCTestCase {
             SentryDependencyContainer.sharedInstance().crashWrapper = crashWrapper
             SentryDependencyContainer.sharedInstance().fileManager = try! SentryFileManager(options: options, dispatchQueueWrapper: TestSentryDispatchQueueWrapper())
 
-            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: crashWrapper)
+            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: crashWrapper, andDispatchQueue: SentryDispatchQueueWrapper())
             SentrySDK.setCurrentHub(hub)
 
             fileManager = try! SentryFileManager(options: options, dispatchQueueWrapper: dispatchQueue)

diff --git a/...entryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsTrackerTests.swift b/...entryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsTrackerTests.swift
@@ -29,7 +29,7 @@ class SentryWatchdogTerminationTrackerTests: NotificationCenterTestCase {
 
             crashWrapper = TestSentryCrashWrapper.sharedInstance()
 
-            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: crashWrapper)
+            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: crashWrapper, andDispatchQueue: SentryDispatchQueueWrapper())
             SentrySDK.setCurrentHub(hub)
         }
 

diff --git a/Tests/SentryTests/SentryHubTests.swift b/Tests/SentryTests/SentryHubTests.swift
@@ -26,6 +26,7 @@ class SentryHubTests: XCTestCase {
         let traceOrigin = "auto"
         let random = TestRandom(value: 0.5)
         let queue = DispatchQueue(label: "SentryHubTests", qos: .utility, attributes: [.concurrent])
+        let dispatchQueueWrapper = TestSentryDispatchQueueWrapper()
 
         init() {
             options = Options()
@@ -52,7 +53,7 @@ class SentryHubTests: XCTestCase {
         }
 
         func getSut(_ options: Options, _ scope: Scope? = nil) -> SentryHub {
-            let hub = SentryHub(client: client, andScope: scope, andCrashWrapper: sentryCrash)
+            let hub = SentryHub(client: client, andScope: scope, andCrashWrapper: sentryCrash, andDispatchQueue: self.dispatchQueueWrapper)
             hub.bindClient(client)
             return hub
         }
@@ -354,12 +355,23 @@ class SentryHubTests: XCTestCase {
         XCTAssertEqual(span.sampled, .no)
     }
 
-    func testCaptureSampledTransaction_ReturnsEmptyId() {
+    func testCaptureTransaction_CapturesEventAsync() {
+        let transaction = sut.startTransaction(transactionContext: TransactionContext(name: fixture.transactionName, operation: fixture.transactionOperation, sampled: .yes))
+
+        let trans = Dynamic(transaction).toTransaction().asAnyObject
+        sut.capture(trans as! Transaction, with: Scope())
+
+        expect(self.fixture.client.captureEventWithScopeInvocations.count) == 1
+        expect(self.fixture.dispatchQueueWrapper.dispatchAsyncInvocations.count) == 1
+    }
+
+    func testCaptureSampledTransaction_DoesNotCaptureEvent() {
         let transaction = sut.startTransaction(transactionContext: TransactionContext(name: fixture.transactionName, operation: fixture.transactionOperation, sampled: .no))
 
         let trans = Dynamic(transaction).toTransaction().asAnyObject
-        let id = sut.capture(trans as! Transaction, with: Scope())
-        id.assertIsEmpty()
+        sut.capture(trans as! Transaction, with: Scope())
+
+        expect(self.fixture.client.captureEventWithScopeInvocations.count) == 0
     }
 
     func testCaptureSampledTransaction_RecordsLostEvent() {

diff --git a/Tests/SentryTests/SentrySDKIntegrationTestsBase.swift b/Tests/SentryTests/SentrySDKIntegrationTestsBase.swift
@@ -26,7 +26,7 @@ class SentrySDKIntegrationTestsBase: XCTestCase {
 
     func givenSdkWithHub(_ options: Options? = nil, scope: Scope = Scope()) {
         let client = TestClient(options: options ?? self.options)
-        let hub = SentryHub(client: client, andScope: scope, andCrashWrapper: TestSentryCrashWrapper.sharedInstance())
+        let hub = SentryHub(client: client, andScope: scope, andCrashWrapper: TestSentryCrashWrapper.sharedInstance(), andDispatchQueue: SentryDispatchQueueWrapper())
 
         SentrySDK.setCurrentHub(hub)
     }

diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift
@@ -47,7 +47,7 @@ class SentrySDKTests: XCTestCase {
             options.releaseName = "1.0.0"
 
             client = TestClient(options: options)!
-            hub = SentryHub(client: client, andScope: scope, andCrashWrapper: TestSentryCrashWrapper.sharedInstance())
+            hub = SentryHub(client: client, andScope: scope, andCrashWrapper: TestSentryCrashWrapper.sharedInstance(), andDispatchQueue: SentryDispatchQueueWrapper())
 
             userFeedback = UserFeedback(eventId: SentryId())
             userFeedback.comments = "Again really?"

diff --git a/Tests/SentryTests/State/SentryHub+Test.h b/Tests/SentryTests/State/SentryHub+Test.h
@@ -2,6 +2,7 @@
 
 @class SentryClient;
 @class SentryCrashWrapper;
+@class SentryDispatchQueueWrapper;
 @protocol SentryIntegrationProtocol;
 NS_ASSUME_NONNULL_BEGIN
 
@@ -11,7 +12,8 @@ SentryHub ()
 
 - (instancetype)initWithClient:(SentryClient *_Nullable)client
                       andScope:(SentryScope *_Nullable)scope
-               andCrashWrapper:(SentryCrashWrapper *)crashAdapter;
+               andCrashWrapper:(SentryCrashWrapper *)crashAdapter
+              andDispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue;
 
 - (NSArray<id<SentryIntegrationProtocol>> *)installedIntegrations;
 - (NSSet<NSString *> *)installedIntegrationNames;

diff --git a/Tests/SentryTests/State/TestHub.swift b/Tests/SentryTests/State/TestHub.swift
@@ -40,8 +40,8 @@ class TestHub: SentryHub {
     }
 
     var capturedTransactionsWithScope = Invocations<(transaction: [String: Any], scope: Scope)>()
-    override func capture(_ transaction: Transaction, with scope: Scope) -> SentryId {
+    override func capture(_ transaction: Transaction, with scope: Scope) {
         capturedTransactionsWithScope.record((transaction.serialize(), scope))
-        return super.capture(transaction, with: scope)
+        super.capture(transaction, with: scope)
     }
 }
diff --git a/Tests/SentryTests/Transaction/SentrySpanTests.swift b/Tests/SentryTests/Transaction/SentrySpanTests.swift
@@ -34,7 +34,7 @@ class SentrySpanTests: XCTestCase {
         }
 
         func getSut(client: SentryClient) -> Span {
-            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: TestSentryCrashWrapper.sharedInstance())
+            let hub = SentryHub(client: client, andScope: nil, andCrashWrapper: TestSentryCrashWrapper.sharedInstance(), andDispatchQueue: TestSentryDispatchQueueWrapper())
             return hub.startTransaction(name: someTransaction, operation: someOperation)
         }