Fix DevTools extension analytics (#6651)

flutter · Nov 3, 2023 · 348a6c7 · 348a6c7
1 parent 4474099
commit 348a6c7
Show file tree

Hide file tree

Showing 21 changed files with 193 additions and 40 deletions.
diff --git a/packages/devtools_app/lib/src/extensions/embedded/view.dart b/packages/devtools_app/lib/src/extensions/embedded/view.dart
@@ -32,7 +32,7 @@ class _EmbeddedExtensionViewState extends State<EmbeddedExtensionView> {
     super.initState();
     ga.impression(
       gac.DevToolsExtensionEvents.extensionScreenName(
-        widget.controller.extensionConfig.name,
+        widget.controller.extensionConfig,
       ),
       gac.DevToolsExtensionEvents.embeddedExtension.name,
     );

diff --git a/packages/devtools_app/lib/src/extensions/extension_screen.dart b/packages/devtools_app/lib/src/extensions/extension_screen.dart
@@ -55,9 +55,7 @@ class _ExtensionScreenBodyState extends State<_ExtensionScreenBody> {
 
   void _init() {
     ga.screen(
-      gac.DevToolsExtensionEvents.extensionScreenName(
-        widget.extensionConfig.name,
-      ),
+      gac.DevToolsExtensionEvents.extensionScreenName(widget.extensionConfig),
     );
     extensionController =
         createEmbeddedExtensionController(widget.extensionConfig)..init();

diff --git a/packages/devtools_app/lib/src/extensions/extension_screen_controls.dart b/packages/devtools_app/lib/src/extensions/extension_screen_controls.dart
@@ -52,7 +52,7 @@ class EmbeddedExtensionHeader extends StatelessWidget {
               url: extension.issueTrackerLink,
               gaScreenName: gac.DevToolsExtensionEvents.extensionScreenId.name,
               gaSelectedItemDescription:
-                  gac.DevToolsExtensionEvents.extensionFeedback(extensionName),
+                  gac.DevToolsExtensionEvents.extensionFeedback(extension),
             ),
             context: context,
           ),
@@ -95,7 +95,7 @@ class EmbeddedExtensionHeader extends StatelessWidget {
                         ga.select(
                           gac.DevToolsExtensionEvents.extensionScreenId.name,
                           gac.DevToolsExtensionEvents.extensionForceReload(
-                            extension.displayName,
+                            extension,
                           ),
                         );
                         onForceReload();
@@ -172,9 +172,7 @@ class DisableExtensionDialog extends StatelessWidget {
           onPressed: () {
             ga.select(
               gac.DevToolsExtensionEvents.extensionScreenId.name,
-              gac.DevToolsExtensionEvents.extensionDisableManual(
-                extension.displayName,
-              ),
+              gac.DevToolsExtensionEvents.extensionDisableManual(extension),
             );
             unawaited(
               extensionService.setExtensionEnabledState(
@@ -205,7 +203,6 @@ class EnableExtensionPrompt extends StatelessWidget {
   @override
   Widget build(BuildContext context) {
     final theme = Theme.of(context);
-    final extensionName = extension.displayName;
     return Center(
       child: Column(
         mainAxisAlignment: MainAxisAlignment.center,
@@ -245,7 +242,7 @@ class EnableExtensionPrompt extends StatelessWidget {
                 label: 'Enable',
                 gaScreen: gac.DevToolsExtensionEvents.extensionScreenId.name,
                 gaSelection: gac.DevToolsExtensionEvents.extensionEnablePrompt(
-                  extensionName,
+                  extension,
                 ),
                 elevated: true,
                 onPressed: () {
@@ -262,7 +259,7 @@ class EnableExtensionPrompt extends StatelessWidget {
                 label: 'No, hide this screen',
                 gaScreen: gac.DevToolsExtensionEvents.extensionScreenId.name,
                 gaSelection: gac.DevToolsExtensionEvents.extensionDisablePrompt(
-                  extensionName,
+                  extension,
                 ),
                 onPressed: () {
                   unawaited(

diff --git a/packages/devtools_app/lib/src/extensions/extension_settings.dart b/packages/devtools_app/lib/src/extensions/extension_settings.dart
@@ -142,9 +142,7 @@ class ExtensionSetting extends StatelessWidget {
         onPressed: () {
           ga.select(
             gac.DevToolsExtensionEvents.extensionSettingsId.name,
-            gac.DevToolsExtensionEvents.extensionEnableManual(
-              extension.name.toLowerCase(),
-            ),
+            gac.DevToolsExtensionEvents.extensionEnableManual(extension),
           );
           unawaited(
             extensionService.setExtensionEnabledState(
@@ -161,9 +159,7 @@ class ExtensionSetting extends StatelessWidget {
         onPressed: () {
           ga.select(
             gac.DevToolsExtensionEvents.extensionSettingsId.name,
-            gac.DevToolsExtensionEvents.extensionDisableManual(
-              extension.name.toLowerCase(),
-            ),
+            gac.DevToolsExtensionEvents.extensionDisableManual(extension),
           );
           unawaited(
             extensionService.setExtensionEnabledState(

diff --git a/packages/devtools_app/lib/src/shared/analytics/constants.dart b/packages/devtools_app/lib/src/shared/analytics/constants.dart
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:devtools_shared/devtools_extensions.dart';
+
 import '../screen.dart';
 
 part 'constants/_cpu_profiler_constants.dart';

diff --git a/packages/devtools_app/lib/src/shared/analytics/constants/_extension_constants.dart b/packages/devtools_app/lib/src/shared/analytics/constants/_extension_constants.dart
@@ -21,34 +21,36 @@ enum DevToolsExtensionEvents {
   embeddedExtension;
 
   /// Event sent via [ga.screen] when an extension screen is opened.
-  static String extensionScreenName(String name) => 'extension-$name';
+  static String extensionScreenName(DevToolsExtensionConfig ext) =>
+      'extension-${ext.analyticsSafeName}';
 
   /// Event sent when a user clicks the "Report an issue" link on an extension
   /// screen.
-  static String extensionFeedback(String name) => 'extensionFeedback-$name';
+  static String extensionFeedback(DevToolsExtensionConfig ext) =>
+      'extensionFeedback-${ext.analyticsSafeName}';
 
   /// Event sent when an extension is enabled because a user manually enabled
   /// it from the extensions settings menu.
-  static String extensionEnableManual(String name) =>
-      'extensionEnable-manual-$name';
+  static String extensionEnableManual(DevToolsExtensionConfig ext) =>
+      'extensionEnable-manual-${ext.analyticsSafeName}';
 
   /// Event sent when an extension is enabled because a user answered the
   /// enablement prompt with "Enable".
-  static String extensionEnablePrompt(String name) =>
-      'extensionEnable-prompt-$name';
+  static String extensionEnablePrompt(DevToolsExtensionConfig ext) =>
+      'extensionEnable-prompt-${ext.analyticsSafeName}';
 
   /// Event sent when an extension is disabled because a user manually disabled
   /// it from the [DisableExtensionDialog] or the main extensions settings menu.
-  static String extensionDisableManual(String name) =>
-      'extensionDisable-manual-$name';
+  static String extensionDisableManual(DevToolsExtensionConfig ext) =>
+      'extensionDisable-manual-${ext.analyticsSafeName}';
 
   /// Event sent when an extension is disabled because a user answered the
   /// enablement prompt with "No, hide this sceen".
-  static String extensionDisablePrompt(String name) =>
-      'extensionDisable-prompt-$name';
+  static String extensionDisablePrompt(DevToolsExtensionConfig ext) =>
+      'extensionDisable-prompt-${ext.analyticsSafeName}';
 
   /// Event sent when an extension is force reloaded from the extension screen
   /// context menu.
-  static String extensionForceReload(String name) =>
-      'extensionForceReload-$name';
+  static String extensionForceReload(DevToolsExtensionConfig ext) =>
+      'extensionForceReload-${ext.analyticsSafeName}';
 }
diff --git a/packages/devtools_app/lib/src/shared/development_helpers.dart b/packages/devtools_app/lib/src/shared/development_helpers.dart
@@ -63,13 +63,15 @@ final List<DevToolsExtensionConfig> debugExtensions = [
     DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
     DevToolsExtensionConfig.versionKey: '1.0.0',
     DevToolsExtensionConfig.pathKey: '/path/to/foo',
+    DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
   }),
   DevToolsExtensionConfig.parse({
     DevToolsExtensionConfig.nameKey: 'bar',
     DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
     DevToolsExtensionConfig.versionKey: '2.0.0',
     DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
     DevToolsExtensionConfig.pathKey: '/path/to/bar',
+    DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
   }),
   DevToolsExtensionConfig.parse({
     DevToolsExtensionConfig.nameKey: 'provider',
@@ -78,6 +80,7 @@ final List<DevToolsExtensionConfig> debugExtensions = [
     DevToolsExtensionConfig.versionKey: '3.0.0',
     DevToolsExtensionConfig.materialIconCodePointKey: 0xe50a,
     DevToolsExtensionConfig.pathKey: '/path/to/provider',
+    DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
   }),
 ];
 

diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml
@@ -27,7 +27,7 @@ dependencies:
   dds_service_extensions: ^1.6.0
   devtools_app_shared: ^0.0.7
   devtools_extensions: ^0.0.10
-  devtools_shared: ^5.0.0
+  devtools_shared: ^6.0.1
   file: ">=6.0.0 <8.0.0"
   file_selector: ^0.8.0
   file_selector_linux: ^0.0.2

diff --git a/packages/devtools_app/test/extensions/extension_analytics_test.dart b/packages/devtools_app/test/extensions/extension_analytics_test.dart
@@ -0,0 +1,90 @@
+// Copyright 2023 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:devtools_app/src/shared/analytics/constants.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import '../test_infra/test_data/extensions.dart';
+
+void main() {
+  group('DevTools extension analytics', () {
+    test(
+      'uses extension name for public package',
+      () {
+        final public = providerExtension;
+        expect(public.isPubliclyHosted, true);
+        expect(public.name, 'provider');
+        expect(public.analyticsSafeName, 'provider');
+        expect(
+          DevToolsExtensionEvents.extensionScreenName(public),
+          'extension-provider',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionFeedback(public),
+          'extensionFeedback-provider',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionEnableManual(public),
+          'extensionEnable-manual-provider',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionEnablePrompt(public),
+          'extensionEnable-prompt-provider',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionDisableManual(public),
+          'extensionDisable-manual-provider',
+        );
+
+        expect(
+          DevToolsExtensionEvents.extensionDisablePrompt(public),
+          'extensionDisable-prompt-provider',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionForceReload(public),
+          'extensionForceReload-provider',
+        );
+      },
+    );
+
+    test(
+      'does not use extension name for private package',
+      () {
+        final private = fooExtension;
+        expect(private.isPubliclyHosted, false);
+        expect(private.name, 'Foo');
+        expect(private.analyticsSafeName, 'private');
+        expect(
+          DevToolsExtensionEvents.extensionScreenName(private),
+          'extension-private',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionFeedback(private),
+          'extensionFeedback-private',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionEnableManual(private),
+          'extensionEnable-manual-private',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionEnablePrompt(private),
+          'extensionEnable-prompt-private',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionDisableManual(private),
+          'extensionDisable-manual-private',
+        );
+
+        expect(
+          DevToolsExtensionEvents.extensionDisablePrompt(private),
+          'extensionDisable-prompt-private',
+        );
+        expect(
+          DevToolsExtensionEvents.extensionForceReload(private),
+          'extensionForceReload-private',
+        );
+      },
+    );
+  });
+}
diff --git a/packages/devtools_app/test/test_infra/test_data/extensions.dart b/packages/devtools_app/test/test_infra/test_data/extensions.dart
@@ -11,6 +11,7 @@ final fooExtension = DevToolsExtensionConfig.parse({
   DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
   DevToolsExtensionConfig.versionKey: '1.0.0',
   DevToolsExtensionConfig.pathKey: '/path/to/foo',
+  DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
 });
 
 final barExtension = DevToolsExtensionConfig.parse({
@@ -19,6 +20,7 @@ final barExtension = DevToolsExtensionConfig.parse({
   DevToolsExtensionConfig.versionKey: '2.0.0',
   DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
   DevToolsExtensionConfig.pathKey: '/path/to/bar',
+  DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
 });
 
 final providerExtension = DevToolsExtensionConfig.parse({
@@ -28,4 +30,5 @@ final providerExtension = DevToolsExtensionConfig.parse({
   DevToolsExtensionConfig.versionKey: '3.0.0',
   DevToolsExtensionConfig.materialIconCodePointKey: 0xe50a,
   DevToolsExtensionConfig.pathKey: '/path/to/provider',
+  DevToolsExtensionConfig.isPubliclyHostedKey: 'true',
 });
diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md
@@ -2,7 +2,7 @@
 * Soften breaking change from 0.0.6 to add the `profilePlatformChannels` service extension,
 improving backwards compatibility for older versions of Flutter.
 * Bump `package:vm_service` dependency to ^13.0.0.
-* Bump the `package:devtools_shared` dependency to ^6.0.0.
+* Bump the `package:devtools_shared` dependency to ^6.0.1.
 * Remove public getter `libraryRef`, and public methods `getLibrary` and `retrieveFullValueAsString` from `EvalOnDartLibrary`.
 * Change `toString` output for `UnknownEvalException`, `EvalSentinelException`, and `EvalErrorException`.
 * Remove public getters `flutterVersionSummary`, `frameworkVersionSummary`, and `engineVersionSummary` from `FlutterVersion`.

diff --git a/packages/devtools_app_shared/pubspec.yaml b/packages/devtools_app_shared/pubspec.yaml
@@ -9,7 +9,7 @@ environment:
 
 dependencies:
   collection: ^1.15.0
-  devtools_shared: ^6.0.0
+  devtools_shared: ^6.0.1
   flutter:
     sdk: flutter
   logging: ^1.1.1

diff --git a/packages/devtools_extensions/CHANGELOG.md b/packages/devtools_extensions/CHANGELOG.md
@@ -7,7 +7,7 @@ DevTools extension.
 * Add an example `launch.json` file in the `example/foo` directory.
 * Clean up the package readme to make instructions Windows-compatible.
 * Update the README with instructions for joining the Flutter Discord server.
-* Bump `package:devtools_shared` dependency to ^6.0.0
+* Bump `package:devtools_shared` dependency to ^6.0.1
 * Bump `package:devtools_app_shared` dependency to ^0.0.7
 * Bump `package:vm_service` dependency to ^13.0.0.
 

diff --git a/packages/devtools_extensions/pubspec.yaml b/packages/devtools_extensions/pubspec.yaml
@@ -13,7 +13,7 @@ executables:
 
 dependencies:
   args: ^2.4.2
-  devtools_shared: ^6.0.0
+  devtools_shared: ^6.0.1
   devtools_app_shared: ^0.0.7
   flutter:
     sdk: flutter

diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md
@@ -1,3 +1,6 @@
+# 6.0.1
+* Add field `isPublic` to `DevToolsExtensionConfig`.
+
 # 6.0.0
 * Bump `package:vm_service` dependency to ^13.0.0.
 * Remove `ServiceCreator` typedef and replace usages with `VmServiceFactory` typedef from `package:vm_service`.

diff --git a/packages/devtools_shared/lib/src/extensions/extension_manager.dart b/packages/devtools_shared/lib/src/extensions/extension_manager.dart
@@ -78,6 +78,12 @@ class ExtensionsManager {
       }
       for (final extension in extensions) {
         final config = extension.config;
+        // TODO(https://github.com/dart-lang/pub/issues/4042): make this check
+        // more robust.
+        final isPubliclyHosted = (extension.rootUri.path.contains('pub.dev') ||
+                extension.rootUri.path.contains('pub.flutter-io.cn'))
+            .toString();
+
         // This should be relative to the 'extension/devtools/' directory and
         // defaults to 'build';
         final relativeExtensionLocation =
@@ -93,6 +99,7 @@ class ExtensionsManager {
           final extensionConfig = DevToolsExtensionConfig.parse({
             ...config,
             DevToolsExtensionConfig.pathKey: location,
+            DevToolsExtensionConfig.isPubliclyHostedKey: isPubliclyHosted,
           });
           devtoolsExtensions.add(extensionConfig);
         } on StateError catch (e) {