Skip to content

feat(ANR): Frame metrics including time while in background #5681

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jul 23, 2025
Merged

feat(ANR): Frame metrics including time while in background #5681

merged 9 commits into from
Jul 23, 2025

Conversation

itaybre
Copy link
Contributor

@itaybre itaybre commented Jul 22, 2025

The SDK was reporting hangs when coming back to foreground.

Fixes: #5554

@linear Linear
Copy link

linear bot commented Jul 22, 2025

COCOA-447 Delayed AppHang Reports When App Moves to the Background

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 22, 2025 •
edited
Loading

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Frame metrics including time while in background ([#5681](https://github.com/getsentry/sentry-cocoa/pull/5681))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against b25af99

@itaybre itaybre force-pushed the itay/cocoa-447-delayed-apphang-reports-when-app-moves-to-the-background branch from 4684169 to 9b70ceb Compare July 22, 2025 19:20
@codecov Codecov
Copy link

codecov bot commented Jul 22, 2025 •
edited
Loading

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
3794 1 3793 9
View the top 1 failed test(s) by shortest run time 
SentryTests.SentrySDKInternalTests::testStartOnTheMainThread
Stack Traces | 0s run time 
.../Tests/SentryTests/SentrySDKInternalTests.swift:706 - Asynchronous wait failed: Exceeded timeout of 1 seconds, with unfulfilled expectations: "MainThreadTestIntegration install called".

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@itaybre itaybre marked this pull request as ready for review July 22, 2025 20:56
@github-actions GitHub Actions
Copy link
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1220.47 ms 1243.56 ms 23.09 ms
Size 23.75 KiB 904.50 KiB 880.76 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2481950 1221.04 ms 1248.98 ms 27.94 ms
a9fac2e 1212.45 ms 1219.67 ms 7.22 ms
99104c9 1224.84 ms 1247.08 ms 22.24 ms
7148f97 1235.09 ms 1258.07 ms 22.98 ms
6279992 1213.60 ms 1241.38 ms 27.79 ms
aa96485 1215.37 ms 1234.04 ms 18.67 ms
fc0757d 1231.83 ms 1248.98 ms 17.15 ms
7c7ac56 1225.90 ms 1250.22 ms 24.33 ms
8e3a42f 1222.90 ms 1245.67 ms 22.77 ms
51f74d7 1236.31 ms 1247.43 ms 11.12 ms

App size

Revision Plain With Sentry Diff
2481950 23.74 KiB 872.74 KiB 849.00 KiB
a9fac2e 23.75 KiB 879.53 KiB 855.78 KiB
99104c9 23.75 KiB 894.83 KiB 871.09 KiB
7148f97 23.75 KiB 854.78 KiB 831.03 KiB
6279992 23.75 KiB 891.03 KiB 867.28 KiB
aa96485 23.75 KiB 874.46 KiB 850.71 KiB
fc0757d 23.75 KiB 850.73 KiB 826.98 KiB
7c7ac56 23.75 KiB 902.49 KiB 878.74 KiB
8e3a42f 23.75 KiB 880.06 KiB 856.31 KiB
51f74d7 23.74 KiB 874.08 KiB 850.34 KiB

philipphofmann
philipphofmann approved these changes Jul 23, 2025
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @itaybre. I can reproduce wrong frozen frames with the iOS-Swift app from a testflight build by simply putting the app into background for a second, opening it again and then checking the frozen frame number. It would be great if you also did such a test to validate that this problem is now gone. If it is, we can merge this. Or how did you validate that the problem doesn't persist any more with manual tests?

In case you didn't know yet, you can trigger test flights build manually here https://github.com/getsentry/sentry-cocoa/actions/workflows/testflight.yml

Screenshot 2025-07-23 at 16 31 52

And then you need to find the build in testflight with this number
Screenshot 2025-07-23 at 16 33 16

@itaybre
Copy link
Contributor Author

itaybre commented Jul 23, 2025

Good catch @itaybre. I can reproduce wrong frozen frames with the iOS-Swift app from a testflight build by simply putting the app into background for a second, opening it again and then checking the frozen frame number. It would be great if you also did such a test to validate that this problem is now gone. If it is, we can merge this. Or how did you validate that the problem doesn't persist any more with manual tests?

In case you didn't know yet, you can trigger test flights build manually here https://github.com/getsentry/sentry-cocoa/actions/workflows/testflight.yml

Screenshot 2025-07-23 at 16 31 52 And then you need to find the build in testflight with this number Screenshot 2025-07-23 at 16 33 16

Yes, I validated it in testflight

@itaybre
Copy link
Contributor Author

itaybre commented Jul 23, 2025

Prior to the change:

ScreenRecording_07-23-2025.14-40-00_1.MP4

After the change:

ScreenRecording_07-23-2025.14-42-11_1.MP4

@itaybre itaybre changed the title feat(ANR): Ignore frame delays when going into background feat(ANR): Frame metrics including time while in background Jul 23, 2025
@itaybre itaybre merged commit 7ef7ab8 into main Jul 23, 2025
80 of 88 checks passed
@itaybre itaybre deleted the itay/cocoa-447-delayed-apphang-reports-when-app-moves-to-the-background branch July 23, 2025 17:44
@philipphofmann philipphofmann mentioned this pull request Jul 25, 2025
Open
@philipphofmann philipphofmann mentioned this pull request Aug 13, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipphofmann philipphofmann philipphofmann approved these changes

@armcknight armcknight Awaiting requested review from armcknight

@philprime philprime Awaiting requested review from philprime philprime is a code owner

@noahsmartin noahsmartin Awaiting requested review from noahsmartin noahsmartin is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Delayed AppHang Reports When App Moves to the Background
2 participants
@itaybre @philipphofmann