[dartdev] Reimplement the dartdev analytics with events and custom dimensions, matching the flutter cli analytics patterns where possible

Bug: dart-lang/sdk#43198
Change-Id: Iff3ac619965142c10864559ff396b64130f18daf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161200
Reviewed-by: Devon Carew <[email protected]>
Commit-Queue: Jaime Wren <[email protected]>

Author: jwren

pkg/dartdev/lib/dartdev.dart

@@ -22,6 +22,7 @@ import 'src/commands/pub.dart';
 import 'src/commands/run.dart';
 import 'src/commands/test.dart';
 import 'src/core.dart';
+import 'src/events.dart';
 import 'src/experiments.dart';
 import 'src/sdk.dart';
 import 'src/utils.dart';
@@ -35,10 +36,6 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
   final stopwatch = Stopwatch();
   int result;
 
-  // The Analytics instance used to report information back to Google Analytics,
-  // see lib/src/analytics.dart.
-  Analytics analytics;
-
   // The exit code for the dartdev process, null indicates that it has not yet
   // been set yet. The value is set in the catch and finally blocks below.
   int exitCode;
@@ -47,7 +44,9 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
   Object exception;
   StackTrace stackTrace;
 
-  analytics =
+  // The Analytics instance used to report information back to Google Analytics,
+  // see lib/src/analytics.dart.
+  Analytics analytics =
       createAnalyticsInstance(args.contains('--disable-dartdev-analytics'));
 
   // If we have not printed the analyticsNoticeOnFirstRunMessage to stdout,
@@ -92,11 +91,13 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
     // RunCommand.ddsHost = ddsUrl[0];
     // RunCommand.ddsPort = ddsUrl[1];
   }
+
   String commandName;
 
   try {
     stopwatch.start();
     final runner = DartdevRunner(args);
+
     // Run can't be called with the '--disable-dartdev-analytics' flag, remove
     // it if it is contained in args.
     if (args.contains('--disable-dartdev-analytics')) {
@@ -119,14 +120,6 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
         )
         .toList();
 
-    // Before calling to run, send the first ping to analytics to have the first
-    // ping, as well as the command itself, running in parallel.
-    if (analytics.enabled) {
-      commandName = getCommandStr(args, runner.commands.keys.toList());
-      // ignore: unawaited_futures
-      analytics.sendEvent(eventCategory, commandName);
-    }
-
     // If ... help pub ... is in the args list, remove 'help', and add '--help'
     // to the end of the list.  This will make it possible to use the help
     // command to access subcommands of pub such as `dart help pub publish`, see
@@ -135,6 +128,17 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
       args = PubUtils.modifyArgs(args);
     }
 
+    // For the commands format and migrate, dartdev itself sends the
+    // sendScreenView notification to analytics, for all other
+    // dartdev commands (instances of DartdevCommand) the commands send this
+    // to analytics.
+    commandName = ArgParserUtils.getCommandStr(args);
+    if (analytics.enabled &&
+        (commandName == formatCmdName || commandName == migrateCmdName)) {
+      // ignore: unawaited_futures
+      analytics.sendScreenView(commandName);
+    }
+
     // Finally, call the runner to execute the command, see DartdevRunner.
     result = await runner.run(args);
   } catch (e, st) {
@@ -157,7 +161,22 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
 
     // Send analytics before exiting
     if (analytics.enabled) {
-      analytics.setSessionValue(exitCodeParam, exitCode);
+      // For commands that are not DartdevCommand instances, we manually create
+      // and send the UsageEvent from here:
+      if (commandName == formatCmdName) {
+        // ignore: unawaited_futures
+        FormatUsageEvent(
+          exitCode: exitCode,
+          args: args,
+        ).send(analyticsInstance);
+      } else if (commandName == migrateCmdName) {
+        // ignore: unawaited_futures
+        MigrateUsageEvent(
+          exitCode: exitCode,
+          args: args,
+        ).send(analyticsInstance);
+      }
+
       // ignore: unawaited_futures
       analytics.sendTiming(commandName, stopwatch.elapsedMilliseconds,
           category: 'commands');

pkg/dartdev/lib/src/analytics.dart

@@ -26,7 +26,6 @@ const String analyticsDisabledNoticeMessage = '''
   ║ `dart --enable-analytics`                                                  ║
   ╚════════════════════════════════════════════════════════════════════════════╝
 ''';
-const String _unknownCommand = '<unknown>';
 const String _appName = 'dartdev';
 const String _dartDirectoryName = '.dart';
 const String _settingsFileName = 'dartdev.json';
@@ -40,30 +39,32 @@ Dart programming language (https://dart.dev).
 const String eventCategory = 'dartdev';
 const String exitCodeParam = 'exitCode';
 
-Analytics instance;
+Analytics _instance;
+
+Analytics get analyticsInstance => _instance;
 
 /// Create and return an [Analytics] instance, this value is cached and returned
 /// on subsequent calls.
 Analytics createAnalyticsInstance(bool disableAnalytics) {
-  if (instance != null) {
-    return instance;
+  if (_instance != null) {
+    return _instance;
   }
 
   // Dartdev tests pass a hidden 'disable-dartdev-analytics' flag which is
   // handled here.
   // Also, stdout.hasTerminal is checked, if there is no terminal we infer that
   // a machine is running dartdev so we return analytics shouldn't be set.
   if (disableAnalytics) {
-    instance = DisabledAnalytics(_trackingId, _appName);
-    return instance;
+    _instance = DisabledAnalytics(_trackingId, _appName);
+    return _instance;
   }
 
   var settingsDir = getDartStorageDirectory();
   if (settingsDir == null) {
     // Some systems don't support user home directories; for those, fail
     // gracefully by returning a disabled analytics object.
-    instance = DisabledAnalytics(_trackingId, _appName);
-    return instance;
+    _instance = DisabledAnalytics(_trackingId, _appName);
+    return _instance;
   }
 
   if (!settingsDir.existsSync()) {
@@ -72,8 +73,8 @@ Analytics createAnalyticsInstance(bool disableAnalytics) {
     } catch (e) {
       // If we can't create the directory for the analytics settings, fail
       // gracefully by returning a disabled analytics object.
-      instance = DisabledAnalytics(_trackingId, _appName);
-      return instance;
+      _instance = DisabledAnalytics(_trackingId, _appName);
+      return _instance;
     }
   }
 
@@ -85,22 +86,8 @@ Analytics createAnalyticsInstance(bool disableAnalytics) {
   }
 
   var settingsFile = File(path.join(settingsDir.path, _settingsFileName));
-  instance = DartdevAnalytics(_trackingId, settingsFile, _appName);
-  return instance;
-}
-
-/// Return the first member from [args] that occurs in [allCommands], otherwise
-/// '<unknown>' is returned.
-///
-/// 'help' is special cased to have 'dart analyze help', 'dart help analyze',
-/// and 'dart analyze --help' all be recorded as a call to 'help' instead of
-/// 'help' and 'analyze'.
-String getCommandStr(List<String> args, List<String> allCommands) {
-  if (args.contains('help') || args.contains('-h') || args.contains('--help')) {
-    return 'help';
-  }
-  return args.firstWhere((arg) => allCommands.contains(arg),
-      orElse: () => _unknownCommand);
+  _instance = DartdevAnalytics(_trackingId, settingsFile, _appName);
+  return _instance;
 }
 
 /// The directory used to store the analytics settings file.

pkg/dartdev/lib/src/commands/analyze.dart

@@ -5,22 +5,26 @@
 import 'dart:async';
 import 'dart:io';
 
+import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 
 import '../core.dart';
+import '../events.dart';
 import '../sdk.dart';
 import '../utils.dart';
 import 'analyze_impl.dart';
 
 class AnalyzeCommand extends DartdevCommand<int> {
+  static const String cmdName = 'analyze';
+
   /// The maximum length of any of the existing severity labels.
   static const int _severityWidth = 7;
 
   /// The number of spaces needed to indent follow-on lines (the body) under the
   /// message. The width left for the severity label plus the separator width.
   static const int _bodyIndentWidth = _severityWidth + 3;
 
-  AnalyzeCommand() : super('analyze', "Analyze the project's Dart code.") {
+  AnalyzeCommand() : super(cmdName, "Analyze the project's Dart code.") {
     argParser
       ..addFlag('fatal-infos',
           help: 'Treat info level issues as fatal.', negatable: false)
@@ -32,7 +36,7 @@ class AnalyzeCommand extends DartdevCommand<int> {
   String get invocation => '${super.invocation} [<directory>]';
 
   @override
-  FutureOr<int> run() async {
+  FutureOr<int> runImpl() async {
     if (argResults.rest.length > 1) {
       usageException('Only one directory is expected.');
     }
@@ -153,4 +157,19 @@ class AnalyzeCommand extends DartdevCommand<int> {
       return 0;
     }
   }
+
+  @override
+  UsageEvent createUsageEvent(int exitCode) => AnalyzeUsageEvent(
+        usagePath,
+        exitCode: exitCode,
+        args: argResults.arguments,
+      );
+}
+
+/// The [UsageEvent] for the analyze command.
+class AnalyzeUsageEvent extends UsageEvent {
+  AnalyzeUsageEvent(String usagePath,
+      {String label, @required int exitCode, @required List<String> args})
+      : super(AnalyzeCommand.cmdName, usagePath,
+            label: label, args: args, exitCode: exitCode);
 }

pkg/dartdev/lib/src/commands/compile.dart

@@ -6,9 +6,11 @@ import 'dart:async';
 import 'dart:io';
 
 import 'package:dart2native/generate.dart';
+import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 
 import '../core.dart';
+import '../events.dart';
 import '../sdk.dart';
 import '../vm_interop_handler.dart';
 
@@ -39,12 +41,13 @@ bool checkFile(String sourcePath) {
     stderr.flush();
     return false;
   }
-
   return true;
 }
 
-class CompileJSCommand extends DartdevCommand<int> {
-  CompileJSCommand() : super('js', 'Compile Dart to JavaScript.') {
+class CompileJSCommand extends CompileSubcommandCommand {
+  static const String cmdName = 'js';
+
+  CompileJSCommand() : super(cmdName, 'Compile Dart to JavaScript.') {
     argParser
       ..addOption(
         commonOptions['outputFile'].flag,
@@ -63,7 +66,7 @@ class CompileJSCommand extends DartdevCommand<int> {
   String get invocation => '${super.invocation} <dart entry point>';
 
   @override
-  FutureOr<int> run() async {
+  FutureOr<int> runImpl() async {
     if (!Sdk.checkArtifactExists(sdk.dart2jsSnapshot)) {
       return 255;
     }
@@ -97,7 +100,10 @@ class CompileJSCommand extends DartdevCommand<int> {
   }
 }
 
-class CompileSnapshotCommand extends DartdevCommand<int> {
+class CompileSnapshotCommand extends CompileSubcommandCommand {
+  static const String jitSnapshotCmdName = 'jit-snapshot';
+  static const String kernelCmdName = 'kernel';
+
   final String commandName;
   final String help;
   final String fileExt;
@@ -121,7 +127,7 @@ class CompileSnapshotCommand extends DartdevCommand<int> {
   String get invocation => '${super.invocation} <dart entry point>';
 
   @override
-  FutureOr<int> run() async {
+  FutureOr<int> runImpl() async {
     // We expect a single rest argument; the dart entry point.
     if (argResults.rest.length != 1) {
       // This throws.
@@ -157,7 +163,10 @@ class CompileSnapshotCommand extends DartdevCommand<int> {
   }
 }
 
-class CompileNativeCommand extends DartdevCommand<int> {
+class CompileNativeCommand extends CompileSubcommandCommand {
+  static const String exeCmdName = 'exe';
+  static const String aotSnapshotCmdName = 'aot-snapshot';
+
   final String commandName;
   final String format;
   final String help;
@@ -194,7 +203,7 @@ Remove debugging information from the output and save it separately to the speci
   String get invocation => '${super.invocation} <dart entry point>';
 
   @override
-  FutureOr<int> run() async {
+  FutureOr<int> runImpl() async {
     if (!Sdk.checkArtifactExists(genKernel) ||
         !Sdk.checkArtifactExists(genSnapshot)) {
       return 255;
@@ -230,30 +239,64 @@ Remove debugging information from the output and save it separately to the speci
   }
 }
 
-class CompileCommand extends DartdevCommand {
-  CompileCommand() : super('compile', 'Compile Dart to various formats.') {
+abstract class CompileSubcommandCommand extends DartdevCommand<int> {
+  CompileSubcommandCommand(String name, String description,
+      {bool hidden = false})
+      : super(name, description, hidden: hidden);
+
+  @override
+  UsageEvent createUsageEvent(int exitCode) => CompileUsageEvent(
+        usagePath,
+        exitCode: exitCode,
+        args: argResults.arguments,
+      );
+}
+
+class CompileCommand extends DartdevCommand<int> {
+  static const String cmdName = 'compile';
+
+  CompileCommand() : super(cmdName, 'Compile Dart to various formats.') {
     addSubcommand(CompileJSCommand());
     addSubcommand(CompileSnapshotCommand(
-      commandName: 'jit-snapshot',
+      commandName: CompileSnapshotCommand.jitSnapshotCmdName,
       help: 'to a JIT snapshot.',
       fileExt: 'jit',
       formatName: 'app-jit',
     ));
     addSubcommand(CompileSnapshotCommand(
-      commandName: 'kernel',
+      commandName: CompileSnapshotCommand.kernelCmdName,
       help: 'to a kernel snapshot.',
       fileExt: 'dill',
       formatName: 'kernel',
     ));
     addSubcommand(CompileNativeCommand(
-      commandName: 'exe',
+      commandName: CompileNativeCommand.exeCmdName,
       help: 'to a self-contained executable.',
       format: 'exe',
     ));
     addSubcommand(CompileNativeCommand(
-      commandName: 'aot-snapshot',
+      commandName: CompileNativeCommand.aotSnapshotCmdName,
       help: 'to an AOT snapshot.',
       format: 'aot',
     ));
   }
+
+  @override
+  UsageEvent createUsageEvent(int exitCode) => null;
+
+  @override
+  FutureOr<int> runImpl() {
+    // do nothing, this command is never run
+    return 0;
+  }
+}
+
+/// The [UsageEvent] for all compile commands, we could have each compile
+/// event be its own class instance, but for the time being [usagePath] takes
+/// care of the only difference.
+class CompileUsageEvent extends UsageEvent {
+  CompileUsageEvent(String usagePath,
+      {String label, @required int exitCode, @required List<String> args})
+      : super(CompileCommand.cmdName, usagePath,
+            label: label, exitCode: exitCode, args: args);
 }