From 354f6e7f5477c0f2ef71db2fa82dadc91ac33ac6 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Fri, 18 Oct 2024 11:21:41 +0200 Subject: [PATCH] Fix: Dont create transaction for unused ViewControllers (#4448) Its possible to create a view controller but never add it to the view hierarchy. This will create a transaction with data that is not helpful --- CHANGELOG.md | 1 + Sources/Sentry/SentryPerformanceTracker.m | 1 + Sources/Sentry/SentryTracer.m | 9 ++++++++- Sources/Sentry/SentryTracerConfiguration.m | 1 + Sources/Sentry/include/SentryTracerConfiguration.h | 7 +++++++ ...entryUIViewControllerPerformanceTrackerTests.swift | 3 +++ Tests/SentryTests/Transaction/SentryTracerTests.swift | 11 ++++++++++- 7 files changed, 31 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c83e047c95..5c02c2e4b25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ via the option `swizzleClassNameExclude`. - Data race in SentrySwizzleInfo.originalCalled (#4434) - Delete old session replay files (#4446) - Thread running at user-initiated quality-of-service for session replay (#4439) +- Don't create transactions for unused UIViewControllers (#4448) ### Improvements diff --git a/Sources/Sentry/SentryPerformanceTracker.m b/Sources/Sentry/SentryPerformanceTracker.m index ed24bf25387..cf1a3e98c02 100644 --- a/Sources/Sentry/SentryPerformanceTracker.m +++ b/Sources/Sentry/SentryPerformanceTracker.m @@ -86,6 +86,7 @@ - (SentrySpanId *)startSpanWithName:(NSString *)name configuration:[SentryTracerConfiguration configurationWithBlock:^( SentryTracerConfiguration *configuration) { configuration.waitForChildren = YES; + configuration.finishMustBeCalled = YES; }]]; [(SentryTracer *)newSpan setDelegate:self]; diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index 427590ceb42..f5d41be75fc 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -303,7 +303,8 @@ - (void)deadlineTimerFired } } - [self finishWithStatus:kSentrySpanStatusDeadlineExceeded]; + _finishStatus = kSentrySpanStatusDeadlineExceeded; + [self finishInternal]; } - (void)cancelDeadlineTimer @@ -581,6 +582,12 @@ - (void)finishInternal } }]; + if (self.configuration.finishMustBeCalled && !self.wasFinishCalled) { + SENTRY_LOG_DEBUG( + @"Not capturing transaction because finish was not called before timing out."); + return; + } + @synchronized(_children) { if (_configuration.idleTimeout > 0.0 && _children.count == 0) { SENTRY_LOG_DEBUG(@"Was waiting for timeout for UI event trace but it had no children, " diff --git a/Sources/Sentry/SentryTracerConfiguration.m b/Sources/Sentry/SentryTracerConfiguration.m index c14ec504f4b..5fbb920fbd5 100644 --- a/Sources/Sentry/SentryTracerConfiguration.m +++ b/Sources/Sentry/SentryTracerConfiguration.m @@ -21,6 +21,7 @@ - (instancetype)init if (self = [super init]) { self.idleTimeout = 0; self.waitForChildren = NO; + self.finishMustBeCalled = NO; } return self; } diff --git a/Sources/Sentry/include/SentryTracerConfiguration.h b/Sources/Sentry/include/SentryTracerConfiguration.h index 9fdb9c228c7..76bc217337c 100644 --- a/Sources/Sentry/include/SentryTracerConfiguration.h +++ b/Sources/Sentry/include/SentryTracerConfiguration.h @@ -28,6 +28,13 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic) BOOL waitForChildren; +/** + * This flag indicates whether the trace should be captured when the timeout triggers. + * If Yes, this tracer will be discarced in case the timeout triggers. + * Default @c NO + */ +@property (nonatomic) BOOL finishMustBeCalled; + #if SENTRY_TARGET_PROFILING_SUPPORTED /** * Whether to sample a profile corresponding to this transaction diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift index abae8db883e..5408af4e390 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift @@ -136,6 +136,9 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase { XCTAssertEqual(tracer.transactionContext.nameSource, .component) XCTAssertEqual(tracer.transactionContext.origin, origin) XCTAssertFalse(tracer.isFinished) + + let config = try XCTUnwrap(Dynamic(tracer).configuration.asObject as? SentryTracerConfiguration) + XCTAssertTrue(config.finishMustBeCalled) sut.viewControllerViewDidLoad(viewController) { if let blockSpan = self.getStack(tracker).last { diff --git a/Tests/SentryTests/Transaction/SentryTracerTests.swift b/Tests/SentryTests/Transaction/SentryTracerTests.swift index 105c47712f1..c4bba334640 100644 --- a/Tests/SentryTests/Transaction/SentryTracerTests.swift +++ b/Tests/SentryTests/Transaction/SentryTracerTests.swift @@ -95,7 +95,7 @@ class SentryTracerTests: XCTestCase { } #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) - func getSut(waitForChildren: Bool = true, idleTimeout: TimeInterval = 0.0) -> SentryTracer { + func getSut(waitForChildren: Bool = true, idleTimeout: TimeInterval = 0.0, finishMustBeCalled: Bool = false) -> SentryTracer { let tracer = hub.startTransaction( with: transactionContext, bindToScope: false, @@ -104,6 +104,7 @@ class SentryTracerTests: XCTestCase { $0.waitForChildren = waitForChildren $0.timerFactory = self.timerFactory $0.idleTimeout = idleTimeout + $0.finishMustBeCalled = finishMustBeCalled })) return tracer } @@ -1292,6 +1293,14 @@ class SentryTracerTests: XCTestCase { } #endif + func testFinishShouldBeCalled_Timeout_NotCaptured() { + fixture.dispatchQueue.blockBeforeMainBlock = { true } + + let sut = fixture.getSut(finishMustBeCalled: true) + fixture.timerFactory.fire() + assertTransactionNotCaptured(sut) + } + @available(*, deprecated) func testSetExtra_ForwardsToSetData() { let sut = fixture.getSut()