Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create protobuf generation GitHub action #938

Merged
merged 4 commits into from
Jul 16, 2020

Conversation

evantorrie
Copy link
Contributor

@evantorrie evantorrie commented Jul 14, 2020

This is in preparation for automatically generating updated protobuf-derived *.pb.go files from a specific version of https://github.com/open-telemetry/opentelemetry-proto checked in as a git submodule in internal/opentelemetry-proto (coming in a subsequent PR). This allows source files in this repo to avoid having any direct dependency on *.pb.go files from github.com/open-telemetry/opentelemetry-proto/go/gen and thus should resolve #793. Any sources within this repo which depend on the location of the protobuf generated files will need to be updated to reflect their new location which will be at go.opentelemetry.io/otel/internal/opentelemetry-proto-gen/.

Synopsis of Github Action

Whenever a PR with an update to internal/opentelemetry-proto is created, this github action will be invoked on the PR and will

  • checkout the source from the PR, including the source from the submodule pointing to a specific commit of https://github.com/open-telemetry/opentelemetry-proto
  • run make -f Makefile.proto protobuf, which
    • rewrites the option go_package = ... lines in the .proto files to a package which resides at go.opentelemetry.io/otel/internal/opentelemetry-proto-gen, i.e. inside this repo.
    • uses a pinned version of the docker image namely/protoc-all to generate the new and changed *.pb.go files, copying them into the internal/opentelemetry-proto-gen directory.
  • use the stefanzweifel/git-auto-commit-action@v4 github action to commit any changes to these files as a new commit in the PR.

The existing build action in CircleCI will run unchanged, but unlike this github action, will not populate the git submodule. Similarly, developers should run make precommit at the top-level of the repo with the git submodule at internal/opentelemetry-proto left unpopulated in all cases except when testing out an update to the .proto files. This avoids go test ./... pulling in dependencies needed in the opentelemetry-proto repo, but not needed for this repo.

@lizthegrey
Copy link
Member

Love this change.

@evantorrie
Copy link
Contributor Author

For an example of what happens after a PR with a new version of the git submodule, you can take a look at my tests here: evantorrie#35
The github action commit is this one: evantorrie@115349f

The PR itself still shows as failed because I don't have anything larger than the default "medium" CircleCI runner size for my personal account, so the links/tests of the normal build fail, but this should not be a problem on this account.

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

Nice 👍

Makefile.proto Outdated Show resolved Hide resolved
Move file-local mode variable to line by itself.

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MrAlias MrAlias merged commit dd79483 into open-telemetry:master Jul 16, 2020
@MrAlias MrAlias mentioned this pull request Jul 20, 2020
@evantorrie evantorrie deleted the protogen-github-action branch August 10, 2020 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider generating Go types for OTLP in this repo
3 participants