[metrics_center] Remove github dependency (#323)

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/github_helper.dart

@@ -1,35 +0,0 @@
-// Copyright 2014 The Flutter 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:github/github.dart';
-
-/// Singleton class to query some Github info with an in-memory cache.
-class GithubHelper {
-  /// Return the singleton helper.
-  factory GithubHelper() {
-    return _singleton;
-  }
-
-  GithubHelper._internal();
-
-  /// The result is cached in memory so querying the same thing again in the
-  /// same process is fast.
-  ///
-  /// Our unit test requires that calling this method 1000 times for the same
-  /// `githubRepo` and `sha` should be done in 1 second.
-  Future<DateTime> getCommitDateTime(String githubRepo, String sha) async {
-    final String key = '$githubRepo/commit/$sha';
-    if (_commitDateTimeCache[key] == null) {
-      final RepositoryCommit commit = await _github.repositories
-          .getCommit(RepositorySlug.full(githubRepo), sha);
-      _commitDateTimeCache[key] = commit.commit.committer.date;
-    }
-    return _commitDateTimeCache[key];
-  }
-
-  static final GithubHelper _singleton = GithubHelper._internal();
-
-  final GitHub _github = GitHub(auth: findAuthenticationFromEnvironment());
-  final Map<String, DateTime> _commitDateTimeCache = <String, DateTime>{};
-}

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,17 @@ 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';
+    // [commitTime] is not guranteed to be UTC. Ensure it is so all results
+    // pushed to GCS are the same timezone.
+    final DateTime commitUtcTime = commitTime.toUtc();
+    final String month = commitUtcTime.month.toString().padLeft(2, '0');
+    final String day = commitUtcTime.day.toString().padLeft(2, '0');
+    final String hour = commitUtcTime.hour.toString().padLeft(2, '0');
+    final String dateComponents = '${commitUtcTime.year}/$month/$day/$hour';
     return '$topComponent/$dateComponents/$revision/values.json';
   }
 
@@ -392,7 +392,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 +410,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/github_helper_test.dart

@@ -1,41 +0,0 @@
-// Copyright 2014 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-@Timeout(Duration(seconds: 3600))
-
-import 'package:metrics_center/src/github_helper.dart';
-
-import 'common.dart';
-
-void main() {
-  test('GithubHelper gets correct commit date time', () async {
-    final GithubHelper helper = GithubHelper();
-    expect(
-      await helper.getCommitDateTime(
-        'flutter/flutter',
-        'ad20d368ffa09559754e4b2b5c12951341ca3b2d',
-      ),
-      equals(DateTime.parse('2019-12-06 03:33:01.000Z')),
-    );
-  });
-
-  test('GithubHelper is a singleton', () {
-    final GithubHelper helper1 = GithubHelper();
-    final GithubHelper helper2 = GithubHelper();
-    expect(helper1, equals(helper2));
-  });
-
-  test('GithubHelper can query the same commit 1000 times within 1 second',
-      () async {
-    final DateTime start = DateTime.now();
-    for (int i = 0; i < 1000; i += 1) {
-      await GithubHelper().getCommitDateTime(
-        'flutter/flutter',
-        'ad20d368ffa09559754e4b2b5c12951341ca3b2d',
-      );
-    }
-    final Duration duration = DateTime.now().difference(start);
-    expect(duration, lessThan(const Duration(seconds: 1)));
-  });
-}

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,35 +314,27 @@ 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.utc(2019, 12, 04, 23),
       ),
       equals('flutter-flutter/2019/12/04/23/$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.utc(2019, 12, 03, 20),
       ),
       equals('flutter-engine/2019/12/03/20/$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.utc(2020, 01, 03, 15),
       ),
       equals('flutter-engine/2020/01/03/15/$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,18 +502,27 @@ Future<void> main() async {
     () async {
       expect(
         await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterFrameworkRepo, kFrameworkRevision1),
+          kFlutterFrameworkRepo,
+          kFrameworkRevision1,
+          DateTime.utc(2019, 12, 04, 23),
+        ),
         equals(
             'flutter-flutter/2019/12/04/23/$kFrameworkRevision1/values.json'),
       );
       expect(
         await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterEngineRepo, kEngineRevision1),
+          kFlutterEngineRepo,
+          kEngineRevision1,
+          DateTime.utc(2019, 12, 03, 20),
+        ),
         equals('flutter-engine/2019/12/03/20/$kEngineRevision1/values.json'),
       );
       expect(
         await SkiaPerfGcsAdaptor.computeObjectName(
-            kFlutterEngineRepo, kEngineRevision2),
+          kFlutterEngineRepo,
+          kEngineRevision2,
+          DateTime.utc(2020, 01, 03, 15),
+        ),
         equals('flutter-engine/2020/01/03/15/$kEngineRevision2/values.json'),
       );
     },
@@ -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,18 +551,19 @@ 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]);
   });
@@ -562,7 +571,8 @@ Future<void> main() async {
   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(