diff --git a/CHANGELOG.md b/CHANGELOG.md index ea220b54eae..fde42b354e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - TTFD waits for next drawn frame (#3505) - Fix TTID/TTFD for app start transactions (#3512): TTID/TTFD spans and measurements for app start transaction now include the app start duration. - Fix a race condition in SentryTracer (#3523) +- Missing transactions when not calling `reportFullyDisplayed` (#3477) ## 8.17.2 diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index 6ae6ee7e959..5a33a615d4b 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -70,6 +70,7 @@ @property (nonatomic) BOOL wasFinishCalled; @property (nonatomic, nullable, strong) NSTimer *deadlineTimer; @property (nonnull, strong) SentryTracerConfiguration *configuration; +@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue; #if SENTRY_TARGET_PROFILING_SUPPORTED @property (nonatomic) BOOL isProfiling; @@ -132,6 +133,7 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti _startSystemTime = SentryDependencyContainer.sharedInstance.dateProvider.systemTime; _configuration = configuration; + _dispatchQueue = SentryDependencyContainer.sharedInstance.dispatchQueueWrapper; self.transactionContext = transactionContext; _children = [[NSMutableArray alloc] init]; @@ -198,10 +200,10 @@ - (void)dispatchIdleTimeout { @synchronized(_idleTimeoutLock) { if (_idleTimeoutBlock != NULL) { - [_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock]; + [_dispatchQueue dispatchCancel:_idleTimeoutBlock]; } __weak SentryTracer *weakSelf = self; - _idleTimeoutBlock = [_configuration.dispatchQueueWrapper createDispatchBlock:^{ + _idleTimeoutBlock = [_dispatchQueue createDispatchBlock:^{ if (weakSelf == nil) { SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything."); return; @@ -215,15 +217,14 @@ - (void)dispatchIdleTimeout // If the transaction has no children, the SDK will discard it. [self finishInternal]; } else { - [_configuration.dispatchQueueWrapper dispatchAfter:_configuration.idleTimeout - block:_idleTimeoutBlock]; + [_dispatchQueue dispatchAfter:_configuration.idleTimeout block:_idleTimeoutBlock]; } } } - (BOOL)hasIdleTimeout { - return _configuration.idleTimeout > 0 && _configuration.dispatchQueueWrapper != nil; + return _configuration.idleTimeout > 0; } - (BOOL)isAutoGeneratedTransaction @@ -235,7 +236,8 @@ - (void)cancelIdleTimeout { @synchronized(_idleTimeoutLock) { if ([self hasIdleTimeout]) { - [_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock]; + [SentryDependencyContainer.sharedInstance.dispatchQueueWrapper + dispatchCancel:_idleTimeoutBlock]; } } } @@ -243,7 +245,7 @@ - (void)cancelIdleTimeout - (void)startDeadlineTimer { __weak SentryTracer *weakSelf = self; - [_configuration.dispatchQueueWrapper dispatchOnMainQueue:^{ + [_dispatchQueue dispatchOnMainQueue:^{ weakSelf.deadlineTimer = [weakSelf.configuration.timerFactory scheduledTimerWithTimeInterval:SENTRY_AUTO_TRANSACTION_DEADLINE repeats:NO @@ -285,7 +287,7 @@ - (void)cancelDeadlineTimer // The timer must be invalidated from the thread on which the timer was installed, see // https://developer.apple.com/documentation/foundation/nstimer/1415405-invalidate#1770468 - [_configuration.dispatchQueueWrapper dispatchOnMainQueue:^{ + [_dispatchQueue dispatchOnMainQueue:^{ if (weakSelf == nil) { SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not invalidating deadlineTimer."); return; @@ -454,7 +456,9 @@ - (void)canBeFinished } if (!self.wasFinishCalled || hasUnfinishedChildSpansToWaitFor) { - SENTRY_LOG_DEBUG(@"Span with id %@ has children but isn't waiting for them right now.", + SENTRY_LOG_DEBUG( + @"Span with id %@ has children but hasn't finished yet so isn't waiting " + @"for them right now.", self.spanId.sentrySpanIdString); return; } diff --git a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m index 781fc808598..095040c1a49 100644 --- a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m +++ b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m @@ -19,7 +19,6 @@ @interface SentryUIEventTrackerTransactionMode () -@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueueWrapper; @property (nonatomic, assign) NSTimeInterval idleTimeout; @property (nullable, nonatomic, strong) NSMutableArray *activeTransactions; @@ -27,11 +26,9 @@ @implementation SentryUIEventTrackerTransactionMode -- (instancetype)initWithDispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper - idleTimeout:(NSTimeInterval)idleTimeout +- (instancetype)initWithIdleTimeout:(NSTimeInterval)idleTimeout { if (self = [super init]) { - self.dispatchQueueWrapper = dispatchQueueWrapper; self.idleTimeout = idleTimeout; self.activeTransactions = [NSMutableArray new]; } @@ -91,7 +88,6 @@ - (void)handleUIEvent:(NSString *)action SentryTracerConfiguration *config) { config.idleTimeout = self.idleTimeout; config.waitForChildren = YES; - config.dispatchQueueWrapper = self.dispatchQueueWrapper; }]]; SENTRY_LOG_DEBUG(@"Automatically started a new transaction with name: " diff --git a/Sources/Sentry/SentryUIEventTrackingIntegration.m b/Sources/Sentry/SentryUIEventTrackingIntegration.m index 130b3792b3a..e35db492bb5 100644 --- a/Sources/Sentry/SentryUIEventTrackingIntegration.m +++ b/Sources/Sentry/SentryUIEventTrackingIntegration.m @@ -2,7 +2,6 @@ #if SENTRY_HAS_UIKIT -# import # import # import # import @@ -25,10 +24,8 @@ - (BOOL)installWithOptions:(SentryOptions *)options return NO; } - SentryDependencyContainer *dependencies = [SentryDependencyContainer sharedInstance]; - SentryUIEventTrackerTransactionMode *mode = [[SentryUIEventTrackerTransactionMode alloc] - initWithDispatchQueueWrapper:dependencies.dispatchQueueWrapper - idleTimeout:options.idleTimeout]; + SentryUIEventTrackerTransactionMode *mode = + [[SentryUIEventTrackerTransactionMode alloc] initWithIdleTimeout:options.idleTimeout]; self.uiEventTracker = [[SentryUIEventTracker alloc] initWithMode:mode]; diff --git a/Sources/Sentry/include/SentryTracerConfiguration.h b/Sources/Sentry/include/SentryTracerConfiguration.h index cb38c47745f..5b81a0ffbb4 100644 --- a/Sources/Sentry/include/SentryTracerConfiguration.h +++ b/Sources/Sentry/include/SentryTracerConfiguration.h @@ -26,11 +26,6 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic) BOOL waitForChildren; -/** - * A dispatch queue wrapper to intermediate between the tracer and dispatch calls. - */ -@property (nonatomic, strong, nullable) SentryDispatchQueueWrapper *dispatchQueueWrapper; - #if SENTRY_TARGET_PROFILING_SUPPORTED /** * Whether to sample a profile corresponding to this transaction diff --git a/Sources/Sentry/include/SentryUIEventTrackerTransactionMode.h b/Sources/Sentry/include/SentryUIEventTrackerTransactionMode.h index 4df6f1ea968..43b24d65d30 100644 --- a/Sources/Sentry/include/SentryUIEventTrackerTransactionMode.h +++ b/Sources/Sentry/include/SentryUIEventTrackerTransactionMode.h @@ -2,15 +2,12 @@ #if SENTRY_HAS_UIKIT -@class SentryDispatchQueueWrapper; - NS_ASSUME_NONNULL_BEGIN @interface SentryUIEventTrackerTransactionMode : NSObject SENTRY_NO_INIT -- (instancetype)initWithDispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper - idleTimeout:(NSTimeInterval)idleTimeout; +- (instancetype)initWithIdleTimeout:(NSTimeInterval)idleTimeout; @end diff --git a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift index 8aa8b5e7d3c..8ed4b6c5a20 100644 --- a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift +++ b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift @@ -51,6 +51,7 @@ class SentryProfilerSwiftTests: XCTestCase { } SentryDependencyContainer.sharedInstance().dispatchFactory = dispatchFactory SentryDependencyContainer.sharedInstance().timerFactory = timeoutTimerFactory + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueueWrapper systemWrapper.overrides.cpuUsage = NSNumber(value: mockCPUUsage) systemWrapper.overrides.memoryFootprintBytes = mockMemoryFootprint @@ -76,7 +77,6 @@ class SentryProfilerSwiftTests: XCTestCase { if let idleTimeout = idleTimeout { $0.idleTimeout = idleTimeout } - $0.dispatchQueueWrapper = self.dispatchQueueWrapper $0.waitForChildren = true $0.timerFactory = self.timeoutTimerFactory })) diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift index adc04dbc21d..70c77257c45 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift @@ -31,7 +31,6 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { return SentryTracer(transactionContext: TransactionContext(operation: "ui.load"), hub: hub, configuration: SentryTracerConfiguration(block: { $0.waitForChildren = true $0.timerFactory = self.timerFactory - $0.dispatchQueueWrapper = TestSentryDispatchQueueWrapper() })) } } @@ -41,6 +40,7 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { override func setUp() { super.setUp() SentryDependencyContainer.sharedInstance().dateProvider = fixture.dateProvider + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = TestSentryDispatchQueueWrapper() } override func tearDown() { diff --git a/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift b/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift index 57cfcde65d0..24cb5b2f156 100644 --- a/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift +++ b/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift @@ -16,7 +16,8 @@ class SentryUIEventTrackerTests: XCTestCase { init () { dispatchQueue.blockBeforeMainBlock = { false } SentryDependencyContainer.sharedInstance().swizzleWrapper = swizzleWrapper - uiEventTrackerMode = SentryUIEventTrackerTransactionMode(dispatchQueueWrapper: dispatchQueue, idleTimeout: 3.0) + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueue + uiEventTrackerMode = SentryUIEventTrackerTransactionMode(idleTimeout: 3.0) } func getSut() -> SentryUIEventTracker { diff --git a/Tests/SentryTests/Transaction/SentryTracerTests.swift b/Tests/SentryTests/Transaction/SentryTracerTests.swift index 122d04d3bf4..9d78a6fa0ef 100644 --- a/Tests/SentryTests/Transaction/SentryTracerTests.swift +++ b/Tests/SentryTests/Transaction/SentryTracerTests.swift @@ -52,6 +52,7 @@ class SentryTracerTests: XCTestCase { dispatchQueue.blockBeforeMainBlock = { false } SentryDependencyContainer.sharedInstance().dateProvider = currentDateProvider + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueue appStart = currentDateProvider.date() appStartEnd = appStart.addingTimeInterval(appStartDuration) @@ -87,30 +88,16 @@ class SentryTracerTests: XCTestCase { } #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) - func getSut(waitForChildren: Bool = true) -> SentryTracer { + func getSut(waitForChildren: Bool = true, idleTimeout: TimeInterval = 0.0) -> SentryTracer { let tracer = hub.startTransaction( with: transactionContext, bindToScope: false, customSamplingContext: [:], configuration: SentryTracerConfiguration(block: { $0.waitForChildren = waitForChildren - $0.dispatchQueueWrapper = self.dispatchQueue $0.timerFactory = self.timerFactory - })) - return tracer - } - - func getSut(idleTimeout: TimeInterval = 0.0, dispatchQueueWrapper: SentryDispatchQueueWrapper) -> SentryTracer { - let tracer = hub.startTransaction( - with: transactionContext, - bindToScope: false, - customSamplingContext: [:], - configuration: SentryTracerConfiguration(block: { $0.idleTimeout = idleTimeout - $0.dispatchQueueWrapper = dispatchQueueWrapper - $0.waitForChildren = true - }) - ) + })) return tracer } } @@ -283,7 +270,7 @@ class SentryTracerTests: XCTestCase { } func testDeadlineTimer_OnlyForAutoTransactions() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child1 = sut.startChild(operation: fixture.transactionOperation) let child2 = sut.startChild(operation: fixture.transactionOperation) let child3 = sut.startChild(operation: fixture.transactionOperation) @@ -306,12 +293,14 @@ class SentryTracerTests: XCTestCase { } func testDeadlineTimer_MultipleSpansFinishedInParallel() { - let sut = fixture.getSut(idleTimeout: 0.01, dispatchQueueWrapper: SentryDispatchQueueWrapper()) + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = SentryDispatchQueueWrapper() + let sut = fixture.getSut(idleTimeout: 0.01) testConcurrentModifications(writeWork: { _ in let child = sut.startChild(operation: self.fixture.transactionOperation) child.finish() }) + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = fixture.dispatchQueue } func testFinish_CheckDefaultStatus() { @@ -323,7 +312,7 @@ class SentryTracerTests: XCTestCase { func testIdleTransactionWithStatus_KeepsStatusWhenAutoFinishing() { let status = SentrySpanStatus.aborted - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) sut.status = status let child = sut.startChild(operation: fixture.transactionOperation) @@ -339,13 +328,13 @@ class SentryTracerTests: XCTestCase { func testIdleTransaction_CreatingDispatchBlockFails_NoTransactionCaptured() { fixture.dispatchQueue.createDispatchBlockReturnsNULL = true - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) assertTransactionNotCaptured(sut) } func testIdleTransaction_CreatingDispatchBlockFailsForFirstChild_FinishesTransaction() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) fixture.dispatchQueue.createDispatchBlockReturnsNULL = true @@ -426,7 +415,7 @@ class SentryTracerTests: XCTestCase { #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) func testFinish_IdleTimeout_ExceedsMaxDuration_NoTransactionCaptured() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) advanceTime(bySeconds: 500) @@ -436,7 +425,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_NoChildren_TransactionNotCaptured() { - _ = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + _ = fixture.getSut(idleTimeout: fixture.idleTimeout) fixture.dispatchQueue.invokeLastDispatchAfter() @@ -444,7 +433,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_NoChildren_SpanOnScopeUnset() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) fixture.hub.scope.span = sut @@ -454,7 +443,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_InvokesDispatchAfterWithCorrectWhen() { - _ = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + _ = fixture.getSut(idleTimeout: fixture.idleTimeout) fixture.dispatchQueue.invokeLastDispatchAfter() @@ -462,7 +451,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_SpanAdded_IdleTimeoutCancelled() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) sut.startChild(operation: fixture.transactionOperation) @@ -471,7 +460,8 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeoutWithRealDispatchQueue_SpanAdded_IdleTimeoutCancelled() { - let sut = fixture.getSut(idleTimeout: 0.1, dispatchQueueWrapper: SentryDispatchQueueWrapper()) + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = SentryDispatchQueueWrapper() + let sut = fixture.getSut(idleTimeout: 0.1) let child = sut.startChild(operation: fixture.transactionOperation) let grandChild = child.startChild(operation: fixture.transactionOperation) @@ -481,10 +471,11 @@ class SentryTracerTests: XCTestCase { delayNonBlocking(timeout: 0.5) assertOneTransactionCaptured(sut) + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = fixture.dispatchQueue } func testIdleTimeout_TwoChildren_FirstFinishes_WaitsForTheOther() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child1 = sut.startChild(operation: fixture.transactionOperation) let child2 = sut.startChild(operation: fixture.transactionOperation) @@ -502,7 +493,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_ChildSpanFinished_IdleStarted() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) let child = sut.startChild(operation: fixture.transactionOperation) @@ -525,7 +516,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_TimesOut_TrimsEndTimestamp() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child1 = sut.startChild(operation: fixture.transactionOperation) advanceTime(bySeconds: 1.0) @@ -544,7 +535,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_CallFinish_TrimsEndTimestamp() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child = sut.startChild(operation: fixture.transactionOperation) advanceTime(bySeconds: 1.0) @@ -567,7 +558,7 @@ class SentryTracerTests: XCTestCase { // Interact with sut in extra function so ARC deallocates it func getSut() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) _ = sut.startChild(operation: fixture.transactionOperation) } @@ -598,7 +589,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeoutWithUnfinishedChildren_TimesOut_TrimsEndTimestamp() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child1 = sut.startChild(operation: fixture.transactionOperation) advanceTime(bySeconds: 1.0) @@ -616,7 +607,7 @@ class SentryTracerTests: XCTestCase { } func testIdleTimeout_CallFinish_WaitsForChildren_DoesntStartTimeout() { - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child = sut.startChild(operation: fixture.transactionOperation) XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) @@ -914,7 +905,7 @@ class SentryTracerTests: XCTestCase { func testFinishCallback_CalledWhenTracerFinishes() { let callbackExpectation = expectation(description: "FinishCallback called") - let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue) + let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let block: (SentryTracer) -> Void = { tracer in XCTAssertEqual(sut, tracer)