tonic
tonic copied to clipboard
tonic: inject an GrpcMethod extension value to request
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())
// ...
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
@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.
@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.
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.
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.
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.
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.
@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 :-)
@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.
@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.