diff --git a/packages/metrics_center/CHANGELOG.md b/packages/metrics_center/CHANGELOG.md index 63cf5593f9..4e629f28af 100644 --- a/packages/metrics_center/CHANGELOG.md +++ b/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 diff --git a/packages/metrics_center/lib/src/common.dart b/packages/metrics_center/lib/src/common.dart index e4864e2d64..ee17c40a2a 100644 --- a/packages/metrics_center/lib/src/common.dart +++ b/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 update(List points); + Future update(List points, DateTime commitTime); } /// Create `AuthClient` in case we only have an access token without the full diff --git a/packages/metrics_center/lib/src/flutter.dart b/packages/metrics_center/lib/src/flutter.dart index af2a8ad02b..89e35ae48f 100644 --- a/packages/metrics_center/lib/src/flutter.dart +++ b/packages/metrics_center/lib/src/flutter.dart @@ -52,8 +52,8 @@ class FlutterDestination extends MetricDestination { } @override - Future update(List points) async { - await _skiaPerfDestination.update(points); + Future update(List points, DateTime commitTime) async { + await _skiaPerfDestination.update(points, commitTime); } final SkiaPerfDestination _skiaPerfDestination; diff --git a/packages/metrics_center/lib/src/github_helper.dart b/packages/metrics_center/lib/src/github_helper.dart deleted file mode 100644 index 9e33b487f3..0000000000 --- a/packages/metrics_center/lib/src/github_helper.dart +++ /dev/null @@ -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 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 _commitDateTimeCache = {}; -} diff --git a/packages/metrics_center/lib/src/skiaperf.dart b/packages/metrics_center/lib/src/skiaperf.dart index b145857a3d..2a9c4bdba8 100644 --- a/packages/metrics_center/lib/src/skiaperf.dart +++ b/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 computeObjectName(String githubRepo, String revision, - {GithubHelper githubHelper}) async { + static Future 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 update(List points) async { + Future update(List 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 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 diff --git a/packages/metrics_center/pubspec.yaml b/packages/metrics_center/pubspec.yaml index 68284e0768..cc8fac0afc 100644 --- a/packages/metrics_center/pubspec.yaml +++ b/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 diff --git a/packages/metrics_center/test/flutter_test.dart b/packages/metrics_center/test/flutter_test.dart index 398516b824..277e400b86 100644 --- a/packages/metrics_center/test/flutter_test.dart +++ b/packages/metrics_center/test/flutter_test.dart @@ -44,6 +44,7 @@ void main() { final FlutterDestination dst = await FlutterDestination.makeFromCredentialsJson(credentialsJson, isTesting: true); - dst.update([simplePoint]); + dst.update([simplePoint], + DateTime.fromMillisecondsSinceEpoch(123)); }, skip: credentialsJson == null); } diff --git a/packages/metrics_center/test/github_helper_test.dart b/packages/metrics_center/test/github_helper_test.dart deleted file mode 100644 index 3d53248b7d..0000000000 --- a/packages/metrics_center/test/github_helper_test.dart +++ /dev/null @@ -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))); - }); -} diff --git a/packages/metrics_center/test/skiaperf_test.dart b/packages/metrics_center/test/skiaperf_test.dart index 41d873709e..d8157115d6 100644 --- a/packages/metrics_center/test/skiaperf_test.dart +++ b/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 protectedRun( @@ -317,35 +314,27 @@ Future main() async { }); test('SkiaPerfGcsAdaptor computes name correctly', () async { - final MockGithubHelper mockHelper = MockGithubHelper(); - when(mockHelper.getCommitDateTime( - kFlutterFrameworkRepo, kFrameworkRevision1)) - .thenAnswer((_) => Future.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.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.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 main() async { final SkiaPerfGcsAdaptor skiaPerfGcs = SkiaPerfGcsAdaptor(testBucket); final String testObjectName = await SkiaPerfGcsAdaptor.computeObjectName( - kFlutterFrameworkRepo, kFrameworkRevision1); + kFlutterFrameworkRepo, + kFrameworkRevision1, + DateTime.fromMillisecondsSinceEpoch(123)); final List writePoints = [ SkiaPerfPoint.fromPoint(cocoonPointRev1Metric1), @@ -393,7 +384,9 @@ Future 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 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.fromPoint(cocoonPointRev1Metric1), @@ -452,7 +447,9 @@ Future 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.fromPoint(enginePoint1), @@ -505,18 +502,27 @@ Future 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 main() async { final SkiaPerfGcsAdaptor mockGcs = MockSkiaPerfGcsAdaptor(); final GcsLock mockLock = MockGcsLock(); final SkiaPerfDestination dst = SkiaPerfDestination(mockGcs, mockLock); - await dst.update([cocoonPointRev1Metric1]); - await dst.update([cocoonPointRev1Metric2]); + await dst.update([cocoonPointRev1Metric1], + DateTime.fromMillisecondsSinceEpoch(123)); + await dst.update([cocoonPointRev1Metric2], + DateTime.fromMillisecondsSinceEpoch(123)); List 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), [kTaskName]); @@ -543,18 +551,19 @@ Future main() async { final MetricPoint updated = MetricPoint(kValue3, cocoonPointRev1Metric1.tags); - await dst.update([updated, cocoonPointRev2Metric1]); + await dst.update([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), [kValue2, kValue3]); }); @@ -562,7 +571,8 @@ Future main() async { Future skiaPerfDestinationIntegrationTest() async { final SkiaPerfDestination destination = SkiaPerfDestination(SkiaPerfGcsAdaptor(testBucket), testLock); - await destination.update([cocoonPointRev1Metric1]); + await destination.update([cocoonPointRev1Metric1], + DateTime.fromMillisecondsSinceEpoch(123)); } test(