diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cf255edf6e..3a5949b98d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ - Save framework without UIKit/AppKit as Github Asset for releases (#3858) - Fix crash associated with runtime collision in global C function names (#3862) +### Improvements + +- Capture transactions on a background thread (#3892) + ## 8.24.0 ### Features diff --git a/Sources/Sentry/SentryHub.m b/Sources/Sentry/SentryHub.m index d5628492a22..3f4d7fa1909 100644 --- 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) NSMutableArray> *installedIntegrations; @property (nonatomic, strong) NSMutableSet *installedIntegrationNames; @@ -58,6 +59,7 @@ - (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]; @@ -65,7 +67,7 @@ - (instancetype)initWithClient:(nullable SentryClient *)client 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]; @@ -86,9 +88,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; } @@ -269,25 +273,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 *)additionalEnvelopeItems +- (void)captureTransaction:(SentryTransaction *)transaction + withScope:(SentryScope *)scope + additionalEnvelopeItems:(NSArray *)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 index 0a95cf674cc..7072b2b278d 100644 --- a/Sources/Sentry/include/SentryHub+Private.h +++ b/Sources/Sentry/include/SentryHub+Private.h @@ -52,11 +52,11 @@ SentryHub () additionalEnvelopeItems:(NSArray *)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 *)additionalEnvelopeItems; +- (void)captureTransaction:(SentryTransaction *)transaction + withScope:(SentryScope *)scope + additionalEnvelopeItems:(NSArray *)additionalEnvelopeItems; - (void)captureEnvelope:(SentryEnvelope *)envelope; diff --git a/Tests/SentryTests/Integrations/Session/SentrySessionGeneratorTests.swift b/Tests/SentryTests/Integrations/Session/SentrySessionGeneratorTests.swift index 04c9fc7288f..d9e2d8bc297 100644 --- 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 index 21fe8accef1..e8c55836252 100644 --- 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/Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsIntegrationTests.swift b/Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsIntegrationTests.swift index 786cb130e96..4600de49051 100644 --- a/Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsIntegrationTests.swift +++ b/Tests/SentryTests/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/Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsTrackerTests.swift b/Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsTrackerTests.swift index e122421e938..8ac5c8771c8 100644 --- a/Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationsTrackerTests.swift +++ b/Tests/SentryTests/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 index 18cf26b0931..04b59bacb96 100644 --- 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 index cef6df91dc8..7a54a6c47db 100644 --- 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 index 50dec90fff0..ab48269348d 100644 --- 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 index 9fce2d64ca3..94f4f8a02a0 100644 --- 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> *)installedIntegrations; - (NSSet *)installedIntegrationNames; diff --git a/Tests/SentryTests/State/TestHub.swift b/Tests/SentryTests/State/TestHub.swift index c66c452063f..56affc451c7 100644 --- 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 index 59a3a4a575a..fa6a08e94ea 100644 --- 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) }