Skip to content

Collect impeller analytics for appbundles#184146

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
gmackall:collect_impeller_analytics_for_appbundles
Mar 26, 2026
Merged

Collect impeller analytics for appbundles#184146
auto-submit[bot] merged 3 commits into
flutter:masterfrom
gmackall:collect_impeller_analytics_for_appbundles

Conversation

@gmackall

Copy link
Copy Markdown
Member

While looking in to #184127 I noticed we don't collect analytics for impeller when building appbundles. I assume this is a simple oversight, hence this PR.

@gmackall gmackall marked this pull request as ready for review March 25, 2026 18:24
@github-actions github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 25, 2026

@zanderso zanderso left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request extends Impeller analytics reporting to build_appbundle.dart and refines existing reporting in build_aar.dart and build_apk.dart by removing explanatory comments. It also introduces new tests for build_appbundle.dart to validate Impeller settings from AndroidManifest.xml and corrects minor grammatical errors in test descriptions. Feedback highlights opportunities to reduce code duplication by extracting the Impeller analytics reporting logic into a shared helper method and by consolidating common test utility functions and boilerplate code within the new build_appbundle_test.dart.

Comment on lines +176 to +178
final bool impellerEnabled = project.android.computeImpellerEnabled();
final buildLabel = impellerEnabled ? 'manifest-impeller-enabled' : 'manifest-impeller-disabled';
globals.analytics.send(Event.flutterBuildInfo(label: buildLabel, buildType: 'android'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This logic for sending Impeller analytics is now duplicated across build_aar.dart, build_apk.dart, and this file. To improve maintainability and reduce code duplication, consider extracting this logic into a shared helper method. A good place for this could be on the AndroidProject class, which would also encapsulate the computeImpellerEnabled logic.

For example, you could add a method to AndroidProject like this:

// In packages/flutter_tools/lib/src/project.dart within AndroidProject class
void reportImpellerAnalytics({String? buildKey}) {
  final bool impellerEnabled = computeImpellerEnabled();
  final String keyPart = buildKey == null ? '' : '-$buildKey';
  final String status = impellerEnabled ? 'enabled' : 'disabled';
  final String buildLabel = 'manifest$keyPart-impeller-$status';
  globals.analytics.send(Event.flutterBuildInfo(label: buildLabel, buildType: 'android'));
}

Then, this file and others could call project.android.reportImpellerAnalytics(). While modifying project.dart might be out of scope for this PR, it's a good follow-up to consider.

},
);

group('Impeller AndroidManifest.xml setting', () {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

There are a couple of opportunities to reduce code duplication in this new test group:

  1. The writeManifestMetadata helper function is also present in build_aar_test.dart and build_apk_test.dart. Consider extracting it to a shared test utility file (e.g., test/src/android_common.dart) to promote code reuse.

  2. The try/finally block for changing the current working directory is repeated in all three new tests. This could be extracted into a local helper function to reduce boilerplate. For example:

    Future<void> runBuildInProject(String projectPath) async {
      final Directory oldCwd = globals.localFileSystem.currentDirectory;
      try {
        globals.localFileSystem.currentDirectory = globals.localFileSystem.directory(
          projectPath,
        );
        await runBuildAppBundleCommand(projectPath);
      } finally {
        globals.localFileSystem.currentDirectory = oldCwd;
      }
    }

    Each test could then simply call await runBuildInProject(projectPath);.

@gmackall gmackall added the CICD Run CI/CD label Mar 25, 2026
buildNumber: buildNumber,
);

// When an aar is successfully built, record to analytics whether Impeller

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I also deleted these comments in the two places they existed. The first half is obvious, and the second half was no longer true (and more generally is a type of comment I dislike, as it has a tendency of becoming un-true)

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Mar 26, 2026
Merged via the queue into flutter:master with commit b9ac10b Mar 26, 2026
151 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
mboetger pushed a commit to mboetger/flutter that referenced this pull request Mar 26, 2026
While looking in to flutter#184127 I
noticed we don't collect analytics for impeller when building
appbundles. I assume this is a simple oversight, hence this PR.

---------

Co-authored-by: Gray Mackall <[email protected]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Mar 26, 2026
…11372)

Manual roll Flutter from 82d96ef98a33 to e79bf6cbc140 (32 revisions)

Manual roll requested by [email protected]

flutter/flutter@82d96ef...e79bf6c

2026-03-26 [email protected] Roll Dart SDK from a3af8949863e to 7587a31814c6 (1 revision) (flutter/flutter#184213)
2026-03-26 [email protected] Adds rockchip series to block list for vulkan. (flutter/flutter#184207)
2026-03-26 [email protected] [ios]add ci/cd label to ios triage (flutter/flutter#184214)
2026-03-26 [email protected] Roll Skia from 10c97361d8f3 to bee5a06ef578 (1 revision) (flutter/flutter#184203)
2026-03-26 [email protected] Roll Fuchsia Linux SDK from rS5ezRgehkw26fKRX... to BIlBJNOlKjQeRFoFy... (flutter/flutter#184197)
2026-03-26 [email protected] Roll Packages from 5909bdd to 0dd2410 (3 revisions) (flutter/flutter#184198)
2026-03-26 [email protected] fix: use atomic write for engine.stamp to prevent race conditions (flutter/flutter#184131)
2026-03-26 [email protected] Roll Skia from 77dfb68002cd to 10c97361d8f3 (1 revision) (flutter/flutter#184195)
2026-03-26 [email protected] Roll Dart SDK from 349dbbbdba99 to a3af8949863e (1 revision) (flutter/flutter#184194)
2026-03-26 [email protected] Roll Skia from 81ef2238cb09 to 77dfb68002cd (12 revisions) (flutter/flutter#184186)
2026-03-26 [email protected] Revert "Keep glyphs for variable fonts (#183857)" (flutter/flutter#184147)
2026-03-26 [email protected] Expand simple shape path optimization logic and move it from dl_dispatcher to dl_builder (flutter/flutter#184096)
2026-03-26 [email protected] Fix merge changelog workflow. (flutter/flutter#184145)
2026-03-26 [email protected] Add notes on HCPP to `Android-Platform-Views.md` (flutter/flutter#183859)
2026-03-26 [email protected] Adds explicit name to the cicd label job. (flutter/flutter#184070)
2026-03-26 [email protected] Roll Dart SDK from 26f80b9403f5 to 349dbbbdba99 (3 revisions) (flutter/flutter#184176)
2026-03-26 [email protected] Collect impeller analytics for appbundles (flutter/flutter#184146)
2026-03-26 [email protected] [Dot shorthands] Migrate examples/api/lib/material (flutter/flutter#183963)
2026-03-26 [email protected] Roll Dart DevTools to a version with correct CIPD tags (flutter/flutter#184172)
2026-03-25 [email protected] Pipes ScrollCacheExtent through more scroll views (flutter/flutter#184078)
2026-03-25 [email protected] Add widget of the week link in SensitiveContent documentation (flutter/flutter#183972)
2026-03-25 [email protected] Roll Packages from 8dcfd11 to 5909bdd (13 revisions) (flutter/flutter#184123)
2026-03-25 [email protected] Roll Dart SDK from c48f0c954d86 to 26f80b9403f5 (1 revision) (flutter/flutter#184117)
2026-03-25 [email protected] Roll Skia from 51725ae49ef5 to 81ef2238cb09 (3 revisions) (flutter/flutter#184115)
2026-03-25 [email protected] Roll Fuchsia Linux SDK from M3sjWggTQmP2AD4bS... to rS5ezRgehkw26fKRX... (flutter/flutter#184114)
2026-03-25 [email protected] Add SensitiveContent widget sample code (flutter/flutter#183846)
2026-03-25 [email protected] SelectableRegion should passthrough constraints to child unmodified (flutter/flutter#184083)
2026-03-25 [email protected] Roll Dart SDK from ce171d5026ff to c48f0c954d86 (2 revisions) (flutter/flutter#184105)
2026-03-25 [email protected] Roll Skia from 789ad6b12776 to 51725ae49ef5 (3 revisions) (flutter/flutter#184106)
2026-03-25 [email protected] Roll Skia from f4e59f82dc69 to 789ad6b12776 (7 revisions) (flutter/flutter#184098)
2026-03-25 [email protected] Roll Dart SDK from b08bd0a3ee39 to ce171d5026ff (4 revisions) (flutter/flutter#184095)
2026-03-24 [email protected] [flutter_goldens] Remove dead check on null being in a list of non-nullables (flutter/flutter#183938)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

...
ahmedsameha1 pushed a commit to ahmedsameha1/flutter that referenced this pull request Apr 14, 2026
While looking in to flutter#184127 I
noticed we don't collect analytics for impeller when building
appbundles. I assume this is a simple oversight, hence this PR.

---------

Co-authored-by: Gray Mackall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants