Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impr: Capture transactions on a bg thread #3892

Merged
merged 5 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 20 additions & 10 deletions Sources/Sentry/SentryHub.m
Original file line number Diff line number Diff line change
Expand Up @@ -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<id<SentryIntegrationProtocol>> *installedIntegrations;
@property (nonatomic, strong) NSMutableSet<NSString *> *installedIntegrationNames;
Expand All @@ -58,14 +59,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];
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<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
Expand Down
8 changes: 4 additions & 4 deletions Sources/Sentry/include/SentryHub+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,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
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems;

- (void)captureEnvelope:(SentryEnvelope *)envelope;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@

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())

Check warning on line 145 in Tests/SentryTests/Integrations/Session/SentrySessionGeneratorTests.swift

View check run for this annotation

Codecov / codecov/patch

Tests/SentryTests/Integrations/Session/SentrySessionGeneratorTests.swift#L145

Added line #L145 was not covered by tests
SentrySDK.setCurrentHub(hub)

crashIntegration = SentryCrashIntegration(crashAdapter: sentryCrash, andDispatchQueueWrapper: TestSentryDispatchQueueWrapper())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
20 changes: 16 additions & 4 deletions Tests/SentryTests/SentryHubTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryTests/SentrySDKIntegrationTestsBase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryTests/SentrySDKTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?"
Expand Down
4 changes: 3 additions & 1 deletion Tests/SentryTests/State/SentryHub+Test.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

@class SentryClient;
@class SentryCrashWrapper;
@class SentryDispatchQueueWrapper;
@protocol SentryIntegrationProtocol;
NS_ASSUME_NONNULL_BEGIN

Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions Tests/SentryTests/State/TestHub.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion Tests/SentryTests/Transaction/SentrySpanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Loading