[No QA] ci: Add Sentry build size analysis#82952
Conversation
|
@jayeshmangwani Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@jayeshmangwani no C+ review needed here |
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
roryabraham
left a comment
There was a problem hiding this comment.
Ok, looks like this worked for iOS but not Android:
https://expensify.sentry.io/explore/releases/?project=4510228107427840&statsPeriod=24h&tab=mobile-builds
Also, can we add a GitHub workflow annotation (with a ::notice:: log) to link to the uploaded Sentry builds?
Also, NAB --log-level debug is more verbose than I imagined
|
|
|
Great that we have it for ios. Here is also some useful bundle chart, also the potential savings recommendations looks good -> https://expensify.sentry.io/preprod/size/52159/?project=4510228107427840 For the Android issue, the main problem is that it was uploaded with the error: From the CI logs, we can see that the APK was built and uploaded successfully, and the file size is valid (~174MB). Most likely, the failure occurs when Sentry tried to parse the binary AndroidManifest.xml. Sentry recommends using sentry-cli >= 2.58.2 for the build upload command. The workflow was using version 2.58.0 (resolved transitively via @sentry/webpack-plugin), which might be the cause of the issue. I added @sentry/cli as a direct devDependency (pinned to 3.2.0) in package.json |
|
I also added notice logs for better visibility. I know the logs are quite verbose, but once we confirm the flow works as expected, we can remove the |
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@rinej can we get an actual table with proper hyperlinks like we see in the existing build summary?
|
|
Also, it looks like unfortunately the Android build size analysis failed again |
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
Oh yes of course, I was using a skill and forgot to mention that in the prompt 😄 |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
So I think we go ahead and separately see if Sentry can help us with the Android AdHoc builds |
|
Thanks for verifying @Julesssss ! Sentry support suggested adding this verification step to confirm that The file is embedded by the Sentry Android Gradle plugin and is required for Sentry to correctly process the build. If it’s missing, Sentry won’t be able to associate the uploaded artifact with the project. I added that step in recent commit, could you run the action again? Thanks! |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
I checked the logs, and the @Julesssss , given the urgency of this ticket (I’m not entirely sure how urgent it is), we have the following options:
|
Thanks 👍 This isn't super urgent, but lets merge the release build analysis, which is working. We can keep the issue open in the meantime to work on adhoc builds. |
|
ok, that makes sense👍 |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
Julesssss
left a comment
There was a problem hiding this comment.
Looking good. running test build again in meantime
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Julesssss
left a comment
There was a problem hiding this comment.
Moving forward here with the release build analysis
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.3.41-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|

Explanation of Change
Uploads Android and iOS AdHoc builds to Sentry for size analysis after the Rock Remote Build step
Uses the ARTIFACT_PATH env var set by the Rock actions:
Android:
https://github.com/callstackincubator/android/blob/4cedf4d9b5c167452c96fe67233577e0fde9a025/action.yml#L274
iOS:
https://github.com/callstackincubator/ios/blob/dd30f7e53eee2ea6a59509793d0a30fbb5c91216/action.yml#L372
Fixed Issues
$ #82850
PROPOSAL:
Tests
testBuild.ymlfrom this branchOffline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari