Skip to content

Update runner to handle logic for both analytics packages #124606

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 12 commits into from
Apr 12, 2023
Merged

Update runner to handle logic for both analytics packages #124606

auto-submit merged 12 commits into from
Apr 12, 2023

Conversation

@eliasyishak
Copy link
Contributor

@eliasyishak eliasyishak commented Apr 11, 2023 •
edited
Loading

Fixes: #124605

Handles both of the use cases mentioned in the issue

Example of what the output would look like for a developer just starting with Flutter today where both analytics reporting systems are enabled

### FIRST MESSAGE
  ╔════════════════════════════════════════════════════════════════════════════╗
  ║                 Welcome to Flutter! - https://flutter.dev                  ║
  ║                                                                            ║
  ║ The Flutter tool uses Google Analytics to anonymously report feature usage ║
  ║ statistics and basic crash reports. This data is used to help improve      ║
  ║ Flutter tools over time.                                                   ║
  ║                                                                            ║
  ║ Flutter tool analytics are not sent on the very first run. To disable      ║
  ║ reporting, type 'flutter config --no-analytics'. To display the current    ║
  ║ setting, type 'flutter config'. If you opt out of analytics, an opt-out    ║
  ║ event will be sent, and then no further information will be sent by the    ║
  ║ Flutter tool.                                                              ║
  ║                                                                            ║
  ║ By downloading the Flutter SDK, you agree to the Google Terms of Service.  ║
  ║ Note: The Google Privacy Policy describes how data is handled in this      ║
  ║ service.                                                                   ║
  ║                                                                            ║
  ║ Moreover, Flutter includes the Dart SDK, which may send usage metrics and  ║
  ║ crash reports to Google.                                                   ║
  ║                                                                            ║
  ║ Read about data we send with crash reports:                                ║
  ║ https://flutter.dev/docs/reference/crash-reporting                         ║
  ║                                                                            ║
  ║ See Google's privacy policy:                                               ║
  ║ https://policies.google.com/privacy                                        ║
  ╚════════════════════════════════════════════════════════════════════════════╝

### SECOND MESSAGE
The Flutter CLI developer tool uses Google Analytics to report usage and diagnostic data
along with package dependencies, and crash reporting to send basic crash reports.
This data is used to help improve the Dart platform, Flutter framework, and related tools.

Telemetry is not sent on the very first run.
To disable reporting of telemetry, run this terminal command:

[dart|flutter] --disable-telemetry.
If you opt out of telemetry, an opt-out event will be sent,
and then no further information will be sent.
This data is collected in accordance with the
Google Privacy Policy (https://policies.google.com/privacy).

### MESSAGE EXPLAINING WHY THERE WERE TWO MESSAGES
You have received two consent messages because
the flutter tool is migrating to a new analytics system
Opting out of collection will opt you out of the legacy
and new analytics collection systems
You can opt out by running `flutter --disable-telemetry`
or `flutter config --no-analytics

# MESSAGE THAT GETS PRINTED FOR PREVIOUSLY OPTED OUT USERS
Please note that analytics reporting was already disabled, and will continue to be disabled.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 11, 2023
eliasyishak
eliasyishak commented Apr 11, 2023
View reviewed changes
eliasyishak
eliasyishak commented Apr 11, 2023
View reviewed changes
@eliasyishak eliasyishak mentioned this pull request Apr 11, 2023
Closed
zanderso
zanderso reviewed Apr 11, 2023
View reviewed changes
packages/flutter_tools/lib/runner.dart

Future<int> _exit(int code, {required ShutdownHooks shutdownHooks}) async {
// Prints the welcome message if needed.
final FirstRunMessenger messenger =
Copy link
Member

@zanderso zanderso Apr 11, 2023

Choose a reason for hiding this comment

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

small nit: Splitting the logic from here down to 309 may help with readability and testability.

Copy link
Contributor Author

@eliasyishak eliasyishak Apr 11, 2023

Choose a reason for hiding this comment

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

That was my first approach but I realized that after running globals.flutterUsage.printWelcome(); which is below this line, the messenger's state for messenger.shouldDisplayLicenseTerms(); is changed after printing the welcome message.

So I was trying to grab the boolean before printing the message

packages/flutter_tools/lib/runner.dart Outdated
// If the user has opted out of legacy analytics, we will continue
// to opt them out of unified analytics and inform them
if (!legacyAnalyticsMessageShown && !globals.flutterUsage.enabled) {
await globals.analytics.setTelemetry(false);
Copy link
Member

@zanderso zanderso Apr 11, 2023

Choose a reason for hiding this comment

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

I'm not sure what the best way to do this is, but ideally, new analytics should inherit the opt-out as close as possible to the tool starting up.

Copy link
Contributor Author

@eliasyishak eliasyishak Apr 11, 2023

Choose a reason for hiding this comment

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

Makes sense, moving it to the top of runZoned below the logic added for disabling telemetry

@eliasyishak eliasyishak marked this pull request as ready for review April 12, 2023 13:24
@eliasyishak
Copy link
Contributor Author

eliasyishak commented Apr 12, 2023

@zanderso this PR is ready for a final review, tests have been added to ensure that legacy opt out status is being passed to the new unified analytics

@eliasyishak eliasyishak added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023
@auto-submit auto-submit bot merged commit 51b23bc into flutter:master Apr 12, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
51b23bc Update runner to handle logic for both analytics packages (fl…
0c6d893 
…utter/flutter#124606)
@eliasyishak eliasyishak deleted the handle-legacy-and-unified-analytics-messages branch April 13, 2023 15:49
christopherfujino
christopherfujino reviewed Apr 13, 2023
View reviewed changes
packages/flutter_tools/lib/runner.dart
'the flutter tool is migrating to a new analytics system. '
'Disabling analytics collection will disable both the legacy '
'and new analytics collection systems. '
'You can disable analytics reporting by running either `flutter --disable-telemetry` '
Copy link
Contributor

@christopherfujino christopherfujino Apr 13, 2023

Choose a reason for hiding this comment

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

--disable-telemetry is only for a single run, right? I think we should clarify this.

Copy link
Contributor

@christopherfujino christopherfujino Apr 13, 2023

Choose a reason for hiding this comment

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

@eliasyishak

Copy link
Contributor Author

@eliasyishak eliasyishak Apr 13, 2023

Choose a reason for hiding this comment

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

No, flutter --disable-telemetry is the command for completely turning off telemetry for the new package. This message is a follow up to the original PDD consent message

I believe flutter --suppress-analytics is for the current run

exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
@eliasyishak @exaby73
Update runner to handle logic for both analytics packages (flutter#12…
7bd3cff 
…4606)

Update runner to handle logic for both analytics packages
eliasyishak added a commit to eliasyishak/flutter that referenced this pull request Apr 24, 2023
@eliasyishak
Update runner to handle logic for both analytics packages (flutter#12…
9b0703c 
…4606)

Update runner to handle logic for both analytics packages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanderso zanderso zanderso approved these changes

+1 more reviewer

@christopherfujino christopherfujino christopherfujino left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Display both consent messages to new flutter users for legacy analytics + package:unified_analytics

3 participants

@eliasyishak @zanderso @christopherfujino