Update proto package imports to google.golang.org/protobuf/proto

[delgec]

Hello everyone

Why is this dependency not removed?

It's deprecated and we are constantly receiving warnings in the our applications.

github.com/golang/protobuf v1.5.2

[dfawley]

We looked into this migration a year or two ago, but it's a non-trivial amount of work (due to the API changes) for a relatively small benefit (it's been rewritten internally to use the new library already).

[menghanl]

This was discussed in #5110

[dfawley]

...which was a dupe of #3554

I think we should have an issue for this open, but we have no upcoming plans to do anything here.

[gunturaf]

I tried to remove all references to github.com/golang/protobuf, but stuck with the file testv3.pb.go in reflection/grpc_testing_not_regenerate, it's said that we should not regenerate the pb inside that package... I only came up with several possible solutions:

• turn grpc_testing_not_regenerate into sub-module, but with this approach we need to add a replace directive to the root go.mod to enable go mod tidy
• make reflection package pure of testing by moving all test code to the root test directory, but I think this will still cause the root module requires github.com/golang/protobuf

wdyt?

[dfawley]

FWIW I created golang/protobuf#1442 so we can try to get a non-deprecated way of maintaining our existing API.

[chriscasola]

Is there a path forward now that golang/protobuf#1442 has been resolved? I may be able to contribute.

[easwars]

@chriscasola : Please feel free to send us a PR. But please do keep in mind that old proto package is part of some of our APIs, and some of them are not marked experimental, and therefore cannot be changed.

[Clement-Jean]

The worst thing out of this transition is this part of channelz/service/service_test.go. In order to be able to switch to google.golang.org/protobuf/proto It seems we somehow need to implement protoreflect.ProtoMessage and thus implement the following function by hand:

func (m *OtherSecurityValue) ProtoReflect() protoreflect.Message { return /*TBD*/ }

if anyone knows how to make OtherSecurityValue implement protoreflect.ProtoMessage properly, let me know because I'm almost done switching everything to google.golang.org/protobuf/proto (at least go test ./... says so).

Also, @easwars, I'm interested in knowing which APIs cannot be changed because old proto is part of them.

[easwars]

Also, @easwars, I'm interested in knowing which APIs cannot be changed because old proto is part of them.

The last time we undertook this exercise, I vaguely remember the old proto package being part of some non-experimental APIs. But now:

easwars@minetop: grpc-go$ grep -r "github.com/golang/protobuf/proto" * | grep -v test | grep -v internal | grep -v examples
balancer/grpclb/grpclb_remote_balancer.go:      "github.com/golang/protobuf/proto"
credentials/credentials.go:     "github.com/golang/protobuf/proto"
encoding/proto/proto.go:        "github.com/golang/protobuf/proto"

The reference to the old proto package in:

• grpclb: this is not part of the API. this is just a call to proto.Equal
• credentials: there is a field of type proto.Message in OtherChannelzSecurityValue. But this is marked experimental
• proto: this is the implementation of the proto encoder. Here also, it is not part of the API. I believe we can switch to the new proto package here.

[easwars]

The worst thing out of this transition is this part of channelz/service/service_test.go

@dfawley is working on a major rewrite of the channelz implementation. He might have some ideas here.

[easwars]

I removed the P3 label so that we track this in our weekly meeting to see if we can make progress.

[dfawley]

The worst thing out of this transition is this part of channelz/service/service_test.go.

Please just leave this part alone for now. I've rewritten these tests to not convert away from proto before checking the result; instead it builds the expected proto response and validates the actual response directly against the expectation.

I'm almost done switching everything to google.golang.org/protobuf/proto (at least go test ./... says so).

Woah that's great to hear! I'll assign this issue to you for now.

The part of proto that's in our API is the status package, around Details. With golang/protobuf#1442 resolved, we should have an alternative to use now instead of proto.Message.

[arvindbr8]

#6736 -- in flight to take a first pass at fixing the dependency

[dfawley]

Thanks for the PRs @Clement-Jean . IIUC the only remaining thing is channelz, which is blocked on #6969.

[pavelpatrin]

#6969 is closed now. @dfawley should this go forward?

[dfawley]

Yes, I think we're all clear now to finish it.

Unfortunately, it looks like we'll still keep the dependency in perpetuity because of testing in https://github.com/grpc/grpc-go/blob/cce163274b6cb9de2b2bf0bc742384e5755fee42/reflection/grpc_testing_not_regenerate/testv3.go#L41C9-L41C41

[arvindbr8]

This was closed by automation. Keeping this open and assigning it my self to verify the fix.