Skip to content

Commit

Permalink
Clearer text about what happens with --disable-telemetry + enable-t…
Browse files Browse the repository at this point in the history
…elemetry command (#125995)

Fixes:
- #124411

This PR is cleaning up the `--disable-telemetry` help message to make it clear that opting out will opt out of all telemetry collection for flutter and dart commands. It is also adding the opposite flag `--enable-telemetry` which will enable telemetry collection
  • Loading branch information
eliasyishak committed May 8, 2023
1 parent 6db4162 commit 0d58752
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 20 deletions.
44 changes: 34 additions & 10 deletions packages/flutter_tools/lib/runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,31 +63,55 @@ Future<int> run(
StackTrace? firstStackTrace;
return runZoned<Future<int>>(() async {
try {
if (args.contains('--disable-telemetry') &&
args.contains('--enable-telemetry')) {
throwToolExit(
'Both enable and disable telemetry 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`
//
// Same functionality as `flutter config --no-analytics` for disabling
// except with the `value` hard coded as false
if (args.contains('--disable-telemetry')) {
const bool value = false;
// The tool sends the analytics event *before* toggling the flag
// intentionally to be sure that opt-out events are sent correctly.
AnalyticsConfigEvent(enabled: value).send();
if (!value) {
// Normally, the tool waits for the analytics to all send before the
// tool exits, but only when analytics are enabled. When reporting that
// analytics have been disable, the wait must be done here instead.
await globals.flutterUsage.ensureAnalyticsSent();
}
globals.flutterUsage.enabled = value;
AnalyticsConfigEvent(enabled: false).send();

// Normally, the tool waits for the analytics to all send before the
// tool exits, but only when analytics are enabled. When reporting that
// analytics have been disable, the wait must be done here instead.
await globals.flutterUsage.ensureAnalyticsSent();

globals.flutterUsage.enabled = false;
globals.printStatus('Analytics reporting disabled.');

// TODO(eliasyishak): Set the telemetry for the unified_analytics
// package as well, the above will be removed once we have
// fully transitioned to using the new package
await globals.analytics.setTelemetry(value);
await globals.analytics.setTelemetry(false);
}

// Enable analytics if user passes in the `--enable-telemetry` option
// `flutter --enable-telemetry`
//
// Same functionality as `flutter config --analytics` for enabling
// except with the `value` hard coded as true
if (args.contains('--enable-telemetry')) {
// 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();

globals.flutterUsage.enabled = true;
globals.printStatus('Analytics reporting enabled.');

await globals.analytics.setTelemetry(true);
}


await runner.run(args);

// Triggering [runZoned]'s error callback does not necessarily mean that
Expand Down
13 changes: 10 additions & 3 deletions packages/flutter_tools/lib/src/runner/flutter_command_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ class FlutterCommandRunner extends CommandRunner<void> {
help: 'Suppress analytics reporting for the current CLI invocation.');
argParser.addFlag('disable-telemetry',
negatable: false,
help: 'Disable telemetry reporting when this command runs.');
help: 'Disable telemetry reporting each time a flutter or dart '
'command runs, until it is re-enabled.');
argParser.addFlag('enable-telemetry',
negatable: false,
help: 'Enable telemetry reporting each time a flutter or dart '
'command runs.');
argParser.addOption('packages',
hide: !verboseHelp,
help: 'Path to your "package_config.json" file.');
Expand Down Expand Up @@ -188,8 +193,10 @@ class FlutterCommandRunner extends CommandRunner<void> {
Future<void> runCommand(ArgResults topLevelResults) async {
final Map<Type, Object?> contextOverrides = <Type, Object?>{};

// If the disable-telemetry flag has been passed, return out
if (topLevelResults.wasParsed('disable-telemetry')) {
// If the flag for enabling or disabling telemetry is passed in,
// we will return out
if (topLevelResults.wasParsed('disable-telemetry') ||
topLevelResults.wasParsed('enable-telemetry')) {
return;
}

Expand Down
73 changes: 66 additions & 7 deletions packages/flutter_tools/test/general.shard/runner/runner_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,6 @@ void main() {
});

group('unified_analytics', () {
final Usage legacyAnalytics = TestUsage();
setUp(() {
legacyAnalytics.enabled = false;
});

testUsingContext(
'runner disable telemetry with flag',
() async {
Expand All @@ -346,6 +341,62 @@ void main() {
ProcessManager: () => FakeProcessManager.any(),
},
);

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

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

await runner.run(
<String>['--enable-telemetry'],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
flutterVersion: '[user-branch]/',
shutdownHooks: ShutdownHooks(),
);

expect(globals.analytics.telemetryEnabled, true);
},
overrides: <Type, Generator>{
Analytics: () => FakeAnalytics(fakeTelemetryStatusOverride: false),
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
},
);

testUsingContext(
'throw error when both flags passed',
() async {
io.setExitFunctionForTests((int exitCode) {});

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

final int exitCode = await runner.run(
<String>[
'--disable-telemetry',
'--enable-telemetry',
],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
flutterVersion: '[user-branch]/',
shutdownHooks: ShutdownHooks(),
);

expect(exitCode, 1,
reason: 'Should return 1 due to conflicting options for telemetry');
expect(globals.analytics.telemetryEnabled, true,
reason: 'Should not have changed from initialization');
},
overrides: <Type, Generator>{
Analytics: () => FakeAnalytics(),
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
},
);
});
}

Expand Down Expand Up @@ -489,8 +540,16 @@ class WaitingCrashReporter implements CrashReporter {
/// A fake [Analytics] that will be used to test
/// the --disable-telemetry flag
class FakeAnalytics extends Fake implements Analytics {
bool _fakeTelemetryStatus = true;
bool _fakeShowMessage = true;

FakeAnalytics({bool fakeTelemetryStatusOverride = true})
: _fakeTelemetryStatus = fakeTelemetryStatusOverride,
_fakeShowMessage = fakeTelemetryStatusOverride;

// Both of the members below can be initialized with [fakeTelemetryStatusOverride]
// because if we pass in false for the status, that means we can also
// assume the message has been shown before
bool _fakeTelemetryStatus;
bool _fakeShowMessage;

@override
String get getConsentMessage => 'message';
Expand Down

0 comments on commit 0d58752

Please sign in to comment.