Feature: Add ServiceDesc/FileDescriptor() to UnaryServerInfo/StreamServerInfo

[ppacher]

What version of gRPC are you using?

master

What version of Go are you using (go version)?

Go 1.9

Hi,

maybe this is already solved but I didn't find anything helpful. I'm using ServiceOption and MethodOption to annotate RPC calls with permission checks, however, I did not find an easy way to get the ServiceDescriptorProto/MethodDescriptorProto in a Unary or Stream interceptor. Currently I need to use proto.FileDescriptor(path) which is somehow bad as I need to pass in the path of the proto file that defines the service. I'd suggest to add the ServiceDesc to the UnaryServerInfo/StreamServerInfo structs so interceptors can use the Metadata field to find out the file descriptor proto that defines the service.

If there's another way I would be glad to get some hints.

Best regards
Patrick

[dfawley]

This proposal would mean changing the generated code to fill in the new field. This means new generated code would not be compatible with old gRPC libraries, but new gRPC libraries could support older versions of the generated code. That seems reasonable, but this isn't something we will be able to prioritize for at least a quarter.

[jhump]

@ppacher, in case it helps, we get around this using some boiler-plate in the server wiring code, an interceptor, and the protoreflect library:

methods := map[string]*desc.MethodDescriptor{}
// this example just shows how to do this with a unary interceptor, but
// the same approach works for streaming
interceptor := func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) {
    md := methods[info.FullMethod]
    // stash md in a context using context.WithValue, which now
    // now provides access to it from interceptor and handlers
    newCtx := ctxWithMethodDesc(ctx, md)
    return handler(newCtx, req)
}

// if you already use an interceptor, you can have interceptor above invoke
// it instead of handler OR use something like grpc-middleware to combine
// them into one...
svr := grpc.NewServer(grpc.UnaryInterceptor(interceptor))

somepkg.RegisterSomeServiceServer(svr, someServiceImpl{})
someotherpkg.RegisterSomeOtherServiceServer(svr, someOtherServiceImpl{})
yetanotherpkg.RegisterYetAnotherServiceServer(svr, yetAnotherServiceImpl{})

// when done registering services, you can get the descriptors from the
// gRPC server and stash them into the map that the interceptor uses:
// (error handling omitted for brevity)
sds, _ := grpcreflect.LoadServiceDescriptors(svr)
for _, sd := range sds {
    for _, md := range sd.GetMethods() {
        methodName := fmt.Sprintf("/%s/%s", sd.GetFullyQualifiedName(), md.GetName())
        methods[methodName] = md
    }
}

// Now you can start svr and your interceptors/handlers can get
// the method descriptor from the context and interrogate it 
// (like examining custom options to determine policy)

[jhump]

@dfawley, an alternate approach that does not require changes to protoc-gen-go would be to store the ServiceDesc in the context. It's a little smelly to rely so much on values in context, but it is not out of line with how a lot of other information is already being supplied to interceptors/handlers.

[ppacher]

@jhump thanks for the hint. This seems like a reasonable workaround and avoids using the file paths. Though I think addind ServiceDesc and MethodDesc to the UnaryServerInfo/StreamServerInfo structs is still a good idea. But for now I can go with your approach.

[nate-meta]

@jhump thanks for the pointer!
Is there an easy way to convert the MethodDescriptor back to the original proto extension?

[jhump]

Is there an easy way to convert the MethodDescriptor back to the original proto extension?

I'm afraid you lost me. A method descriptor is not a proto extension, so there is no conversion between. If you are asking how to get the proto extensions from the method descriptor:

var md *desc.MethodDescriptor
opts := md.GetMethodOptions()
proto.HasExtension(opts, E_someGeneratedMethodExtension)

(Where E_someGeneratedMethodExtension would be the extension description generated into Go code from your custom option definition.)

[nate-meta]

Sorry, my question is not entirely related to this thread, but was how to get the proto value described in the extension set on a method.
I.e. how do I unmarshal the value for NestedMessage of "asdf" in this example:

service MyService {
  rpc MethodWithOption (FooRequest) returns (FooResponse) {
    option (some_value_option) = {stuff: "asfd"};
  }
}

extend google.protobuf.MethodOptions {
  NestedMessage some_value_option = 50006;
}

message NestedMessage {
  string stuff = 1;
}

message FooRequest {}
message FooResponse {}

I vaguely remember how to do this in Java, but can't find any documentation for how its done in Go.

[jhump]

@nate-meta, my example showed that (mostly). Extensions in a proto file result in exported *proto.ExtensionDesc values in the generated code named E_<name>. So your example should result in E_SomeValueOption. You can then query it like so:

var md *desc.MethodDescriptor // assume we populate this for current RPC
opts := md.GetMethodOptions() // we get the descriptor's options
// and then get the custom option from that message
nestedVal, _ := proto.GetExtension(opts, E_SomeValueOption)
nestedMsg, ok := nestedVal.(*NestedMessage)
if ok {
  // this will print "asdf"
  fmt.Println(nestedMsg.Stuff)
}

[stale]

This issue is labeled as requiring an update from the reporter, and no update has been received after 7 days. If no update is provided in the next 7 days, this issue will be automatically closed.

[dfawley]

I failed to properly disable the stale-bot, which is malfunctioning (probot/stale#207). It's a 2-step process and I only completed the first step. Sorry, it is properly disabled now.

[ghost]

Are there any plans regarding this topic?

[dfawley]

Not currently. It seems like there are at least two options: adding this to the context or adding it to Unary/StreamServerInfo. If this is urgent and you/someone can volunteer to weigh the pros/cons, brainstorm alternatives, and propose/implement the solution, we will review the proposal and PR.

[jhump]

FWIW, I don't think this is really needed anymore, with v1.4+ of the protobuf runtime. You can instead look it up using the method name:

import "google.golang.org/protobuf/reflect/protoreflect"
import "google.golang.org/protobuf/reflect/protoregistry"

desc, err := protoregistry.GlobalFiles.FindDescriptorByName(protoreflect.FullName(methodName))
if err == nil {
    method := desc.(protoreflect.MethodDescriptor)
    // now we have the method descriptor
}

You probably need to do some massaging to the method name. I think it looks like "/some.Service/Method" whereas the proto registry lookup step needs it formatted as "some.Service.Method".

[dfawley]

I agree with @jhump's assessment, above. Closing.