gRPC reflections

[slinkydeveloper]

This PR implements the grpc reflection service in the ingress #192

[slinkydeveloper]

@tillrohrmann I need your opinion on this design idea: rather than having this new registry, what if we rename MethodDescriptorsRegistry into something like SchemaRegistry to have something more generic, and have a single point where we take care of all the schema shenanigans?

There is a problem with such data structure though, which is that I want its access patterns "optimized" per use case, so I fear it might leak some details of how the reflection service... But perhaps this is fine?

[tillrohrmann]

I would probably start with the SchemaRegistry once we see that there is common functionality and common data that can be shared between the different registries. Until then, I would probably continue with the targeted approach. No need to foresee all eventualities right now.

[slinkydeveloper]

common data that can be shared between the different registries

Common data is definitely already there, given they share the same descriptor pools (which under the hood are Arcs). The problem with keeping them separate is that they might have inconsistent behaviour, given as it is, ReflectionServiceState overwrites symbols with same name, while MethodDescriptorsRegistry won't.

[slinkydeveloper]

For the time being i wired it up in a way that makes more or less sense, although prone to eventual inconsistencies #227 (comment)

[tillrohrmann]

Thanks for creating this PR @slinkydeveloper. Support for gRPC reflection is a cool feature which will improve Restate's usability :-) From what I can tell, the changes make sense to me. Concerning whether to unify the registries or not I would probably look for commonalities between the registries and once we see them clearly emerge, then merge the different registries.

[tillrohrmann]

Does service_names need to be a sorted vector? Could it also be a HashSet or so?

[slinkydeveloper]

It's optimized for writing it out, where the generated interface requires a vec and not a set

[tillrohrmann]

I would probably start with the SchemaRegistry once we see that there is common functionality and common data that can be shared between the different registries. Until then, I would probably continue with the targeted approach. No need to foresee all eventualities right now.

[slinkydeveloper]

I think I addressed most of the comments. Now everything is wired up and works correctly.

I took a shortcut of adding the ingress_grpc dependency to meta, otherwise I needed some work to either split the reflections code, or to move it all to service_metadata. Moving all the code requires to add prost, tonic and co to service_metadata, which doesn't seem right. Splitting the reflections code also doesn't look ok because I prefer to keep all this code as much self contained as possible for the time being, as I'm not completely sure we implement correctly the grpc reflections contract, and keeping everything in the same place helps me to iterate faster on that. Also the registry side of the reflections will probably change considerably in the context of #43, hence the shortcut.

Example:

# No need to provide a descriptor anymore when starting grpcurl and grpcui
$ grpcui -plaintext localhost:9090
gRPC Web UI available at http://127.0.0.1:38421/

# grpcurl also provides some query methods
$ grpcurl -plaintext localhost:9090 list
counter.Counter
$ grpcurl -plaintext localhost:9090 describe
counter.Counter is a service:
service Counter {
  rpc Add ( .counter.CounterAddRequest ) returns ( .google.protobuf.Empty );
  rpc Get ( .counter.CounterRequest ) returns ( .counter.GetResponse );
  rpc GetAndAdd ( .counter.CounterAddRequest ) returns ( .counter.CounterUpdateResult );
  rpc Reset ( .counter.CounterRequest ) returns ( .google.protobuf.Empty );
}

[tillrohrmann]

LGTM. I had only a few minor comments. Also tried the change out and it worked with grpcui and grpcurl :-)