[metrics_center] Remove github dependency

packages/metrics_center/CHANGELOG.md

@@ -1,13 +1,22 @@
+# 0.1.0
+
+- `update` now requires DateTime when commit was merged
+- Removed `github` dependency
+
 # 0.0.9
+
 - Remove legacy datastore and destination.
 
 # 0.0.8
+
 - Allow tests to override LegacyFlutterDestination GCP project id.
 
 # 0.0.7
+
 - Expose constants that were missing since 0.0.4+1.
 
 # 0.0.6
+
 - Allow `datastoreFromCredentialsJson` to specify project id.
 
 # 0.0.5

packages/metrics_center/lib/src/common.dart

@@ -50,7 +50,7 @@ class MetricPoint extends Equatable {
 /// Interface to write [MetricPoint].
 abstract class MetricDestination {
   /// Insert new data points or modify old ones with matching id.
-  Future<void> update(List<MetricPoint> points);
+  Future<void> update(List<MetricPoint> points, DateTime commitTime);
 }
 
 /// Create `AuthClient` in case we only have an access token without the full

packages/metrics_center/lib/src/flutter.dart

@@ -52,8 +52,8 @@ class FlutterDestination extends MetricDestination {
   }
 
   @override
-  Future<void> update(List<MetricPoint> points) async {
-    await _skiaPerfDestination.update(points);
+  Future<void> update(List<MetricPoint> points, DateTime commitTime) async {
+    await _skiaPerfDestination.update(points, commitTime);
   }
 
   final SkiaPerfDestination _skiaPerfDestination;

packages/metrics_center/lib/src/skiaperf.dart

@@ -12,7 +12,6 @@ import 'package:googleapis_auth/auth_io.dart';
 import 'common.dart';
 import 'constants.dart';
 import 'gcs_lock.dart';
-import 'github_helper.dart';
 
 /// A [MetricPoint] modeled after the format that Skia Perf expects.
 ///
@@ -317,16 +316,15 @@ class SkiaPerfGcsAdaptor {
   /// json files can be put in that leaf directory. We intend to use multiple
   /// json files in the future to scale up the system if too many writes are
   /// competing for the same json file.
-  static Future<String> computeObjectName(String githubRepo, String revision,
-      {GithubHelper githubHelper}) async {
+  static Future<String> computeObjectName(
+      String githubRepo, String revision, DateTime commitTime) async {
     assert(_githubRepoToGcsName[githubRepo] != null);
     final String topComponent = _githubRepoToGcsName[githubRepo];
-    final DateTime t = await (githubHelper ?? GithubHelper())
-        .getCommitDateTime(githubRepo, revision);
-    final String month = t.month.toString().padLeft(2, '0');
-    final String day = t.day.toString().padLeft(2, '0');
-    final String hour = t.hour.toString().padLeft(2, '0');
-    final String dateComponents = '${t.year}/$month/$day/$hour';
+    final DateTime commitTimeUtc = commitTime.toUtc();
+    final String month = commitTimeUtc.month.toString().padLeft(2, '0');
+    final String day = commitTimeUtc.day.toString().padLeft(2, '0');
+    final String hour = commitTimeUtc.hour.toString().padLeft(2, '0');
+    final String dateComponents = '${commitTime.year}/$month/$day/$hour';
     return '$topComponent/$dateComponents/$revision/values.json';
   }
 
@@ -392,7 +390,7 @@ class SkiaPerfDestination extends MetricDestination {
   }
 
   @override
-  Future<void> update(List<MetricPoint> points) async {
+  Future<void> update(List<MetricPoint> points, DateTime commitTime) async {
     // 1st, create a map based on git repo, git revision, and point id. Git repo
     // and git revision are the top level components of the Skia perf GCS object
     // name.
@@ -410,8 +408,8 @@ class SkiaPerfDestination extends MetricDestination {
     // 2nd, read existing points from the gcs object and update with new ones.
     for (final String repo in pointMap.keys) {
       for (final String revision in pointMap[repo].keys) {
-        final String objectName =
-            await SkiaPerfGcsAdaptor.computeObjectName(repo, revision);
+        final String objectName = await SkiaPerfGcsAdaptor.computeObjectName(
+            repo, revision, commitTime);
         final Map<String, SkiaPerfPoint> newPoints = pointMap[repo][revision];
         // If too many bots are writing the metrics of a git revision into this
         // single json file (with name `objectName`), the contention on the lock

packages/metrics_center/pubspec.yaml

@@ -1,5 +1,5 @@
 name: metrics_center
-version: 0.0.9
+version: 0.1.0
 description:
   Support multiple performance metrics sources/formats and destinations.
 homepage:
@@ -9,11 +9,9 @@ environment:
   sdk: '>=2.10.0 <3.0.0'
 
 dependencies:
-  args: ^1.6.0
   crypto: ^2.1.5
   equatable: ^1.2.5
   gcloud: ^0.7.3
-  github: ^7.0.4
   googleapis: ^0.56.1
   googleapis_auth: ^0.2.12
   http: ^0.12.2

packages/metrics_center/test/flutter_test.dart

@@ -44,6 +44,7 @@ void main() {
     final FlutterDestination dst =
         await FlutterDestination.makeFromCredentialsJson(credentialsJson,
             isTesting: true);
-    dst.update(<FlutterEngineMetricPoint>[simplePoint]);
+    dst.update(<FlutterEngineMetricPoint>[simplePoint],
+        DateTime.fromMillisecondsSinceEpoch(123));
   }, skip: credentialsJson == null);
 }

packages/metrics_center/test/skiaperf_test.dart

@@ -13,7 +13,6 @@ import 'package:metrics_center/metrics_center.dart';
 import 'package:mockito/mockito.dart';
 
 import 'package:metrics_center/src/constants.dart';
-import 'package:metrics_center/src/github_helper.dart';
 import 'package:metrics_center/src/gcs_lock.dart';
 
 import 'common.dart';
@@ -23,8 +22,6 @@ class MockBucket extends Mock implements Bucket {}
 
 class MockObjectInfo extends Mock implements ObjectInfo {}
 
-class MockGithubHelper extends Mock implements GithubHelper {}
-
 class MockGcsLock implements GcsLock {
   @override
   Future<void> protectedRun(
@@ -317,37 +314,29 @@ Future<void> main() async {
   });
 
   test('SkiaPerfGcsAdaptor computes name correctly', () async {
-    final MockGithubHelper mockHelper = MockGithubHelper();
-    when(mockHelper.getCommitDateTime(
-            kFlutterFrameworkRepo, kFrameworkRevision1))
-        .thenAnswer((_) => Future<DateTime>.value(DateTime(2019, 12, 4, 23)));
     expect(
       await SkiaPerfGcsAdaptor.computeObjectName(
         kFlutterFrameworkRepo,
         kFrameworkRevision1,
-        githubHelper: mockHelper,
+        DateTime.fromMillisecondsSinceEpoch(1575529660000),
       ),
-      equals('flutter-flutter/2019/12/04/23/$kFrameworkRevision1/values.json'),
+      equals('flutter-flutter/2019/12/05/07/$kFrameworkRevision1/values.json'),
     );
-    when(mockHelper.getCommitDateTime(kFlutterEngineRepo, kEngineRevision1))
-        .thenAnswer((_) => Future<DateTime>.value(DateTime(2019, 12, 3, 20)));
     expect(
       await SkiaPerfGcsAdaptor.computeObjectName(
         kFlutterEngineRepo,
         kEngineRevision1,
-        githubHelper: mockHelper,
+        DateTime.fromMillisecondsSinceEpoch(1575432460000),
       ),
-      equals('flutter-engine/2019/12/03/20/$kEngineRevision1/values.json'),
+      equals('flutter-engine/2019/12/04/04/$kEngineRevision1/values.json'),
     );
-    when(mockHelper.getCommitDateTime(kFlutterEngineRepo, kEngineRevision2))
-        .thenAnswer((_) => Future<DateTime>.value(DateTime(2020, 1, 3, 15)));
     expect(
       await SkiaPerfGcsAdaptor.computeObjectName(
         kFlutterEngineRepo,
         kEngineRevision2,
-        githubHelper: mockHelper,
+        DateTime.fromMillisecondsSinceEpoch(1578092860000),
       ),
-      equals('flutter-engine/2020/01/03/15/$kEngineRevision2/values.json'),
+      equals('flutter-engine/2020/01/03/23/$kEngineRevision2/values.json'),
     );
   });
 
@@ -356,7 +345,9 @@ Future<void> main() async {
     final SkiaPerfGcsAdaptor skiaPerfGcs = SkiaPerfGcsAdaptor(testBucket);
 
     final String testObjectName = await SkiaPerfGcsAdaptor.computeObjectName(
-        kFlutterFrameworkRepo, kFrameworkRevision1);
+        kFlutterFrameworkRepo,
+        kFrameworkRevision1,
+        DateTime.fromMillisecondsSinceEpoch(123));
 
     final List<SkiaPerfPoint> writePoints = <SkiaPerfPoint>[
       SkiaPerfPoint.fromPoint(cocoonPointRev1Metric1),
@@ -393,7 +384,9 @@ Future<void> main() async {
     final MockBucket testBucket = MockBucket();
     final SkiaPerfGcsAdaptor skiaPerfGcs = SkiaPerfGcsAdaptor(testBucket);
     final String testObjectName = await SkiaPerfGcsAdaptor.computeObjectName(
-        kFlutterFrameworkRepo, kFrameworkRevision1);
+        kFlutterFrameworkRepo,
+        kFrameworkRevision1,
+        DateTime.fromMillisecondsSinceEpoch(123));
     when(testBucket.info(testObjectName))
         .thenThrow(Exception('No such object'));
     expect((await skiaPerfGcs.readPoints(testObjectName)).length, 0);
@@ -423,7 +416,9 @@ Future<void> main() async {
     final SkiaPerfGcsAdaptor skiaPerfGcs = SkiaPerfGcsAdaptor(testBucket);
 
     final String testObjectName = await SkiaPerfGcsAdaptor.computeObjectName(
-        kFlutterFrameworkRepo, kFrameworkRevision1);
+        kFlutterFrameworkRepo,
+        kFrameworkRevision1,
+        DateTime.fromMillisecondsSinceEpoch(123));
 
     await skiaPerfGcs.writePoints(testObjectName, <SkiaPerfPoint>[
       SkiaPerfPoint.fromPoint(cocoonPointRev1Metric1),
@@ -452,7 +447,9 @@ Future<void> main() async {
     final SkiaPerfGcsAdaptor skiaPerfGcs = SkiaPerfGcsAdaptor(testBucket);
 
     final String testObjectName = await SkiaPerfGcsAdaptor.computeObjectName(
-        kFlutterEngineRepo, engineRevision);
+        kFlutterEngineRepo,
+        engineRevision,
+        DateTime.fromMillisecondsSinceEpoch(123));
 
     await skiaPerfGcs.writePoints(testObjectName, <SkiaPerfPoint>[
       SkiaPerfPoint.fromPoint(enginePoint1),
@@ -505,19 +502,28 @@ Future<void> main() async {
     () async {
       expect(
         await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterFrameworkRepo, kFrameworkRevision1),
+          kFlutterFrameworkRepo,
+          kFrameworkRevision1,
+          DateTime.fromMillisecondsSinceEpoch(1575529660000),
+        ),
         equals(
-            'flutter-flutter/2019/12/04/23/$kFrameworkRevision1/values.json'),
+            'flutter-flutter/2019/12/05/07/$kFrameworkRevision1/values.json'),
       );
       expect(
         await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterEngineRepo, kEngineRevision1),
-        equals('flutter-engine/2019/12/03/20/$kEngineRevision1/values.json'),
+          kFlutterEngineRepo,
+          kEngineRevision1,
+          DateTime.fromMillisecondsSinceEpoch(1575432460000),
+        ),
+        equals('flutter-engine/2019/12/04/04/$kEngineRevision1/values.json'),
       );
       expect(
         await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterEngineRepo, kEngineRevision2),
-        equals('flutter-engine/2020/01/03/15/$kEngineRevision2/values.json'),
+          kFlutterEngineRepo,
+          kEngineRevision2,
+          DateTime.fromMillisecondsSinceEpoch(1578092860000),
+        ),
+        equals('flutter-engine/2020/01/03/23/$kEngineRevision2/values.json'),
       );
     },
     skip: testBucket == null,
@@ -527,11 +533,13 @@ Future<void> main() async {
     final SkiaPerfGcsAdaptor mockGcs = MockSkiaPerfGcsAdaptor();
     final GcsLock mockLock = MockGcsLock();
     final SkiaPerfDestination dst = SkiaPerfDestination(mockGcs, mockLock);
-    await dst.update(<MetricPoint>[cocoonPointRev1Metric1]);
-    await dst.update(<MetricPoint>[cocoonPointRev1Metric2]);
+    await dst.update(<MetricPoint>[cocoonPointRev1Metric1],
+        DateTime.fromMillisecondsSinceEpoch(123));
+    await dst.update(<MetricPoint>[cocoonPointRev1Metric2],
+        DateTime.fromMillisecondsSinceEpoch(123));
     List<SkiaPerfPoint> points = await mockGcs.readPoints(
-        await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterFrameworkRepo, kFrameworkRevision1));
+        await SkiaPerfGcsAdaptor.computeObjectName(kFlutterFrameworkRepo,
+            kFrameworkRevision1, DateTime.fromMillisecondsSinceEpoch(123)));
     expect(points.length, equals(2));
     expectSetMatch(
         points.map((SkiaPerfPoint p) => p.testName), <String>[kTaskName]);
@@ -543,26 +551,28 @@ Future<void> main() async {
     final MetricPoint updated =
         MetricPoint(kValue3, cocoonPointRev1Metric1.tags);
 
-    await dst.update(<MetricPoint>[updated, cocoonPointRev2Metric1]);
+    await dst.update(<MetricPoint>[updated, cocoonPointRev2Metric1],
+        DateTime.fromMillisecondsSinceEpoch(123));
 
     points = await mockGcs.readPoints(
-        await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterFrameworkRepo, kFrameworkRevision2));
+        await SkiaPerfGcsAdaptor.computeObjectName(kFlutterFrameworkRepo,
+            kFrameworkRevision2, DateTime.fromMillisecondsSinceEpoch(123)));
     expect(points.length, equals(1));
     expect(points[0].gitHash, equals(kFrameworkRevision2));
     expect(points[0].value, equals(kValue3));
 
     points = await mockGcs.readPoints(
-        await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterFrameworkRepo, kFrameworkRevision1));
+        await SkiaPerfGcsAdaptor.computeObjectName(kFlutterFrameworkRepo,
+            kFrameworkRevision1, DateTime.fromMillisecondsSinceEpoch(123)));
     expectSetMatch(
         points.map((SkiaPerfPoint p) => p.value), <double>[kValue2, kValue3]);
   });
 
   Future<void> skiaPerfDestinationIntegrationTest() async {
     final SkiaPerfDestination destination =
         SkiaPerfDestination(SkiaPerfGcsAdaptor(testBucket), testLock);
-    await destination.update(<MetricPoint>[cocoonPointRev1Metric1]);
+    await destination.update(<MetricPoint>[cocoonPointRev1Metric1],
+        DateTime.fromMillisecondsSinceEpoch(123));
   }
 
   test(