[Playground] GetMetadata() endpoint #25610

TSultanov · 2023-02-23T15:14:14Z

Backend

Add new endpoint to the Playground runners which returns metadata. Returned metadata includes:

Type of Beam SDK (Java, Go, Python, Scio)
Version of Beam SDK
Commit from which the runner was built
Timestamp of the commit

Frontend

Display frontend version
Display backend version information

resolves #22271
resolves #22542
resolves #21925
resolves #24200

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

playground/api/v1/api.proto

alexeyinkin · 2023-02-23T15:27:08Z

playground/backend/containers/git-functions.gradle

+ext.getGitCommitHash = () -> {
+   def stdout = new ByteArrayOutputStream()
+   exec {
+      executable 'git'


I think we should use Kotlin for new endeavors. Or at least simplify possible migration in the future with

Suggested change

executable 'git'

executable("git")

I haven't found any good ways how to import functions defined in Kotlin DSL into Gradle Groovy. If you have suggestions, I'll be glad to hear them

playground/backend/containers/java/build.gradle

…ol changes

TSultanov · 2023-03-01T15:15:50Z

playground/frontend/playground_components/tools/generate_build_metadata.sh

+#  under the License.
+
+# Run this from the playground_components project root
+FILE='lib/src/build_metadata.g.dart'


@alexeyinkin Can you also add this file to .gitignore so if user would build the project it wouldn't pollute git status?

We have this file in the repository so that the project is immediately runnable after checkout. So adding it to .gitignore has no effect. Unfortunately, it will always pollute git status.

An alternative would be to write these data to a git-ignored .txt file and to download when the frontend starts. The cost is +1 request on each page load. I would rather not do it.

TSultanov · 2023-03-01T15:16:28Z

playground/frontend/playground_components/tools/generate_build_metadata.sh

+git rev-parse --sq HEAD >> $FILE
+echo ';' >> $FILE
+
+echo -n 'const buildCommitSecondsSinceEpoch = ' >> $FILE


use printf instead of echo -n

TSultanov · 2023-03-01T15:19:03Z

playground/frontend/playground_components/tools/generate_build_metadata.sh

+# Run this from the playground_components project root
+FILE='lib/src/build_metadata.g.dart'
+
+echo '// GENERATED CODE - DO NOT MODIFY BY HAND' > $FILE


Use the following pattern to put large snippets of text into files:

cat > $FILE << EOF .... EOF

TSultanov · 2023-03-01T15:20:25Z

playground/frontend/Dockerfile

 RUN apt-get update && apt-get install -y wget xz-utils git

+RUN cat /app/playground_components/lib/src/build_metadata.g.dart


I would rather keep it in build logs. If anything goes wrong with generation, it will be problematic to extract this file content.

Aren't we already printing it out in the script generating this file?

TSultanov · 2023-03-01T15:21:34Z

playground/frontend/lib/config.g.dart

+// TODO(alexeyinkin): When it is merged, find a better place for the remaining
+//  constant and delete this file.
+
+const String kAnalyticsUA = 'G-BXFP2FNCKC';


Rename to config.dart or similar as it no longer is a generated file

This is to prevent merging conflicts because there are other PRs to change this file. When analytics lands, I will.

TSultanov · 2023-03-01T15:26:21Z

playground/frontend/playground_components/tools/generate_build_metadata.sh

+#  under the License.
+
+# Run this from the playground_components project root
+FILE='lib/src/build_metadata.g.dart'


Also, can we determine the path to the script location and use it to construct the full path to this generated file to drop the requirement of being in the playground_components project root?
Or, pass this value as an argument from a gradle task
Or, why not generate this file from gradle directly?

+ determining from the script location.
I would rather not use Gradle for this because Groovy files are currently messy and we cannot do this in Kotlin.

Malarg · 2023-03-02T05:23:38Z

playground/frontend/playground_components/lib/src/controllers/build_metadata.dart

+  final _runnerVersionFutures = <Sdk, Future<GetMetadataResponse>>{};
+
+  /// Returns the router version and starts loading if it is not started yet.
+  ComponentVersion? get routerVersion {


such way don't assure that it returns version and client won't know when it will be available. Suggest make it future and return cached data if the have been loaded

The single place, when this getter is used is _ComponentVersionsWidget Just use FutureBuilder there

The same about getRunnerVersion

Malarg · 2023-03-02T06:29:00Z

playground/frontend/playground_components/lib/src/repositories/backend_urls.dart

+/// - node.play.beam.apache.org
+/// This means that if the stage runs its own container for the given
+/// backend node, it is used. Otherwise the production backend is used.
+List<Uri> _getBackendUrlOptions(String node) {


place this method under _getBackendUrl

Malarg · 2023-03-02T06:32:03Z

playground/frontend/playground_components/lib/src/repositories/backend_urls.dart

+    return urls.first;
+  }
+
+  print('Probing multiple options for $node backend:');


all of these prints are required?

This code behaves differently locally vs staging vs prod, so better logging is required.

Malarg · 2023-03-02T06:34:55Z

playground/frontend/playground_components/lib/src/repositories/backend_urls.dart

+      await client.getMetadata();
+      print('Using $url');
+      return url;
+    } catch (ex) {


avoid catches without on clauses

Malarg · 2023-03-02T07:14:50Z

playground/frontend/playground_components/lib/src/repositories/code_repository.dart

@@ -35,9 +35,11 @@ const kProcessingStartedText = 'The processing has started\n';

 // TODO(alexeyinkin): Rename. This is not a repository but a higher level client.
 class CodeRepository {
-  final CodeClient _client;
+  final CodeClient client;


Client changed to public here just because of one method in build_metadata.dart. I suggest to keep this field incapsulated and turn back to private

playground/frontend/playground_components/lib/src/build_metadata.g.dart

playground/frontend/playground_components/tools/generate_build_metadata.sh

MakarkinSAkvelon · 2023-03-06T11:49:47Z

LGTM

github-actions · 2023-03-06T13:37:17Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

alexeyinkin · 2023-03-06T14:54:03Z

R: @damondouglas

The only failing check is due to snippet renaming, it is addressed in

Playground integration test run and editing #25033

Not merging it here to simplify separate PRs and reduce conflicts.

github-actions · 2023-03-06T14:55:05Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

playground/frontend/CONTRIBUTE.md

…oint-playground

learning/tour-of-beam/backend/README.md

playground/backend/cmd/server/controller.go

playground/api/v1/api.proto

damondouglas · 2023-03-07T22:15:00Z

playground/backend/containers/git-functions.gradle

+ext.getGitCommitHash = () -> {
+   def stdout = new ByteArrayOutputStream()
+   exec {
+      executable('git')
+      args('rev-parse', 'HEAD')
+      standardOutput = stdout
+   }
+   return stdout.toString().trim()
+} as Closure<String>
+
+ext.getGitCommitTimestamp = () -> {
+   def stdout = new ByteArrayOutputStream()
+   exec {
+      executable('git')
+      args('show', '-s', '--format=%ct', 'HEAD')
+      standardOutput = stdout
+   }
+   return stdout.toString().trim()
+} as Closure<String>


Why is a gradle wrapper needed? Why not just run the command in the context where the value is needed?

We need this in 6 separate locations:

Building the frontend container.

Building the router container.

Building the 4 runners.

The 5 backends may be similar but frontend and backend are still two rather distinct users of this. This motivates to do this in a single location like this because duplicating git show -s --format=%ct HEAD and git rev-parse HEAD 2-6 times is more confusing.

Next, we cannot do this in Dockerfile because by the time it is built the directory is unaware of .git.

This leads to the requirement of having the values before the 6 containers' builds start. And their builds are triggered from Gradle. So we cannot shorten this to command $(git rev-parse HEAD), we would still have the whole exec { ... } thing but in 12 locations.

damondouglas · 2023-03-07T22:16:50Z

playground/backend/containers/go/build.gradle

+   name containerImageName(
+         name: project.docker_image_default_repo_prefix + "playground-backend-go",
+         root: project.rootProject.hasProperty(["docker-repository-root"]) ?
+               project.rootProject["docker-repository-root"] :
+               project.docker_image_default_repo_root)
+   files "./build/"
+   tags containerImageTags()
+   buildArgs(
+         ['BASE_IMAGE'   : project.rootProject.hasProperty(["base-image"]) ?
+               project.rootProject["base-image"] :
+               "golang:1.18-bullseye",
+          'SDK_TAG'      : project.rootProject.hasProperty(["sdk-tag"]) ?
+                project.rootProject["sdk-tag"] : project.rootProject.sdk_version,
+          'SDK_TAG_LOCAL': project.rootProject.sdk_version,
+          'GIT_COMMIT'   : getGitCommitHash(),
+          'GIT_TIMESTAMP': getGitCommitTimestamp()])


The risk of wrapping more and more commands with gradle, the difficult it will be to troubleshoot in the future.

IMO this is ok in order to get the reusability

damondouglas · 2023-03-07T22:18:04Z

playground/backend/containers/java/Dockerfile

@@ -55,6 +57,7 @@ FROM apache/beam_java8_sdk:$BEAM_VERSION
 ARG BEAM_VERSION
 ARG SPRING_VERSION=5.3.25
 ARG KAFKA_CLIENTS_VERSION=2.3.1
+ENV BEAM_VERSION=$BEAM_VERSION


We should specify environment variables outside the Docker image using ConfigMap

This is not a runtime configuration parameter. This variable holds the value of Beam SDK version with which the image was built, so it's a compile-time parameter. Storing this value outside of the container makes little sense to me.

playground/backend/internal/environment/environment_service.go

…che#22271)

…nt-playground # Conflicts: # playground/frontend/playground_components/lib/playground_components.dart # playground/frontend/playground_components/pubspec.yaml

damondouglas

I ran through the entire build/deploy solution documented at https://github.com/akvelon/beam/blob/get-metadata-endpoint-playground/playground/terraform/README.md and I'm seeing CORS errors from the domain I specified -Pdns-name= in the documentation. I was unable to test through the installed application whether this PR broke anything. The images built successfully and the k8s manifests installed correctly.

…oint-playground

damccorm

This LGTM, we can iterate further as needed as called out in a few places

TSultanov added 4 commits February 22, 2023 19:08

Add stubs for GetMetadata endpoint

9297acc

Add GetMetadata() endpoint to controller

8d9de57

Add Beam SDK version to go build

a9a5510

Remove commented out code

42b5dba

alexeyinkin suggested changes Feb 23, 2023

View reviewed changes

TSultanov added 3 commits February 24, 2023 14:06

Change BuildCommitTimestamp type to int64

4fd3753

Reformat containers build.gradle

ecd840a

Change syntax in git-functions.gradle

5877451

TSultanov force-pushed the get-metadata-endpoint-playground branch from f4cd8ba to 5877451 Compare February 24, 2023 10:42

TSultanov added 2 commits February 24, 2023 17:40

Make build_commit and beam_sdk fields optional

92fe2b4

Update Tour of Beam protocol definitions to reflect Playground protoc…

90278a3

…ol changes

github-actions bot added the learning label Mar 1, 2023

alexeyinkin added 2 commits March 1, 2023 18:16

Show versions on the frontend (apache#22271)

5cf5099

Remove config.g.dart generation (apache#22271)

62fe1e7

github-actions bot added the build label Mar 1, 2023

Add config.g.dart (apache#22271)

0d41f15

TSultanov commented Mar 1, 2023

View reviewed changes

Malarg suggested changes Mar 2, 2023

View reviewed changes

Address review comments (apache#22271)

2ffc78e