Remove google.golang.org/protobuf dependency from model & storage APIs

[akagami-harsh]

Which problem is this PR solving?

• Part of Remove dependency on deprecated github.com/golang/protobuf #4911

Description of the changes

• using google.golang.org/protobuf in place of github.com/golang/protobuf
• github.com/golang/protobuf is not completely removed it is still being used as an indirect dependency

How was this change tested?

• make test

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: yarn lint and yarn test

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

see 2 files with indirect coverage changes

📢 Thoughts on this report? Let us know!

[mmorel-35]

Hi @akagami-harsh ,
You might need to promote Docker-protobuf to use google.golang.org/protobuf first, I created a PR for that jaegertracing/docker-protobuf#28
Maybe @yurishkuro can confirm that process ?

[mmorel-35]

@akagami-harsh ,
Can you update link to jaeger-idl last commit, it uses protoc-gen-go from google.golang.org. Let’s see if that fixes things .

[akagami-harsh]

@akagami-harsh , Can you update link to jaeger-idl last commit,

how can i do that?

[yurishkuro]

I don't think we need to bump idl sub module because the schemas have not changed. How are you regenerating the types? You need to bump the version of the proto image in the Makefile first.

[akagami-harsh]

How are you regenerating the types?.

As a newcomer to protocol buffers, I've attempted to regenerate it manually using protoc

[yurishkuro]

please use the make target to generate types, not some manual method

[akagami-harsh]

used make proto to generate types but it was throwing this error

protoc-gen-go: unable to determine Go import path for "storage_test.proto"
Please specify either:
      • a "go_package" option in the .proto source file, or
      • a "M" argument on the command line.

after altering the Makefile and *_test.protos . make proto ran successfully, but github.com/golang/protobuf is still present, can you please review it and give some pointers how can i further fix it

[yurishkuro]

I see that the old package is still imported in only one file

grep -rn "github.com/golang/protobuf/proto" .
./proto-gen/api_v3/query_service.pb.gw.go:17:	"github.com/golang/protobuf/proto"

And that file is generated by protoc-gen-grpc-gateway plugin, which probably also needs to be updated in our protobuf Docker image

[yurishkuro]

https://github.com/grpc-ecosystem/grpc-gateway - latest version is 2.18, but our Docker file is using GRPC_GATEWAY_VERSION=1.16.0

[mmorel-35]

Hi @yurishkuro ,
I propose jaegertracing/docker-protobuf#29
I couldn't use 2.18 as it requires Go 1.18 and the image needs Go 1.17 yet.

While consulting grpc-gateway, I saw that they are using buf . Do you know this ? Would it be a good idea to use it instead of docker-protobuf or adapt docker-protobuf to rely on it ?

[yurishkuro]

Is there an issue with bumping Go version?

Alternatively, can we do a smaller bump of gateway, enough to remove the old dep but not to the latest version?

I don't have enough data about buf. As I mentioned, otlp is using gogo currently.

[mmorel-35]

Taking a more recent version than go 1.17.3 in docker-protobuf will require to upgrade alpine version then it leads to upgrade grpc which is failing then

[yurishkuro]

but we're on the latest grpc version in the main repo, why would that one fail? Also, how much of a dependency is between alpine and grpc?

[mmorel-35]

I mean the grpc version in the dockerfile of docker-protobuf.
It is 1.35.0 while there is a 1.59.2 yet.

In dockerfile of docker-protobuf. There is a use of golang:golang:${GO_VERSION}-alpine${ALPINE_VERSION} but not every couple of go and alpine version.
Golang 1.17.3 is the last for alpine 3.13

[yurishkuro]

I think it's worth trying to upgrade everything.

[yurishkuro]

We may want to add a dependabot to that repo - although not sure it will work on a shell script with variables

[yurishkuro]

I am going to merge this, as it's an incremental improvement for the primary code paths.