[No QA] Add Sentry build size analysis for AdHoc builds#82952
[No QA] Add Sentry build size analysis for AdHoc builds#82952rinej wants to merge 12 commits intoExpensify:mainfrom
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. |
Ok, I’ve added the table with the bundle URLs - we should now see them under the AdHoc builds urls |
|
For Android issue:
Looks like we built and uploaded correctly but Sentry cannot parse the APK and the AndroidManifest.xml. The docs say both APK and AAB are supported, but AAB is the preferred format. What we can do:
|
|
Also just to make sure it is the app parsing issue caused by Sentry, we could test the Staging approach, from this branch: On staging we are using the ABB format for Android, which is recommended by Sentry so it should be able to parse the Manifest correctly |
Let's do both! |
|
Ok, I updated the PR to build AAB using Rock - with that we can test if that was the main issue with Sentry. I also adjusted the refactor with splitting the Sentry upload logic into separate buildAndroid and buildIOS files. there might be an issue with downloading adhoc builds in AAB format for adhoc, but let's first verify if this fixes the main issue with Sentry. If yes, we can add conversion |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
|
🚧 @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! 🧪🧪
|
|
The upload was successful, so the issue is with Sentry parsing APK for android. When uploaded AAB it worked fine. For release we upload AAB by default so no additional changed there are needed, with that tested I think we can proceed with the release upload from that PR -> #82937 For add-hocs we need to find another solution, cause we need APK so it is easier to download and install on testing devices. |
|
For the adhocs, the only solution I have for now is to keep the Rock build with the APK as it is (we need the APK for the S3 upload). And add another step to build the AAB. It adds some extra build time, but when Rock uses the cache build, it shouldn’t take too long. We can remove this extra step once Sentry fixes the APK parsing |
|
I think short term that's ok. I posted further thoughts in the #expert-contributors slack room here |
roryabraham
left a comment
There was a problem hiding this comment.
Can you please combine #82937 into this PR and close that other one? I've lost sight of why they're separate and I think I'd have a clearer picture of the changes if they were combined.

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