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.

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

Clean up analytics opt in/out flags #132588

Merged
merged 9 commits into from
Sep 12, 2023
Merged

Clean up analytics opt in/out flags #132588

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aaf9285
`--disable-telemetry` --> `--disable-analytics`
eliasyishak Aug 9, 2023
4c22c2c
Help text for `flutter config` for `--analytics`
eliasyishak Aug 9, 2023
71a2c68
Merge remote-tracking branch 'upstream/master' into 132258-clean-up-c…
eliasyishak Aug 15, 2023
814d200
Merge remote-tracking branch 'upstream/master' into 132258-clean-up-c…
eliasyishak Aug 15, 2023
8ef7a25
Fix analyze errors
eliasyishak Aug 15, 2023
6ad58ef
Hide analytics flag + add interpolation for flags
eliasyishak Sep 12, 2023
1453cf7
Merge branch 'master' into 132258-clean-up-commands-for-analytics-opt…
eliasyishak Sep 12, 2023
649f203
Fix tests
eliasyishak Sep 12, 2023
a3f12d5
Merge branch 'master' into 132258-clean-up-commands-for-analytics-opt…
eliasyishak Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 11 additions & 11 deletions packages/flutter_tools/lib/runner.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,20 @@ Future<int> run(
StackTrace? firstStackTrace;
return runZoned<Future<int>>(() async {
try {
if (args.contains('--disable-telemetry') &&
args.contains('--enable-telemetry')) {
if (args.contains('--disable-analytics') &&
args.contains('--enable-analytics')) {
throwToolExit(
'Both enable and disable telemetry commands were detected '
'Both enable and disable analytics commands were detected '
'when only one can be supplied per invocation.',
exitCode: 1);
}

// Disable analytics if user passes in the `--disable-telemetry` option
// `flutter --disable-telemetry`
// Disable analytics if user passes in the `--disable-analytics` option
// "flutter --disable-analytics"
//
// Same functionality as `flutter config --no-analytics` for disabling
// Same functionality as "flutter config --no-analytics" for disabling
// except with the `value` hard coded as false
if (args.contains('--disable-telemetry')) {
if (args.contains('--disable-analytics')) {
// The tool sends the analytics event *before* toggling the flag
// intentionally to be sure that opt-out events are sent correctly.
AnalyticsConfigEvent(enabled: false).send();
Expand All @@ -95,12 +95,12 @@ Future<int> run(
await globals.analytics.setTelemetry(false);
}

// Enable analytics if user passes in the `--enable-telemetry` option
// `flutter --enable-telemetry`
// Enable analytics if user passes in the `--enable-analytics` option
// `flutter --enable-analytics`
//
// Same functionality as `flutter config --analytics` for enabling
// except with the `value` hard coded as true
if (args.contains('--enable-telemetry')) {
if (args.contains('--enable-analytics')) {
// The tool sends the analytics event *before* toggling the flag
// intentionally to be sure that opt-out events are sent correctly.
AnalyticsConfigEvent(enabled: true).send();
Expand Down Expand Up @@ -317,7 +317,7 @@ Future<int> _exit(int code, {required ShutdownHooks shutdownHooks}) async {
'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 `flutter --disable-telemetry`\n');
'You can disable analytics reporting by running `flutter --disable-analytics`\n');
}

// Invoking this will onboard the flutter tool onto
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter_tools/lib/src/commands/config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import '../runner/flutter_command.dart';
class ConfigCommand extends FlutterCommand {
ConfigCommand({ bool verboseHelp = false }) {
argParser.addFlag('analytics',
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to hide this one by default, since it's redundant with the other ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a good idea, i have hidden it so that it only shows up when running in verbose mode

help: 'Enable or disable reporting anonymously tool usage statistics and crash reports.');
help: 'Enable or disable reporting anonymously tool usage statistics and crash reports.\n'
'(An alias for "--enable-telemetry" and "--disable-telemetry".)');
eliasyishak marked this conversation as resolved.
Show resolved Hide resolved
argParser.addFlag('clear-ios-signing-cert',
negatable: false,
help: 'Clear the saved development certificate choice used to sign apps for iOS device deployment.');
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/globals.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ final BotDetector _defaultBotDetector = BotDetector(
);
Future<bool> get isRunningOnBot => botDetector.isRunningOnBot;

// Analytics instance for package:unified_analytics for telemetry
// Analytics instance for package:unified_analytics for analytics
// reporting for all Flutter and Dart related tooling
Analytics get analytics => context.get<Analytics>()!;

Expand Down
20 changes: 10 additions & 10 deletions packages/flutter_tools/lib/src/runner/flutter_command_runner.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ abstract final class FlutterGlobalOptions {
static const String kColorFlag = 'color';
static const String kContinuousIntegrationFlag = 'ci';
static const String kDeviceIdOption = 'device-id';
static const String kDisableTelemetryFlag = 'disable-telemetry';
static const String kEnableTelemetryFlag = 'enable-telemetry';
static const String kDisableAnalyticsFlag = 'disable-analytics';
static const String kEnableAnalyticsFlag = 'enable-analytics';
static const String kLocalEngineOption = 'local-engine';
static const String kLocalEngineSrcPathOption = 'local-engine-src-path';
static const String kLocalEngineHostOption = 'local-engine-host';
Expand Down Expand Up @@ -101,17 +101,17 @@ class FlutterCommandRunner extends CommandRunner<void> {
defaultsTo: true,
hide: !verboseHelp,
help: 'Allow Flutter to check for updates when this command runs.');
argParser.addFlag(FlutterGlobalOptions.kSuppressAnalyticsFlag,
argParser.addFlag(FlutterGlobalOptions.kEnableAnalyticsFlag,
negatable: false,
help: 'Suppress analytics reporting for the current CLI invocation.');
argParser.addFlag(FlutterGlobalOptions.kDisableTelemetryFlag,
help: 'Enable telemetry reporting each time a flutter or dart '
'command runs.');
argParser.addFlag(FlutterGlobalOptions.kDisableAnalyticsFlag,
negatable: false,
help: 'Disable telemetry reporting each time a flutter or dart '
'command runs, until it is re-enabled.');
argParser.addFlag(FlutterGlobalOptions.kEnableTelemetryFlag,
argParser.addFlag(FlutterGlobalOptions.kSuppressAnalyticsFlag,
negatable: false,
help: 'Enable telemetry reporting each time a flutter or dart '
'command runs.');
help: 'Suppress analytics reporting for the current CLI invocation.');
argParser.addOption(FlutterGlobalOptions.kPackagesOption,
hide: !verboseHelp,
help: 'Path to your "package_config.json" file.');
Expand Down Expand Up @@ -237,8 +237,8 @@ class FlutterCommandRunner extends CommandRunner<void> {

// If the flag for enabling or disabling telemetry is passed in,
// we will return out
if (topLevelResults.wasParsed(FlutterGlobalOptions.kDisableTelemetryFlag) ||
topLevelResults.wasParsed(FlutterGlobalOptions.kEnableTelemetryFlag)) {
if (topLevelResults.wasParsed(FlutterGlobalOptions.kDisableAnalyticsFlag) ||
topLevelResults.wasParsed(FlutterGlobalOptions.kEnableAnalyticsFlag)) {
return;
}

Expand Down
12 changes: 6 additions & 6 deletions packages/flutter_tools/test/general.shard/runner/runner_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ void main() {
expect(globals.analytics.shouldShowMessage, true);

await runner.run(
<String>['--disable-telemetry'],
<String>['--disable-analytics'],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
flutterVersion: '[user-branch]/',
Expand All @@ -343,15 +343,15 @@ void main() {
);

testUsingContext(
'runner enabling telemetry with flag',
'runner enabling analytics with flag',
() async {
io.setExitFunctionForTests((int exitCode) {});

expect(globals.analytics.telemetryEnabled, false);
expect(globals.analytics.shouldShowMessage, false);

await runner.run(
<String>['--enable-telemetry'],
<String>['--enable-analytics'],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
flutterVersion: '[user-branch]/',
Expand All @@ -377,8 +377,8 @@ void main() {

final int exitCode = await runner.run(
<String>[
'--disable-telemetry',
'--enable-telemetry',
'--disable-analytics',
'--enable-analytics',
],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
Expand Down Expand Up @@ -538,7 +538,7 @@ class WaitingCrashReporter implements CrashReporter {
}

/// A fake [Analytics] that will be used to test
/// the --disable-telemetry flag
/// the --disable-analytics flag
class FakeAnalytics extends Fake implements Analytics {

FakeAnalytics({bool fakeTelemetryStatusOverride = true})
Expand Down