Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/metrics_center/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/metrics_center/lib/src/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/metrics_center/lib/src/flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
35 changes: 0 additions & 35 deletions packages/metrics_center/lib/src/github_helper.dart

This file was deleted.

21 changes: 9 additions & 12 deletions packages/metrics_center/lib/src/skiaperf.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -317,16 +316,14 @@ 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 String month = commitTime.month.toString().padLeft(2, '0');
final String day = commitTime.day.toString().padLeft(2, '0');
final String hour = commitTime.hour.toString().padLeft(2, '0');
final String dateComponents = '${commitTime.year}/$month/$day/$hour';
return '$topComponent/$dateComponents/$revision/values.json';
}

Expand Down Expand Up @@ -392,7 +389,7 @@ class SkiaPerfDestination extends MetricDestination {
}

@override
Future<void> update(List<MetricPoint> points) async {
Future<void> update(List<MetricPoint> points, DateTime commitTime) async {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing as I should be using a Utc datetime instead of the local time

// 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.
Expand All @@ -410,8 +407,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
Expand Down
4 changes: 1 addition & 3 deletions packages/metrics_center/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion packages/metrics_center/test/flutter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
41 changes: 0 additions & 41 deletions packages/metrics_center/test/github_helper_test.dart

This file was deleted.

72 changes: 41 additions & 31 deletions packages/metrics_center/test/skiaperf_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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(
Expand Down Expand Up @@ -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.fromMillisecondsSinceEpoch(1575529660000),
),
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.fromMillisecondsSinceEpoch(1575432460000),
),
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.fromMillisecondsSinceEpoch(1578092860000),
),
equals('flutter-engine/2020/01/03/15/$kEngineRevision2/values.json'),
);
Expand All @@ -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),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -505,18 +502,27 @@ 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'),
);
expect(
await SkiaPerfGcsAdaptor.computeObjectName(
kFlutterEngineRepo, kEngineRevision1),
kFlutterEngineRepo,
kEngineRevision1,
DateTime.fromMillisecondsSinceEpoch(1575432460000),
),
equals('flutter-engine/2019/12/03/20/$kEngineRevision1/values.json'),
);
expect(
await SkiaPerfGcsAdaptor.computeObjectName(
kFlutterEngineRepo, kEngineRevision2),
kFlutterEngineRepo,
kEngineRevision2,
DateTime.fromMillisecondsSinceEpoch(1578092860000),
),
equals('flutter-engine/2020/01/03/15/$kEngineRevision2/values.json'),
);
},
Expand All @@ -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]);
Expand All @@ -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(
Expand Down