tonic: inject an GrpcMethod extension value to request

[linw1995]

Motivation

Resolve #772

Solution

Add an extension GrpcMethod to Request via the NamedService trait.
And update tonic-build for making GrpcMethod as a static reference.

This only works on this case

Server::builder()
     .add_service(TestServer::with_interceptor(Svc, interceptor))
// ...

not for the case like

Server::builder()
     .layer(tonic::service::interceptor(interceptor))
     .add_service(TestServer::new())
// ...

[LucioFranco]

I like the idea of adding GrpcMethod (maybe we can find a better name) and inserting that into the extensions. I don't understand why we need a grpc_method trait fn and why we need the extra const?

I would accept this PR if it only contains the GrpcMethod and not the rest as I don't see much value and it seems like we can get away with the simple GrpcMethod part.

[linw1995]

Yes, we can find a better name than "GrpcMethod" and "grpc_method".

The "grpc_method" is applied for ensuring the "GrpcMethod" actually exists in the protobuf definition. And providing static lifetime variables instead of generating by the runtime, I believe will acquire more performance improvement.

And the extra constant variables are useful for references and preventing typo accidents.

Please correct me if something is incorrect. Thank you @LucioFranco

[RyanAmos]

@linw1995 We have a need for something very similar to this for reporting prometheus request metrics. We need one small piece of additional information for us to be able to leverage this completely. We need the method type, which is one of four types.

Curious if you're interested in extending this a tad bit further to include that information on the GrpcMethod struct? Below is an example of what I'm referring to:

pub enum MethodType {
    Unary,
    ClientStream,
    ServerStream,
    BidiStream,
}

Adding that enum as a field on the GrpcMethod struct would make things significantly easier to get the information we need.

[linw1995]

@RyanAmos I am excited about including this in the future. But this pull request currently needs to be reviewed first, then I can put some effort into further work.

[LucioFranco]

What is the goal of this fn? Why can't we just inject a gRPC method into the Request object passed to server handlers?

[linw1995]

Using it can easily retrieve the static gRPC method info via the HTTP URL path. Injecting this info into the Request object is hard to make it happen if we want to keep it in a static lifetime. @LucioFranco

[LucioFranco]

I think I am not convinced yet on the server side of this, but the client side I think would be useful. Could you break this PR into those two sections? I believe the client one should be pretty small and should be easy to merge.

[linw1995]

I think I am not convinced yet on the server side of this, but the client side I think would be useful. Could you break this PR into those two sections? I believe the client one should be pretty small and should be easy to merge.

@LucioFranco I see. I will split it into another PR for the client's implementation.

[ollyde]

Hey guys is there any status on this? :-)

We can't use gRPC without the ability to log times on requests, it's essential for operations to see which routes are taking a long time.

[LucioFranco]

We can't use gRPC without the ability to log times on requests, it's essential for operations to see which routes are taking a long time.

I don't think there is much progress on this but you should be able to do what you want via the http service layer and tracking the paths which correlate to the grpc method being called.

[ollyde]

@LucioFranco thanks for the reply.
We decided to go with another framework that supports it by default. Might be something to consider, this framework was fairly nice :-)

[CodingAnarchy]

@linw1995 You note that this change doesn't work for the case where the interceptor is made a layer over multiple services. Is there some way that could be done? I'm trying to use an interceptor layer for authentication, but I need it to not run on the health check service.

[linw1995]

@CodingAnarchy You might need to use the tonic-health crate in this PR. This will allow you to get the correct service name in your interceptor.