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

fix: Finish TTID when viewWillAppear is skipped #4417

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- Edge case for swizzleClassNameExclude (#4405): Skip creating transactions for UIViewControllers ignored for swizzling
via the option `swizzleClassNameExclude`.
- Add TTID/TTFD spans when loadView gets skipped (#4415)
- Finish TTID correctly when viewWillAppear is skipped (#4417)
- Swizzling RootUIViewController if ignored by `swizzleClassNameExclude` (#4407)

## 8.38.0-beta.1
Expand Down
21 changes: 17 additions & 4 deletions Sources/Sentry/SentryUIViewControllerPerformanceTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,7 @@ - (void)viewControllerViewWillAppear:(UIViewController *)controller
};

[self.tracker activateSpan:spanId duringBlock:duringBlock];

SentryTimeToDisplayTracker *ttdTracker
= objc_getAssociatedObject(controller, &SENTRY_UI_PERFORMANCE_TRACKER_TTD_TRACKER);
[ttdTracker reportInitialDisplay];
[self reportInitialDisplay:controller];
};

[self limitOverride:@"viewWillAppear"
Expand Down Expand Up @@ -320,6 +317,15 @@ - (void)viewControllerViewWillLayoutSubViews:(UIViewController *)controller
OBJC_ASSOCIATION_RETAIN_NONATOMIC);
};
[self.tracker activateSpan:spanId duringBlock:duringBlock];

// According to the Apple docs
// (https://developer.apple.com/documentation/uikit/uiviewcontroller/1621510-viewwillappear),
// viewWillAppear should be called for before the UIViewController is added to the view
// hierarchy. There are some edge cases, though, when this doesn't happen, and we saw
// customers' transactions also proofing this. Therefore, we must also report the initial
// display here, as the customers' transactions had spans for `viewWillLayoutSubviews`.

[self reportInitialDisplay:controller];
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
};

[self limitOverride:@"viewWillLayoutSubviews"
Expand Down Expand Up @@ -421,6 +427,13 @@ - (void)measurePerformance:(NSString *)description
}
}

- (void)reportInitialDisplay:(UIViewController *)controller
{
SentryTimeToDisplayTracker *ttdTracker
= objc_getAssociatedObject(controller, &SENTRY_UI_PERFORMANCE_TRACKER_TTD_TRACKER);
[ttdTracker reportInitialDisplay];
}

@end

#endif // SENTRY_HAS_UIKIT
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,90 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase {
wait(for: [callbackExpectation], timeout: 0)
}

func testReportInitialDisplay_WhenViewWillAppear() throws {
let sut = fixture.getSut()
let viewController = fixture.viewController
let tracker = fixture.tracker
var tracer: SentryTracer?

sut.viewControllerLoadView(viewController) {
let spans = self.getStack(tracker)
tracer = spans.first as? SentryTracer
}
sut.viewControllerViewWillAppear(viewController) {
self.advanceTime(bySeconds: 0.1)
}

reportFrame()
let expectedTTIDTimestamp = fixture.dateProvider.date()

let children: [Span]? = Dynamic(tracer).children as [Span]?

let ttidSpan = try XCTUnwrap(children?.first)
XCTAssertEqual("ui.load.initial_display", ttidSpan.operation)
XCTAssertEqual("TestViewController initial display", ttidSpan.spanDescription)
XCTAssertEqual(expectedTTIDTimestamp, ttidSpan.timestamp)
XCTAssertTrue(ttidSpan.isFinished)
}

func testReportInitialDisplay_WhenViewWillAppearSkipped_WillLayoutSubViewsCalled() throws {
let sut = fixture.getSut()
let viewController = fixture.viewController
let tracker = fixture.tracker
var tracer: SentryTracer?

sut.viewControllerLoadView(viewController) {
let spans = self.getStack(tracker)
tracer = spans.first as? SentryTracer
}

sut.viewControllerViewWillLayoutSubViews(viewController) {
self.advanceTime(bySeconds: 0.1)
}

reportFrame()
let expectedTTIDTimestamp = fixture.dateProvider.date()

let children: [Span]? = Dynamic(tracer).children as [Span]?

let ttidSpan = try XCTUnwrap(children?.first)
XCTAssertEqual("ui.load.initial_display", ttidSpan.operation)
XCTAssertEqual("TestViewController initial display", ttidSpan.spanDescription)
XCTAssertEqual(expectedTTIDTimestamp, ttidSpan.timestamp)
XCTAssertTrue(ttidSpan.isFinished)
}

func testReportInitialDisplay_WhenViewWillAppearAndWillLayoutSubviews() throws {
let sut = fixture.getSut()
let viewController = fixture.viewController
let tracker = fixture.tracker
var tracer: SentryTracer?

sut.viewControllerLoadView(viewController) {
let spans = self.getStack(tracker)
tracer = spans.first as? SentryTracer
}
sut.viewControllerViewWillAppear(viewController) {
self.advanceTime(bySeconds: 0.1)
}

reportFrame()
let expectedTTIDTimestamp = fixture.dateProvider.date()

// It's doubtful that the OS renders a frame between viewWillAppear and viewWillLayoutSubViews, but if it does, we need to pick the end TTID on viewWillAppear.
sut.viewControllerViewWillLayoutSubViews(viewController) {
self.advanceTime(bySeconds: 0.1)
}

let children: [Span]? = Dynamic(tracer).children as [Span]?

let ttidSpan = try XCTUnwrap(children?.first)
XCTAssertEqual("ui.load.initial_display", ttidSpan.operation)
XCTAssertEqual("TestViewController initial display", ttidSpan.spanDescription)
XCTAssertEqual(expectedTTIDTimestamp, ttidSpan.timestamp)
XCTAssertTrue(ttidSpan.isFinished)
}

func testReportFullyDisplayed() throws {
let sut = fixture.getSut()
sut.enableWaitForFullDisplay = true
Expand Down
Loading