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

App Size Metrics: add support for Android Universal APKs #365

Merged
merged 10 commits into from
May 31, 2022

Conversation

AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented May 19, 2022

This work is part of the App Metrics project [ref: paaHJt-37V-p2] and implements the fastlane actions necessary to send iOS and Android app size metrics [ref: paaHJt-3od-p2#comment-6098] to the App Metrics server.

What

This builds on top of #364 and adds support for including "Universal APKs" metrics for Android.

(I suggest to get #364 merged first before this #365, to keep the diffs of each PR separate and easier to review in isolation)

Why

This will mostly be useful if/when we want to send app size metrics for Installable Builds, which are build using ./gradlew assemble<app>JalapenoDebug and thus produce a single, Universal .apk — as opposed to an .aab bundle like we do for beta and release builds that we upload to the Play Store)

How

  • New universal_apk_path parameter for the android_send_app_size_metrics action
  • The aab_path parameter is no longer mandatory, to allow the case where we only pass the universal APK but no .aab. It's still required to have at least one of universal_apk_path or aab_path (if not both) though.
  • Changed a key used in the Android metrics payload to the more explicit "Optimized File Size" for R8-optimized APKs (i.e. file-size as reported by apkanalyzer, as opposed to the .apk file size on disk).
    • Hopefully that will help differentiate in particular the Universal APK's file size on disk vs the Universal APK's optimized file size ("optimized" as in "after R8 optimizations and according to apkanalyzer", as opposed to "optimized for a specific device architecture", that latter being instead indicated by the split: metadata)
  • Moved the keys used in each metrics payload into constants at the top of the action, making them easier to spot and change

To Test

@AliSoftware AliSoftware self-assigned this May 19, 2022
@AliSoftware AliSoftware changed the base branch from trunk to app-size-metrics May 19, 2022 18:53
@AliSoftware AliSoftware force-pushed the app-size-metrics+android-universal-apk branch 2 times, most recently from 3464735 to 40b5c58 Compare May 24, 2022 18:07
@AliSoftware AliSoftware marked this pull request as ready for review May 24, 2022 18:57
@AliSoftware AliSoftware requested review from mokagio and a team May 24, 2022 19:02
Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

image

👍 :shipit:

# Keys used by the metrics payload
AAB_FILE_SIZE_KEY = 'AAB File Size'.freeze # value from `File.size` of the `.aab`
UNIVERSAL_APK_FILE_SIZE_KEY = 'Universal APK File Size'.freeze # value from `File.size` of the Universal `.apk`
UNIVERSAL_APK_SPLIT_NAME = 'Universal'.freeze # pseudo-name of the split representing the Universal `.apk`
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious: Why is at "pseudo-name"?

Copy link
Contributor Author

@AliSoftware AliSoftware May 26, 2022

Choose a reason for hiding this comment

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

Because for all the other splits, the name (i.e. the value used for the split: metadata) is provided by bundletool (it's the basename of the .apk produced by it in the <tmpdir>/splits/*.apk); while for the universal APK it's just a name of our choosing and not some official name provided by the Android tooling, so it could be any name we want it to be.

In fact if we wanted to be pedantic, the Universal APK should probably not be considered a "split" per se — it's not the name of a CPU architecture after all. So it's a bit like if we said for iOS that the Universal .ipa was a "thinned version for the device named 'Universal'": pseudo-name because not really the name of a split/CPU-arch but one we made up to represent/mean "multiple archs in a single FAT file".

@AliSoftware AliSoftware force-pushed the app-size-metrics+android-universal-apk branch from a838422 to 4a15e07 Compare May 31, 2022 17:44
Base automatically changed from app-size-metrics to trunk May 31, 2022 17:51
@AliSoftware AliSoftware merged commit d804711 into trunk May 31, 2022
@AliSoftware AliSoftware deleted the app-size-metrics+android-universal-apk branch May 31, 2022 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants