gnmi_ext proto error when doing protoreflect

[sbapu-arista]

We are seeing issues with the way gnmi_ext.pb.go is generated from gnmi_ext.proto
When using https://github.com/jhump/protoreflect for doing reflection on gnmi, we are getting this error

File not found: github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto

Digging a little more we found that the file descriptor generated in gnmi_ext.pb.go
here at line 549
549 var File_proto_gnmi_ext_gnmi_ext_proto protoreflect.FileDescriptor
Has a relative path
21 // source: proto/gnmi_ext/gnmi_ext.proto
as indicated in line 21 of the file
While the golang import path is
github.com/openconfig/gnmi/proto/gnmi_ext/
as imported in github.com/openconfig/gnmi/proto/gnmi/gnmi.pb.go

It would be great if we can generate gnmi_ext.pb.go with relative path
github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto
instead of
proto/gnmi_ext/gnmi_ext.proto

We were able to test that the issue gets resolved by regenerating gnmi_ext.pb.go with the said relative path.

[jxx-gg]

While looking, just curious what kind of file description information are you looking at? Will the existing one implemented by default (google.golang.org/protobuf/reflect/protoreflect) meet your requirement? e.g.

e := gnmi_ext.Extension{}
d := e.ProtoReflect().Descriptor() 

[sbapu-arista]

Hi Johnson,
This is not exactly a problem with opeconfig/gnmi but with the actual protoreflect tools. Google's protoreflect being one of them. I'm hoping this issue will make it clear
golang/protobuf#567

A better explanation of what we are facing
We use gRPC reflection to get the proto from services(when calls are made over plain http e.g: restful wrappers for grpc). As part of resolving a service, protoreflect resolves all of the imports in the file that the service is contained in[1]. However, this resolution is exact match, so the name of the imported file must match what the path the proto file descriptor is registered[2] as. The path the proto-file descriptor is registered as is path to the file passed to protoc.

In this case, it looks like gnmi_ext.proto was generated with 'protoc <opts...> proto/gnmi_ext/gnmi_ext.proto', as shown by the '// source: proto/gnmi_ext/gnmi_ext.proto' comment. This doesn't match the import in gnmi.proto, which is "github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto".

[1] This dependency resolution is specific to the reflection library we use.
[2] This registration is due to how protoc-go/go's protobuf works and there isn't a way to override the path aside from regenerating the file via protoc.

[jxx-gg]

Thanks for the context. Can you please share how did you generate the gnmi_ext.pb.go with the corrected relative path?

[sbapu-arista]

Hi Johnson,
The gnmi is a vendor repo for us and given that, I was able to fix it by doing this from base of the repo.

protoc --proto_path=vendor/ --go_out=vendor/ github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto

In your case I guess you'll have to do something like
protoc --proto_path=$GOPATH/src/ --go_out=$GOPATH/src/ --go_opt= github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto

You should be able to verify by looking at this line in generated code
21 // source: proto/gnmi_ext/gnmi_ext.proto
Should become
21 // source: github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto

and the file descriptor should become
549 var File_github_com_openconfig_gnmi_proto_gnmi_ext_gnmi_ext_proto protoreflect.FileDescriptor

You may do the same for gnmi.proto as well I think but it isn't imported in any other proto I believe so it won't be necessary.

[jxx-gg]

I have updated the compile_protos.sh file and recompiled the protos. Please let me know if you are still having trouble.

[sbapu-arista]

Looks good. Thanks for doing this quickly.

[dopufol]

the problem is not resolved in the latest version with reflection

using grpurl to run gnmi.gNMI.Capabilities would result the following error

grpcurl -plaintext localhost gnmi.gNMI.Capabilities
Error invoking method "gnmi.gNMI.Capabilities": failed to query for service descriptor "gnmi.gNMI": file "proto/gnmi/gnmi.proto" included an unresolvable reference to ".gnmi_ext.Extension"

this issue is similar to:
openconfig/gribi#37
openconfig/gnsi#176

@gcsl @dplore