Move model.proto here and add Dockerfile to build protobuf clients

[zoidyzoidzoid]

Which problem is this PR solving?

Closes https://github.com/jaegertracing/jaeger-idl/issues/55

Short description of the changes

I've added a Docker image that builds a small reproducible environment for tracking what dependencies are necessary for building clients to use the Jaeger protobuf messages. This can be used for building clients for other languages.

The image built from the Dockerfile in this MR builds for Python, Java, and Go.

I need to investigate if there is like a PROTOC_PATH or something where we could hide the includes. It'd make the Dockerfile much simpler to understand.

I was considering adding .gitkeep files or something in the necessary go/, java/, and python/ directories, since they're necessary for the image to build the clients.

I'm not sure if we wanna bundle the clients as is with the necessary language-specific packaging stuff, or not. That's probably out of scope of this PR though.

@yurishkuro mentioned here https://github.com/jaegertracing/jaeger/issues/1484#issuecomment-485863571 that someone should try the prototool image, ~which I have not~. Unfortunately no luck with it. It doesn't include the necessary imported protos, it seems.

It currently prints one warning, assuming the output directories exist:

# protoc -I=. -I=$GOPATH/src -I=$GOPATH/src/github.com/gogo/protobuf -I=$GOPATH/src/github.com/googleapis/googleapis --gofast_out=go/ --java_out=java/ --python_out=python/ proto/model.proto
proto/model.proto:20:1: warning: Import google/api/annotations.proto but not used.

# docker build -t jaegertracing/jaeger-idl .
#  docker run -it --rm -v $(pwd):/usr/src/app jaegertracing/jaeger-idl
proto/model.proto:20:1: warning: Import google/api/annotations.proto but
not used.

[pavolloffay]

There are multiple files in the proto files https://github.com/jaegertracing/jaeger/tree/master/model/proto. Could we add them here all?

[pavolloffay]

Could you please add a test to CI? We need to build the image and build the model for at least one language to verify the build works.

[zoidyzoidzoid]

Thanks for the thorough review, folks. 🙏

There are multiple files in the proto files https://github.com/jaegertracing/jaeger/tree/master/model/proto. Could we add them here all?

Great point. That makes sense. I've moved them all here

I am not clear how this would work with versioning of the dependencies that we use at runtime in the Jaeger backend.

This is a great question. One idea I considered is that we could import from the jaeger-idl submodule in jaegertracing/jaeger, and commit the generated clients here.

I think that makes more sense than generating them from this repo into the other repo, since it'll make versioning and bumping the clients make more sense, when it comes to building a versioned artifact that includes the client and proto files that match.

Then we can also add a CI job to generate the clients and error if they differ from what's in the repo, since then we can catch if folks update the protobuf definitions without re-generating the clients.

Could you post some details about what experiments you ran with the prototool, and what was missing there?

So the prototool Docker image builds a super small image that seemed to be missing the necessary third-party protos we import from, but I could make a PR to add them.

± docker run -it --rm uber/prototool
/ # find /  -name "*.proto"
./usr/include/google/protobuf/api.proto
./usr/include/google/protobuf/any.proto
./usr/include/google/protobuf/field_mask.proto
./usr/include/google/protobuf/source_context.proto
./usr/include/google/protobuf/wrappers.proto
./usr/include/google/protobuf/descriptor.proto
./usr/include/google/protobuf/struct.proto
./usr/include/google/protobuf/timestamp.proto
./usr/include/google/protobuf/empty.proto
./usr/include/google/protobuf/type.proto
./usr/include/google/protobuf/duration.proto
./usr/include/google/protobuf/compiler/plugin.proto

But we need the GRPC ones and gogoproto ones:

This branch adds the gogoproto, since we're downloading that dependency already: https://github.com/uber/prototool/compare/dev...zoidbergwill:dev?expand=1

Not sure if the uber/prototool folks would wanna see a PR that adds the googleapis protos to that Docker image.

Another option is vendoring the googleapis and gogoproto protobuf files?

Could you please add a test to CI? We need to build the image and build the model for at least one language to verify the build works.

Still need to do this. Will hopefully get it done tomorrow.

[zoidyzoidzoid]

Ooooh, I see the awesome thrift stuff in the Makefile. I'll do something similar to that for the CI.

[zoidyzoidzoid]

What you get from the new make protoc-gen-go-client

± make protoc-gen-go-client
docker build -f proto/Dockerfile -t jaegertracing/jaeger-idl-protoc .
mkdir -p "${HOME}/src/go/src/github.com/jaegertracing/jaeger/idl/go"
docker run -it --rm -v ${HOME}//src/go/src/github.com/jaegertracing/jaeger/idl:/work jaegertracing/jaeger-idl-protoc
docker run -it --rm -v ${HOME}/src/go/src/github.com/jaegertracing/jaeger/idl:/work jaegertracing/jaeger-idl-protoc proto/model.proto
model.proto:20:1: warning: Import google/api/annotations.proto but not used.
docker run -it --rm -v ${HOME}/src/go/src/github.com/jaegertracing/jaeger/idl:/work jaegertracing/jaeger-idl-protoc proto/model_test.proto
docker run -it --rm -v ${HOME}//src/go/src/github.com/jaegertracing/jaeger/idl:/work jaegertracing/jaeger-idl-protoc proto/zipkin.proto

± find go                                                                                                                                         
go
go/model_test.pb.go
go/proto
go/proto/model.pb.go
go/api_v2
go/api_v2/sampling.pb.go
go/api_v2/query.pb.go
go/api_v2/collector.pb.go
go/model.pb.go
go/zipkin.pb.go

[zoidyzoidzoid]

Do we wanna move all protobuf files out of the Jaeger repo into here?

[yurishkuro]

I think this is being replaced by #61 - cc @pavolloffay

[pavolloffay]

Yes, this seems like done in #61.

thanks for the PR @zoidbergwill