Skip to content

[Tooling] Draft: Upload app metrics #16580

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

Closed
wants to merge 5 commits into from
Closed

Conversation

AliSoftware
Copy link
Contributor

🚧 This is WIP to test the new App Size Metrics API and fastlane action implemented in wordpress-mobile/release-toolkit#364

Internal Reference: paaHJt-3od-p2

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 17, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 17, 2022

You can test the changes on this Pull Request by downloading the APKs:

@AliSoftware
Copy link
Contributor Author

AliSoftware commented May 17, 2022

Rats: WPAndroid is still using CircleCi and its .circleci/config.yml for the Installable Builds, and there Installable Builds are built directly using gradlew assembleWordpress<Flavor><Type> -PinstallableBuildVersionName=…… — as opposed to using fastlane build_and_upload_release, fastlane build_alpha, fastlane build_beta and fastlane build_internal like I thought it would 😔

In practice CircleCI only calls those fastlane build_* lanes during the "Release Build" workflow

So in its current state my demo here won't work on Installable Builds, only on Alpha, Beta and Final Releases.

Which implies Jalapeno Debug Universal APK (and no AAB)
@AliSoftware AliSoftware force-pushed the tooling/app-size-metrics branch from 7a4f7ac to 2b54143 Compare May 19, 2022 19:18
@AliSoftware
Copy link
Contributor Author

So in its current state my demo here won't work on Installable Builds, only on Alpha, Beta and Final Releases.

This has now been fixed:

  • App Size Metrics: add support for Android Universal APKs release-toolkit#365 added support for the android_send_app_size_metrics to use a "Universal APK" as the input file as well (instead of an .aab)
  • This PR is now pointing to that new branch of the releae-toolkit and uses that new option to pass to the action the "Universal APK" produced by ./gradlew assembleWordPressJalapenoDebug during "Installable Build" jobs on PRs.

With that new change, you can see that the payload for both WordPress and Jetpack app metrics have been properly generated and saved as artifacts for the CI (until we get a proper runnin Metrics server to send them to).

@AliSoftware
Copy link
Contributor Author

Note: this PR Draft was initially created to test that the new android_send_app_size_metrics from wordpress-mobile/release-toolkit#364 + wordpress-mobile/release-toolkit#365 worked as expected in client repos with actual APKs and AABs.

But I'm not yet sure we'd really want to send app size metrics for every Installable Build like what this Demo PR does here — and thus size data for "Universal APKs" — in practice once the Metrics server goes live (as opposed to limit it to only send data for betas and final builds, and thus size data for .aab files, but not "Universal APKs"). Cc @jkmassel thoughts?

  • On one end that would make a lot of data, probably too much given how many PRs we have and that each PR can generate an Installable Build multiple times (on each new commit being pushed)
  • On the other end, better have more data and filter it out, rather than less data and regretting not having it later…

@mokagio
Copy link
Contributor

mokagio commented May 26, 2022

My two cents on whether to send app size data for every Installable Build is that we should not do it just yet. Alongside @AliSoftware legitimate questions about having too much data, there's also the fact that we are just starting out with this project. It seems wise to start small and add the extra information later on.

And while it's true that it's better to have more data and filter it out than to not have it and regret it, I think that applies more for analytics that can allow to understand user behavior (where more details give better insight) or in an even sourcing service (where you might want to replay the whole collection of events through a more advanced ML model in the future). In our case, we're talking about how the app size varies between commits on a feature branch. I think we can live with less granularity in this instance.

@AliSoftware AliSoftware modified the milestones: 20.0, 20.1 May 30, 2022
@spencertransier spencertransier modified the milestones: 20.1, 20.2 Jun 13, 2022
@AliSoftware AliSoftware modified the milestones: 20.2, 20.3 Jun 27, 2022
@AliSoftware AliSoftware modified the milestones: 20.3, 20.4 Jul 11, 2022
@ParaskP7 ParaskP7 modified the milestones: 20.4, 20.5 Jul 25, 2022
@AliSoftware AliSoftware removed this from the 20.5 milestone Aug 8, 2022
@AliSoftware AliSoftware added this to the 20.6 milestone Aug 8, 2022
@AliSoftware AliSoftware modified the milestones: 20.6, Future Aug 22, 2022
@justtwago justtwago closed this Dec 1, 2023
@jkmassel jkmassel deleted the tooling/app-size-metrics branch October 17, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants