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 pre warmed app start measurement #1897

Closed
4 tasks done
philipphofmann opened this issue Jun 17, 2022 · 9 comments · Fixed by #1969
Closed
4 tasks done

Fix pre warmed app start measurement #1897

philipphofmann opened this issue Jun 17, 2022 · 9 comments · Fixed by #1969

Comments

@philipphofmann
Copy link
Member

philipphofmann commented Jun 17, 2022

Description

Since iOS 15, the system might decide to pre-warm your app before the user tries to open it. In such cases, we can't reliably measure the app start, and we started to drop reporting app start measurements with #1896. Otherwise, we would get app start times ranging from one minute to even days.

According to a former Apple engineer, you should use MetricKit to collect statistics on app start because MetricKit uses internal OS values, which are not exposed otherwise. MetricKit works on iOS 13 and above. Maybe it's time to replace our own SentryAppStartTracker.

See also #1661. [Internal Notion page] (https://www.notion.so/sentry/Detecting-iOS-15-App-Prewarming-c892f118bdd34b558eb187796ed5e424) on how to detect pre-warmed app starts with data we can't share in public, and another Notion page for a DACI on how to report pre-warmed app starts again.

Related PRs:

@bruno-garcia
Copy link
Member

Can we get values from MetricKit about the startup related to the transaction that launched the app?

@philipphofmann
Copy link
Member Author

philipphofmann commented Jun 17, 2022

Can we get values from MetricKit about the startup related to the transaction that launched the app?

No, we can't because we don't get launch data in real-time. You get it once per day: see #1661 (comment)

@philipphofmann
Copy link
Member Author

philipphofmann commented Jun 17, 2022

According to the Apple docs

Prewarming an app results in an indeterminate amount of time between when the prewarming phase completes and when the user, or system, fully launches the app. Because of this, use MetricKit to accurately measure user-driven launch and resume times instead of manually signposting various points of the launch sequence.

And

Prewarming executes an app’s launch sequence up until, but not including, when main() calls UIApplicationMain(::::).

So we could check if the runtime init and the process start time are some threshold in the past to assume it is an pre warmed app start. We could then just ditch Pre Main and UIKit and Application Init and create a new app start type pre warmed. MetricKit has a special histogram for pre warmed app starts. Taken from our docs:

@philipphofmann
Copy link
Member Author

philipphofmann commented Jun 20, 2022

Prewarming can execute code up to viewDidLoad of a UIViewController, and keep your app in the background. It can pause at any stage from creating the process up to viewDidLoad of a UIViewController or something similar. This is a contradiction to the Apple docs Prewarming executes an app’s launch sequence up until, but not including, when main() calls UIApplicationMain(::::).

I dug into some transactions of customers and found a few transactions similar to this one:

Screen Shot 2022-06-20 at 09 39 34

The above screenshot shows missing instrumentation of around 4 hours in between viewDidLoad and viewWillAppear. This can't be the case if the app was in the foreground because the watchdog would kill the app.

Discover query to reveal such transactions: event.type:transaction measurements.app_start_warm:>500 measurements.app_start_warm:<2000 transaction.duration:>10000

An answer on StackOverflow supports this claim.

More screenshots

Discover query used:event.type:transaction measurements.app_start_warm:>10000

Pausing after process creation

Screen Shot 2022-06-20 at 09 48 49

Screen Shot 2022-06-20 at 09 53 40

Pausing after process creation and runtime init
Screen Shot 2022-06-20 at 09 54 16

@philipphofmann
Copy link
Member Author

Maybe using __mod_init_func in the data section of Mach-O binary could help. An example of how to use it.

@philipphofmann
Copy link
Member Author

It seems like we can use, see https://eisel.me/startup

if ([[NSProcessInfo processInfo].environment[@"ActivePrewarm"] isEqual:@"1"]) {
      // The ActivePrewarm variable indicates whether the app was launched via pre-warming.
      // Based on this, for example, you can choose whether or not to include the pre-main time in your calculation based on that.
}

@philipphofmann
Copy link
Member Author

@mitsuhiko, @k-fish, and I agreed on reporting the app start type in the app context as start_type : cold | cold.prewarmed | warm | warm.prewarmed. Internal Notion Page for more context.

@philipphofmann philipphofmann moved this from In Progress to Needs Review in Mobile & Cross Platform SDK Jul 20, 2022
@bruno-garcia
Copy link
Member

To sum up: latest sdk version doesn't send bogus app start anymore. The startup time metric is accurate. It doesn't send pre warn info yet. It's the last pending feature we want to add to this.

@github-actions
Copy link

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@philipphofmann philipphofmann moved this from Blocked to In Progress in Mobile & Cross Platform SDK Oct 27, 2022
philipphofmann added a commit that referenced this issue Nov 14, 2022
Report pre-warmed app starts by dropping the first app start spans if pre-warming paused during these steps.
This approach will shorten the app start duration, but it represents the duration a user has to wait after clicking
the app icon until the app is responsive. We report the app start type in the appContext, so Sentry can make
changes to the UI for prewarmed app starts.

Fixes GH-1897
Repository owner moved this from In Progress to Done in Mobile & Cross Platform SDK Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants