review comments

Author: kenzieschmoll

pkgs/unified_analytics/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 7.0.0
 - Added a required parameter `screen` to the `Event.devtoolsEvent` constructor.
 - Added an optional parameter `additionalMetrics` to the `Event.devtoolsEvent` constructor.
+- Added `CustomMetrics` class for unified_analytics clients to define custom event metrics.
 - Removed parameters `uiDurationMicros`, `rasterDurationMicros`, `shaderCompilationDurationMicros`,
 `traceEventCount`, `cpuSampleCount`, `cpuStackDepth`, `heapDiffObjectsBefore`, `heapDiffObjectsAfter`,
 `heapObjectsTotal`, `rootSetCount`, `rowCount`, `inspectorTreeControllerId`, `androidAppId`, `iosBundleId`

pkgs/unified_analytics/lib/src/event.dart

@@ -358,10 +358,10 @@ final class Event {
 
   /// Event that is sent from DevTools for various different actions as
   /// indicated by the [eventCategory].
-  /// 
+  ///
   /// The optional parameters in the parameter list contain metadata that is
   /// sent with each event, when available.
-  /// 
+  ///
   /// [additionalMetrics] may contain any additional data for the event being
   /// sent. This often looks like metrics that are unique to the event or to a
   /// specific screen.
@@ -387,7 +387,7 @@ final class Event {
     String? isEmbedded,
     String? ideLaunchedFeature,
     String? isWasm,
-    Map<String, Object?> additionalMetrics = const {},
+    CustomMetrics? additionalMetrics,
   })  : eventName = DashEvent.devtoolsEvent,
         eventData = {
           'screen': screen,
@@ -411,7 +411,9 @@ final class Event {
           if (ideLaunchedFeature != null)
             'ideLaunchedFeature': ideLaunchedFeature,
           if (isWasm != null) 'isWasm': isWasm,
-          ...additionalMetrics..removeWhere((key, value) => value == null),
+          if (additionalMetrics != null)
+            ...additionalMetrics.toMap()
+              ..removeWhere((key, value) => value == null),
         };
 
   /// Event that contains the results for a specific doctor validator.
@@ -846,3 +848,15 @@ final class Event {
     }
   }
 }
+
+/// A base class for custom metrics that will be defined by a unified_analytics
+/// client.
+///
+/// This base type can be used as a parameter in any event constructor that
+/// allows custom metrics to be added by a unified_analytics client.
+abstract class CustomMetrics {
+  /// Converts the custom metrics data to a [Map] object.
+  /// 
+  /// This must be a JSON encodable [Map].
+  Map<String, Object?> toMap();
+}

pkgs/unified_analytics/test/event_test.dart

@@ -6,6 +6,7 @@ import 'dart:mirrors';
 
 import 'package:test/test.dart';
 import 'package:unified_analytics/src/enums.dart';
+import 'package:unified_analytics/src/event.dart';
 import 'package:unified_analytics/unified_analytics.dart';
 
 void main() {
@@ -576,11 +577,14 @@ void main() {
           isEmbedded: 'isEmbedded',
           ideLaunchedFeature: 'ideLaunchedFeature',
           isWasm: 'true',
-          additionalMetrics: {
-            'someMetric': 100,
-            'otherMetric': false,
-            'shouldBeRemoved': null,
-          },
+          additionalMetrics: _TestMetrics(
+            stringField: 'test',
+            // Since this value is null, it should not be included in the event
+            // JSON below.
+            nullableField: null,
+            intField: 100,
+            boolField: false,
+          ),
         );
 
     final constructedEvent = generateEvent();
@@ -606,9 +610,11 @@ void main() {
       'ideLaunchedFeature',
     );
     expect(constructedEvent.eventData['isWasm'], 'true');
-    expect(constructedEvent.eventData['someMetric'], 100);
-    expect(constructedEvent.eventData['otherMetric'], false);
-    expect(constructedEvent.eventData.length, 19);
+    expect(constructedEvent.eventData['stringField'], 'test');
+    expect(constructedEvent.eventData['intField'], 100);
+    expect(constructedEvent.eventData['boolField'], false);
+    expect(constructedEvent.eventData.containsKey('nullableField'), false);
+    expect(constructedEvent.eventData.length, 20);
   });
 
   test('Confirm all constructors were checked', () {
@@ -682,3 +688,25 @@ void main() {
     expect(eventConstructed, isNull);
   });
 }
+
+class _TestMetrics extends CustomMetrics {
+  _TestMetrics({
+    required this.stringField,
+    required this.nullableField,
+    required this.intField,
+    required this.boolField,
+  });
+
+  final String stringField;
+  final String? nullableField;
+  final int intField;
+  final bool boolField;
+
+  @override
+  Map<String, Object?> toMap() => {
+        'stringField': stringField,
+        'nullableField': nullableField,
+        'intField': intField,
+        'boolField': boolField,
+      };
+}