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
getsentry · Oct 18, 2024 · 354f6e7 · 354f6e7
1 parent 324f8b0
commit 354f6e7
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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/...grations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift b/...grations/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
@@ -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()