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

reportFullDisplay finishing the wrong span #4533

Open
brustolin opened this issue Nov 13, 2024 · 1 comment
Open

reportFullDisplay finishing the wrong span #4533

brustolin opened this issue Nov 13, 2024 · 1 comment

Comments

@brustolin
Copy link
Contributor

brustolin commented Nov 13, 2024

Description

There is a problem with the reportFullDisplay API:

  • ViewControllerA opens, starts a HTTP request, user clicks a button and opens ViewControllerB.
  • ViewControllerB starts an HTTP request.
  • ViewControllerA’s HTTP request ends and SentrySDK.reportFullyDisplayed is called.

This will finish TTFD for ViewControllerB, but it is still waiting for its request to respond.

Test inside SentryUIViewControllerPerformanceTrackerTests.swift from #4531 to reproduce the problem:

func test_waitForFullDisplay_NewViewControllerLoaded_BeforeReportTTFD_withBackgroundWork() throws {
    class CustomVC: UIViewController {
        var workSpan: Span?

        override func viewDidAppear(_ animated: Bool) {
            super.viewDidAppear(animated)
            //Start some work, it could be a http request
            workSpan = SentrySDK.span?.startChild(operation: "Work")
        }

        // We will use a function to finish the work
        // but it could be the request response
        func finishWork() {
            workSpan?.finish()
            SentrySDK.reportFullyDisplayed()
        }
    }

    let dateProvider = fixture.dateProvider
    let sut = fixture.getSut()
    let tracker = fixture.tracker
    let firstController = CustomVC()
    let secondController = CustomVC()

    var firstTracer: SentryTracer?
    var secondTracer: SentryTracer?

    sut.enableWaitForFullDisplay = true

    dateProvider.advance(by: 1)
    sut.viewControllerLoadView(firstController) {
        firstController.loadView()
        firstTracer = self.getStack(tracker).first as? SentryTracer
    }

    sut.viewControllerViewDidLoad(firstController) {
        firstController.viewDidLoad()
    }

    dateProvider.advance(by: 1)
    sut.viewControllerViewWillAppear(firstController) {
        firstController.viewWillAppear(false)
    }

    sut.viewControllerViewDidAppear(firstController) {
        firstController.viewDidAppear(false)
    }

    fixture.displayLinkWrapper.normalFrame()

    let firstFullDisplaySpan = try XCTUnwrap(firstTracer?.children.first { $0.operation == "ui.load.full_display" })

    XCTAssertFalse(firstFullDisplaySpan.isFinished)

    XCTAssertEqual(firstTracer?.traceId, SentrySDK.span?.traceId)

    dateProvider.advance(by: 1)
    sut.viewControllerLoadView(secondController) {
        secondController.loadView()
        secondTracer = self.getStack(tracker).first as? SentryTracer
    }

    // The work of the first controller asynchronously ended here, and a new frame was rendered
    // while the second controller hasn’t appeared yet.
    dateProvider.advance(by: 1)
    firstController.finishWork()
    fixture.displayLinkWrapper.normalFrame()

    dateProvider.advance(by: 1)
    sut.viewControllerViewWillAppear(secondController) {
        secondController.viewWillAppear(false)
    }
    sut.viewControllerViewDidAppear(secondController) {
        secondController.viewDidAppear(false)
    }
    fixture.displayLinkWrapper.normalFrame()

    dateProvider.advance(by: 1)
    let timeOfSecondControllerFinishWork = dateProvider.date()
    secondController.finishWork()

    fixture.displayLinkWrapper.normalFrame()

    XCTAssertTrue(firstFullDisplaySpan.isFinished)
    XCTAssertEqual(.deadlineExceeded, firstFullDisplaySpan.status)

    let secondFullDisplaySpan = try XCTUnwrap(secondTracer?.children.first { $0.operation == "ui.load.full_display" })

    XCTAssertEqual(secondFullDisplaySpan.timestamp, timeOfSecondControllerFinishWork)
}
@philipphofmann
Copy link
Member

The proper solution for this is to extend the reportFullyDisplay method to allow passing in an argument to tell the SDK for which UIViewController fullyDisplay is reported. We still need to figure out what this API looks like, and we should align across all mobile SDKs.

@philipphofmann philipphofmann moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

2 participants