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

protoc failing when attempting to generate bindings for gRPC storage plugin #2686

Closed
js8080 opened this issue Dec 10, 2020 · 12 comments · Fixed by #2687
Closed

protoc failing when attempting to generate bindings for gRPC storage plugin #2686

js8080 opened this issue Dec 10, 2020 · 12 comments · Fixed by #2687

Comments

@js8080
Copy link
Contributor

js8080 commented Dec 10, 2020

I am attempting to create a C#-based gRPC Storage Plugin and I am getting stuck on the very first step of generating the bindings using protoc.

The documentation says:

To generate the bindings for your language you would use protoc with the appropriate xx_out= flag. This is detailed in the protobuf documentation and you can see an example of how it is done for Go in the top level Jaeger Makefile.

I'm running WSL 1 with Ubuntu and first I cloned the jaegertracing repo:

$ cd /mnt/c/source/repos
$ git clone https://github.com/jaegertracing/jaeger.git
$ cd jaeger/plugin/storage/grpc/proto

Then I tried to run protoc on plugin/storage/grpc/proto/storage.proto as follows:

$ protoc \
-I/mnt/c/source/repos/jaeger/plugin/storage/grpc/proto \
--csharp_out=/mnt/c/source/repos/myjaegerstorageplugin \
storage.proto

This resulted in the following errors:

gogoproto/gogo.proto: File not found.
model.proto: File not found.
storage.proto: Import "gogoproto/gogo.proto" was not found or had errors.
storage.proto: Import "model.proto" was not found or had errors.
storage.proto:49:14: "jaeger.api_v2.DependencyLink" is not defined.
storage.proto:55:5: "jaeger.api_v2.Span" is not defined.
storage.proto:120:14: "jaeger.api_v2.Span" is not defined.

So I then studied the Makefile a bit and attempted to run make proto but that failed because I don't have docker on my WSL 1. Then I tried editing the Makefile to call protoc locally instead of using docker... Still no success. Then I realized there is a separate jaeger-idl repository and maybe I am supposed to be referencing the model.proto from there: https://github.com/jaegertracing/jaeger-idl/blob/master/proto/api_v2/model.proto

But this all seems very round-about and unnecessarily difficult when I simply want to generate bindings for the plugin. I shouldn't have to actual compile Jaeger locally for this, right?

Do I need GoGo Protobuf? The documentation doesn't say so.

How does one actually generate the gRPC bindings for the gRPC Storage Plugin?

Also as an aside, when posting this question it says "Please consider asking question via Community Forum first". Is there an official "Community Forum"? I couldn't find one. I searched StackOverflow and found nothing as well.

@yurishkuro
Copy link
Member

You should try using https://github.com/jaegertracing/docker-protobuf/ instead of plain protoc (see Makefile in this repo for examples).

@js8080
Copy link
Contributor Author

js8080 commented Dec 10, 2020

I don't know what I'm doing wrong but it is not working for me. I run this for a simple test:

$ docker run --rm jaegertracing/protobuf --help

My docker downloads the image but it does not print out anything.

If I run this which I based on the Makefile, the command is not interpreted correctly:

docker run --rm -u 1000 -v//c/source/repos/jaeger:/jaeger -w/jaeger jaegertracing/protobuf:0.2.0 -I/jaeger -Iidl/proto/api_v2 -I/usr/include/github.com/gogo/protobuf -Iplugin/storage/grpc/proto --gogo_out=plugins=grpc,Mgoogle/protobuf/descriptor.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types,Mgoogle/api/annotations.proto=github.com/gogo/googleapis/google/api,Mmodel.proto=github.com/jaegertracing/jaeger/model:/jaeger/proto-gen/storage_v1 plugin/storage/grpc/proto/storage.proto

Errors with:

idl/proto/api_v2: warning: directory does not exist.
/usr/include/github: warning: directory does not exist.
.com/gogo/protobuf: No such file or directory

The fact that it breaks up the github and .com portions makes me think docker is interpreting that . in there as a special character. However, I have not been successful in figuring out how to escape it.

@yurishkuro
Copy link
Member

idl/proto/api_v2: warning: directory does not exist

did you initialize git submodule? Looks like your idl dir is empty.

@js8080
Copy link
Contributor Author

js8080 commented Dec 10, 2020

did you initialize git submodule? Looks like your idl dir is empty.

No, that is not mentioned anywhere in the documentation:
https://github.com/jaegertracing/jaeger/tree/master/plugin/storage/grpc

@yurishkuro
Copy link
Member

Please help us improve it, including adding example of generating proto if you make it work

@yurishkuro
Copy link
Member

when you clone Jaeger repo, run git submodule init && git submodule update.

@yurishkuro
Copy link
Member

yurishkuro commented Dec 10, 2020

@js8080
Copy link
Contributor Author

js8080 commented Dec 10, 2020

OK I got some code to generate!

docker run --rm -u 1000 -v/c/source/repos/jaeger:/jaeger -w/jaeger jaegertracing/protobuf:0.2.0 "-I/jaeger -Iidl/proto/api_v2 -I/usr/include/github.com/gogo/protobuf -Iplugin/storage/grpc/proto --csharp_out=/jaeger/code plugin/storage/grpc/proto/storage.proto"

I will create pull request to update the documentation with this info.

@yurishkuro
Copy link
Member

I wonder if we could customize the Makefile, which already generates all this code for Go, to also do other languages. Really, the only difference in your example should be this --csharp_out= part, we could have it as an optional param, and then just have users run make proto PROTO_EXTRA="--csharp_out=..."

@js8080
Copy link
Contributor Author

js8080 commented Dec 10, 2020

I'm OK with that approach but we should also document that capability on the README here:
https://github.com/jaegertracing/jaeger/tree/master/plugin/storage/grpc/README.md

Also, sorry I'm a noob at this. I can't seem to create a branch to make a PR. I'm guessing that's restricted maybe...

@yurishkuro
Copy link
Member

you cannot push a branch to this repo, you need to fork it and push a branch to your fork, then you can create a PR from it. E.g. I have this setup:

$ git remote -v
origin	[email protected]:yurishkuro/jaeger.git (fetch)
origin	[email protected]:yurishkuro/jaeger.git (push)
upstream	[email protected]:jaegertracing/jaeger.git (fetch)
upstream	[email protected]:jaegertracing/jaeger.git (push)

@js8080
Copy link
Contributor Author

js8080 commented Dec 11, 2020

OK, I think I did it right :)
#2687

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 a pull request may close this issue.

2 participants