Unified analytics events for doctor validators (#136647)

Related to tracking issue:
- https://github.com/flutter/flutter/issues/128251

This PR sends analytic events for each of the doctor validators.

This PR below will need to land first in `dart-lang/tools` before this merges.

packages/flutter_tools/lib/runner.dart

@@ -105,6 +105,9 @@ Future<int> run(
           globals.flutterUsage.enabled = true;
           globals.printStatus('Analytics reporting enabled.');
 
+          // 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(true);
         }
 

packages/flutter_tools/lib/src/base/process.dart

@@ -623,6 +623,12 @@ Future<int> exitWithHooks(int code, {required ShutdownHooks shutdownHooks}) asyn
 
   final Completer<void> completer = Completer<void>();
 
+  // Allow any pending analytics events to send and close the http connection
+  //
+  // By default, we will wait 250 ms before canceling any pending events, we
+  // can change the [delayDuration] in the close method if it needs to be changed
+  await globals.analytics.close();
+
   // Give the task / timer queue one cycle through before we hard exit.
   Timer.run(() {
     try {

packages/flutter_tools/lib/src/context_runner.dart

@@ -218,7 +218,10 @@ Future<T> runInContext<T>(
         logger: globals.logger,
         botDetector: globals.botDetector,
       ),
-      Doctor: () => Doctor(logger: globals.logger),
+      Doctor: () => Doctor(
+        logger: globals.logger,
+        clock: globals.systemClock,
+      ),
       DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,
       EmulatorManager: () => EmulatorManager(
         java: globals.java,

packages/flutter_tools/lib/src/doctor.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 
 import 'package:meta/meta.dart';
 import 'package:process/process.dart';
+import 'package:unified_analytics/unified_analytics.dart';
 
 import 'android/android_studio_validator.dart';
 import 'android/android_workflow.dart';
@@ -19,6 +20,7 @@ import 'base/net.dart';
 import 'base/os.dart';
 import 'base/platform.dart';
 import 'base/terminal.dart';
+import 'base/time.dart';
 import 'base/user_messages.dart';
 import 'base/utils.dart';
 import 'cache.dart';
@@ -229,9 +231,15 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
 class Doctor {
   Doctor({
     required Logger logger,
-  }) : _logger = logger;
+    required SystemClock clock,
+    Analytics? analytics,
+  })  : _logger = logger,
+        _clock = clock,
+        _analytics = analytics ?? globals.analytics;
 
   final Logger _logger;
+  final SystemClock _clock;
+  final Analytics _analytics;
 
   List<DoctorValidator> get validators {
     return DoctorValidatorsProvider._instance.validators;
@@ -375,6 +383,10 @@ class Doctor {
     bool doctorResult = true;
     int issues = 0;
 
+    // This timestamp will be used on the backend of GA4 to group each of the events that
+    // were sent for each doctor validator and its result
+    final int analyticsTimestamp = _clock.now().millisecondsSinceEpoch;
+
     for (final ValidatorTask validatorTask in startedValidatorTasks ?? startValidatorTasks()) {
       final DoctorValidator validator = validatorTask.validator;
       final Status status = _logger.startSpinner(
@@ -404,6 +416,37 @@ class Doctor {
           break;
       }
       if (sendEvent) {
+        if (validator is GroupedValidator) {
+          for (int i = 0; i < validator.subValidators.length; i++) {
+            final DoctorValidator subValidator = validator.subValidators[i];
+
+            // Ensure that all of the subvalidators in the group have
+            // a corresponding subresult incase a validator crashed
+            final ValidationResult subResult;
+            try {
+              subResult = validator.subResults[i];
+            } on RangeError {
+              continue;
+            }
+
+            _analytics.send(Event.doctorValidatorResult(
+              validatorName: subValidator.title,
+              result: subResult.typeStr,
+              statusInfo: subResult.statusInfo,
+              partOfGroupedValidator: true,
+              doctorInvocationId: analyticsTimestamp,
+            ));
+          }
+        } else {
+          _analytics.send(Event.doctorValidatorResult(
+            validatorName: validator.title,
+            result: result.typeStr,
+            statusInfo: result.statusInfo,
+            partOfGroupedValidator: false,
+            doctorInvocationId: analyticsTimestamp,
+          ));
+        }
+        // TODO(eliasyishak): remove this after migrating from package:usage
         DoctorResultEvent(validator: validator, result: result).send();
       }
 
@@ -727,8 +770,10 @@ class DeviceValidator extends DoctorValidator {
 class DoctorText {
   DoctorText(
     BufferLogger logger, {
+    SystemClock? clock,
     @visibleForTesting Doctor? doctor,
-  }) : _doctor = doctor ?? Doctor(logger: logger), _logger = logger;
+  })  : _doctor = doctor ?? Doctor(logger: logger, clock: clock ?? globals.systemClock),
+        _logger = logger;
 
   final BufferLogger _logger;
   final Doctor _doctor;

packages/flutter_tools/lib/src/runner/flutter_command_runner.dart

@@ -304,6 +304,7 @@ class FlutterCommandRunner extends CommandRunner<void> {
 
         if ((topLevelResults[FlutterGlobalOptions.kSuppressAnalyticsFlag] as bool?) ?? false) {
           globals.flutterUsage.suppressAnalytics = true;
+          globals.analytics.suppressTelemetry();
         }
 
         globals.flutterVersion.ensureVersionFile();

packages/flutter_tools/test/general.shard/runner/runner_test.dart

@@ -18,12 +18,12 @@ import 'package:flutter_tools/src/globals.dart' as globals;
 import 'package:flutter_tools/src/reporting/crash_reporting.dart';
 import 'package:flutter_tools/src/reporting/reporting.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
-import 'package:test/fake.dart';
 import 'package:unified_analytics/unified_analytics.dart';
 
 import '../../src/common.dart';
 import '../../src/context.dart';
 import '../../src/fake_http_client.dart';
+import '../../src/fakes.dart';
 
 const String kCustomBugInstructions = 'These are instructions to report with a custom bug tracker.';
 
@@ -317,13 +317,24 @@ void main() {
   });
 
   group('unified_analytics', () {
+    late FakeAnalytics fakeAnalytics;
+    late MemoryFileSystem fs;
+
+    setUp(() {
+      fs = MemoryFileSystem.test();
+
+      fakeAnalytics = getInitializedFakeAnalyticsInstance(
+        fs: fs,
+        fakeFlutterVersion: FakeFlutterVersion(),
+      );
+    });
+
     testUsingContext(
       'runner disable telemetry with flag',
       () async {
         io.setExitFunctionForTests((int exitCode) {});
 
         expect(globals.analytics.telemetryEnabled, true);
-        expect(globals.analytics.shouldShowMessage, true);
 
         await runner.run(
           <String>['--disable-analytics'],
@@ -336,7 +347,7 @@ void main() {
         expect(globals.analytics.telemetryEnabled, false);
       },
       overrides: <Type, Generator>{
-        Analytics: () => FakeAnalytics(),
+        Analytics: () => fakeAnalytics,
         FileSystem: () => MemoryFileSystem.test(),
         ProcessManager: () => FakeProcessManager.any(),
       },
@@ -347,8 +358,17 @@ void main() {
       () async {
         io.setExitFunctionForTests((int exitCode) {});
 
+        expect(globals.analytics.telemetryEnabled, true);
+
+        await runner.run(
+          <String>['--disable-analytics'],
+          () => <FlutterCommand>[],
+          // This flutterVersion disables crash reporting.
+          flutterVersion: '[user-branch]/',
+          shutdownHooks: ShutdownHooks(),
+        );
+
         expect(globals.analytics.telemetryEnabled, false);
-        expect(globals.analytics.shouldShowMessage, false);
 
         await runner.run(
           <String>['--enable-analytics'],
@@ -361,7 +381,7 @@ void main() {
         expect(globals.analytics.telemetryEnabled, true);
       },
       overrides: <Type, Generator>{
-        Analytics: () => FakeAnalytics(fakeTelemetryStatusOverride: false),
+        Analytics: () => fakeAnalytics,
         FileSystem: () => MemoryFileSystem.test(),
         ProcessManager: () => FakeProcessManager.any(),
       },
@@ -373,7 +393,6 @@ void main() {
         io.setExitFunctionForTests((int exitCode) {});
 
         expect(globals.analytics.telemetryEnabled, true);
-        expect(globals.analytics.shouldShowMessage, true);
 
         final int exitCode = await runner.run(
           <String>[
@@ -392,7 +411,7 @@ void main() {
             reason: 'Should not have changed from initialization');
       },
       overrides: <Type, Generator>{
-        Analytics: () => FakeAnalytics(),
+        Analytics: () => fakeAnalytics,
         FileSystem: () => MemoryFileSystem.test(),
         ProcessManager: () => FakeProcessManager.any(),
       },
@@ -536,38 +555,3 @@ class WaitingCrashReporter implements CrashReporter {
     return _future;
   }
 }
-
-/// A fake [Analytics] that will be used to test
-/// the --disable-analytics flag
-class FakeAnalytics extends Fake implements Analytics {
-
-  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';
-
-  @override
-  bool get shouldShowMessage => _fakeShowMessage;
-
-  @override
-  void clientShowedMessage() {
-    _fakeShowMessage = false;
-  }
-
-  @override
-  Future<void> setTelemetry(bool reportingBool) {
-    _fakeTelemetryStatus = reportingBool;
-    return Future<void>.value();
-  }
-
-  @override
-  bool get telemetryEnabled => _fakeTelemetryStatus;
-}

packages/flutter_tools/test/general.shard/unified_analytics_test.dart

@@ -5,7 +5,6 @@
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/reporting/unified_analytics.dart';
-import 'package:unified_analytics/src/enums.dart';
 import 'package:unified_analytics/unified_analytics.dart';
 
 import '../src/common.dart';
@@ -13,42 +12,17 @@ import '../src/fakes.dart';
 
 void main() {
   const String userBranch = 'abc123';
-  const String homeDirectoryName = 'home';
-  const DashTool tool = DashTool.flutterTool;
 
   late FileSystem fs;
-  late Directory home;
   late FakeAnalytics analyticsOverride;
 
   setUp(() {
     fs = MemoryFileSystem.test();
-    home = fs.directory(homeDirectoryName);
-
-    // Prepare the tests by "onboarding" the tool into the package
-    // by invoking the [clientShowedMessage] method for the provided
-    // [tool]
-    final FakeAnalytics initialAnalytics = FakeAnalytics(
-      tool: tool,
-      homeDirectory: home,
-      dartVersion: '3.0.0',
-      platform: DevicePlatform.macos,
-      fs: fs,
-      surveyHandler: SurveyHandler(
-        homeDirectory: home,
-        fs: fs,
-      ),
-    );
-    initialAnalytics.clientShowedMessage();
 
-    analyticsOverride = FakeAnalytics(
-      tool: tool,
-      homeDirectory: home,
-      dartVersion: '3.0.0',
-      platform: DevicePlatform.macos,
+    analyticsOverride = getInitializedFakeAnalyticsInstance(
       fs: fs,
-      surveyHandler: SurveyHandler(
-        homeDirectory: home,
-        fs: fs,
+      fakeFlutterVersion: FakeFlutterVersion(
+        branch: userBranch,
       ),
     );
   });
@@ -135,5 +109,17 @@ void main() {
       );
       expect(analytics, isA<NoOpAnalytics>());
     });
+
+    testWithoutContext('Suppression prevents events from being sent', () {
+      expect(analyticsOverride.okToSend, true);
+      analyticsOverride.send(Event.surveyShown(surveyId: 'surveyId'));
+      expect(analyticsOverride.sentEvents, hasLength(1));
+
+      analyticsOverride.suppressTelemetry();
+      expect(analyticsOverride.okToSend, false);
+      analyticsOverride.send(Event.surveyShown(surveyId: 'surveyId'));
+
+      expect(analyticsOverride.sentEvents, hasLength(1));
+    });
   });
 }

packages/flutter_tools/test/src/common.dart

@@ -16,6 +16,10 @@ import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path; // flutter_ignore: package_path_import
 import 'package:test/test.dart' as test_package show test;
 import 'package:test/test.dart' hide test;
+import 'package:unified_analytics/src/enums.dart';
+import 'package:unified_analytics/unified_analytics.dart';
+
+import 'fakes.dart';
 
 export 'package:path/path.dart' show Context; // flutter_ignore: package_path_import
 export 'package:test/test.dart' hide isInstanceOf, test;
@@ -305,3 +309,38 @@ class FileExceptionHandler {
     throw exception;
   }
 }
+
+/// This method is required to fetch an instance of [FakeAnalytics]
+/// because there is initialization logic that is required. An initial
+/// instance will first be created and will let package:unified_analytics
+/// know that the consent message has been shown. After confirming on the first
+/// instance, then a second instance will be generated and returned. This second
+/// instance will be cleared to send events.
+FakeAnalytics getInitializedFakeAnalyticsInstance({
+  required FileSystem fs,
+  required FakeFlutterVersion fakeFlutterVersion,
+}) {
+  final Directory homeDirectory = fs.directory('/');
+  final FakeAnalytics initialAnalytics = FakeAnalytics(
+    tool: DashTool.flutterTool,
+    homeDirectory: homeDirectory,
+    dartVersion: fakeFlutterVersion.dartSdkVersion,
+    platform: DevicePlatform.linux,
+    fs: fs,
+    surveyHandler: SurveyHandler(homeDirectory: homeDirectory, fs: fs),
+    flutterChannel: fakeFlutterVersion.channel,
+    flutterVersion: fakeFlutterVersion.getVersionString(),
+  );
+  initialAnalytics.clientShowedMessage();
+
+  return FakeAnalytics(
+    tool: DashTool.flutterTool,
+    homeDirectory: homeDirectory,
+    dartVersion: fakeFlutterVersion.dartSdkVersion,
+    platform: DevicePlatform.linux,
+    fs: fs,
+    surveyHandler: SurveyHandler(homeDirectory: homeDirectory, fs: fs),
+    flutterChannel: fakeFlutterVersion.channel,
+    flutterVersion: fakeFlutterVersion.getVersionString(),
+  );
+}

packages/flutter_tools/test/src/context.dart

@@ -16,6 +16,7 @@ import 'package:flutter_tools/src/base/process.dart';
 import 'package:flutter_tools/src/base/signals.dart';
 import 'package:flutter_tools/src/base/template.dart';
 import 'package:flutter_tools/src/base/terminal.dart';
+import 'package:flutter_tools/src/base/time.dart';
 import 'package:flutter_tools/src/base/version.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/context_runner.dart';
@@ -291,7 +292,8 @@ class FakeAndroidLicenseValidator extends Fake implements AndroidLicenseValidato
 }
 
 class FakeDoctor extends Doctor {
-  FakeDoctor(Logger logger) : super(logger: logger);
+  FakeDoctor(Logger logger, {super.clock = const SystemClock()})
+      : super(logger: logger);
 
   // True for testing.
   @override