Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 2 additions & 9 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,9 @@ task:
ninja -C out/host_release
benchmark_host_script: |
cd $ENGINE_PATH/src/out/host_release/
./txt_benchmarks --benchmark_format=json > txt_benchmarks.json
./fml_benchmarks --benchmark_format=json > fml_benchmarks.json
./shell_benchmarks --benchmark_format=json > shell_benchmarks.json
./ui_benchmarks --benchmark_format=json > ui_benchmarks.json
$ENGINE_PATH/src/flutter/testing/benchmark/generate_metrics.sh
cd $ENGINE_PATH/src/flutter/testing/benchmark
pub get
dart bin/parse_and_send.dart ../../../out/host_release/txt_benchmarks.json
dart bin/parse_and_send.dart ../../../out/host_release/fml_benchmarks.json
dart bin/parse_and_send.dart ../../../out/host_release/shell_benchmarks.json
dart bin/parse_and_send.dart ../../../out/host_release/ui_benchmarks.json
upload_metrics.sh

# The following test depends on Flutter framework repo. It may fail if the
# framework repo is currently broken.
Expand Down
29 changes: 22 additions & 7 deletions testing/benchmark/bin/parse_and_send.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,33 @@ Future<List<FlutterEngineMetricPoint>> _parse(String jsonFileName) async {
return points;
}

Future<FlutterDestination> connectFlutterDestination() async {
const String kTokenPath = 'TOKEN_PATH';
const String kGcpProject = 'GCP_PROJECT';
final Map<String, String> env = Platform.environment;
if (env.containsKey(kTokenPath) && env.containsKey(kGcpProject)) {
return FlutterDestination.makeFromAccessToken(
File(env[kTokenPath]).readAsStringSync(),
env[kGcpProject],
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

@liyuqian liyuqian Oct 27, 2020

Choose a reason for hiding this comment

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

Technically, no as we default the project id to be flutter-cirrus if it's not provided.

However, to avoid any confusion, I felt it's better to set the token and GCP project id in one place because one might need to change the token when the project is changed (or vice versa). The recipe example you gave me also set the token and the project id in one place.

I'm Ok either way. If there's a difficulty in giving the GCP project id in the environment variables, or if you'd like to omit it, I can easily remove it from this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(The current PR does require env.containsKey(kGcpProject) to be true though. I can remove it so it's not needed.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I advanced a little bit but the authenticated client is rejecting the token: https://chromium-swarm.appspot.com/task?id=4f8147477ee94610

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I need to add LUCI's service account to flutter-cirrus project's access list. Do you have the id of that service account, or can you put a link to your recipe so maybe I can look it up?

Copy link
Contributor

Choose a reason for hiding this comment

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

The service account is "flutter-try-builder@chops-service-accounts.iam.gserviceaccount.com" I tried adding it yesterday but maybe I didn't granted it the correct permissions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's strange. "Cloud Datastore User" should be the correct role... Is there someway to verify that the generated token belongs to that service account (e.g. write a script that takes an access token and use some APIs to print out the id)?

Copy link
Contributor

@godofredoc godofredoc Oct 27, 2020

Choose a reason for hiding this comment

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

The easiest way will be to generate a token using gcloud auth application-default print-access-token and passing it directly to the tool to see if the tool is processing the token in the expected way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gcloud auth application-default print-access-token seems to require a credentials json of the service account to print the access token? If so, I've already done a similar test by generating the token from the credentials and send it to the script: https://github.com/flutter/engine/pull/22019/files#diff-f2e2f624ef84bb33cc75f9c5a09e1c0f9371eb06d5b73ce94fd65ca6ee052b92R40

);
}
return await FlutterDestination.makeFromCredentialsJson(
jsonDecode(Platform.environment['BENCHMARK_GCP_CREDENTIALS'])
as Map<String, dynamic>,
);
}

Future<void> main(List<String> args) async {
if (args.length != 1) {
throw 'Must have one argument: <benchmark_json_file>';
}
final List<FlutterEngineMetricPoint> points = await _parse(args[0]);

// The data will be sent to the Datastore of the GCP project specified through
// environment variable BENCHMARK_GCP_CREDENTIALS. The engine Cirrus job has
// currently configured the GCP project to flutter-cirrus for test. We'll
// eventually migrate to flutter-infra project once the test is done.
final FlutterDestination destination =
await FlutterDestination.makeFromCredentialsJson(
jsonDecode(Platform.environment['BENCHMARK_GCP_CREDENTIALS']),
);
// environment variable BENCHMARK_GCP_CREDENTIALS, or TOKEN_PATH/GCP_PROJECT.
// The engine Cirrus job has currently configured the GCP project to
// flutter-cirrus for test. We'll eventually migrate to flutter-infra project
// once the test is done.
final FlutterDestination destination = await connectFlutterDestination();
await destination.update(points);
}
27 changes: 26 additions & 1 deletion testing/benchmark/test/parse_and_send_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
// found in the LICENSE file.

// @dart = 2.6
import 'dart:convert';
import 'dart:io';

import 'package:gcloud/src/datastore_impl.dart';
import 'package:googleapis_auth/auth_io.dart';
import 'package:path/path.dart' as path;
import 'package:test/test.dart';

void main() {
// In order to run this test, one should download a service account
// In order to run these tests, one should download a service account
// credentials json from a test GCP project, and put that json as
// `secret/test_gcp_credentials.json`. There's a `flutter-test` project for
// Flutter team members.
Expand All @@ -22,4 +26,25 @@ void main() {
'BENCHMARK_GCP_CREDENTIALS': testCred,
});
});

test('parse_and_send succeeds with access token.', () async {
final dynamic testCred =
jsonDecode(File('secret/test_gcp_credentials.json').readAsStringSync())
as Map<String, dynamic>;
final AutoRefreshingAuthClient client = await clientViaServiceAccount(
ServiceAccountCredentials.fromJson(testCred),
DatastoreImpl.SCOPES,
);
final String tokenPath =
path.join(Directory.systemTemp.absolute.path, 'parse_and_send_token');
File(tokenPath).writeAsStringSync(client.credentials.accessToken.data);
final ProcessResult result = Process.runSync('dart', <String>[
'bin/parse_and_send.dart',
'example/txt_benchmarks.json',
], environment: <String, String>{
'TOKEN_PATH': tokenPath,
'GCP_PROJECT': testCred['project_id'] as String,
});
expect(result.exitCode, 0);
});
}