Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

consider gRPC interceptor support #201

Open
kcking opened this issue Aug 30, 2019 · 7 comments
Open

consider gRPC interceptor support #201

kcking opened this issue Aug 30, 2019 · 7 comments

Comments

@kcking
Copy link

kcking commented Aug 30, 2019

We are looking to write some gRPC services in rust, but it looks like tower-gRPC is missing interceptor support. We currently use interceptors in go-grpc for a few things:

  • automatically log prometheus metrics (latency/payload size) based on each gRPC call
  • add opentracing spans
  • log panics to sentry

I noticed that tower_request_modifier can be used to "intercept" incoming requests, but I was wondering if complete interceptor support would be considered by the tower maintainers.

Thanks!

@LucioFranco
Copy link
Member

@kcking hi thanks for the issue! I have not looked too deep at grpc-go's interceptors yet, is this something that would work at the http2 level aka intercepting http::Request<grpc::Body> and http::Response<grpc::Body>?

I think this is 100% something we want to support but I'm actually curious if this is something that could be added more generically to tower instead of just tower-grpc?

@kcking
Copy link
Author

kcking commented Aug 30, 2019

It looks like grpc-go interceptors, such as the unary server interceptor, inject at the grpc request/response level as opposed to http2 level. Their use of blank interfaces makes this simple type-wise in go, and might prove more complicated/less elegant in rust.

The only potential issue I see so far with intercepting at the http2 level is modifying the metadata of gRPC requests/responses. AFAIU, grpc metadata is almost, but not the same as, http headers (as indicated by the existence of https://docs.rs/tower-grpc/0.1.0/tower_grpc/metadata/struct.MetadataMap.html). I haven't looked into it enough to know if there is a simple workaround or not though.

@LucioFranco
Copy link
Member

So MetadataMap is 1:1 with http::HeadersMap. So that should be easy to provide a tower-request-modifier where you can update the metadata, the issue is the body but I don think you generally want to peak at it.

So as far as I see the use cases for interceptors are:

  • updating some headers/metadata
  • wrapping the unary call
  • wrapping each item in a stream

Are there any other use cases I might be missing?

From what I can tell these should all be accomplishable without actually changing tower-grpc. I'm curious to know how the other grpc libraries achieve this type of feature?

@kcking
Copy link
Author

kcking commented Aug 30, 2019

Good to hear re. Metadata!

Those use cases lgtm. I believe the grpc service and method names can be deduced from request headers.

I think the most popular grpc library is the golang one I linked, I haven't looked into others yet. From what it sounds like you are proposing a tower http middleware to support these features. Does tower have any sort of middleware yet?

@LucioFranco
Copy link
Member

@kcking so I think we could provide a middleware that actually will go from http -> tower-grpc request/responses types then its trivial for a user to implement the same interceptors.

The bigger question right now is do we want to spend time on fixing 0.1 future or just doing this all in futures 0.3? Do you have a direct need with old futures?

@kcking
Copy link
Author

kcking commented Aug 30, 2019

Is that http -> tower-grpc conversion already happening somewhere in tower-grpc? I guess I assumed the type signature of the interceptor would just be fn (http::Request) -> http:Response or fn (tower_grpc::Request) -> tower_grpc::Response

We'd be totally fine with moving right to futures 0.3. Is there a general timeline for 0.3 support in tower?

@LucioFranco
Copy link
Member

@kcking soon for futures 0.3! 😄

So we have these internal fns https://github.com/tower-rs/tower-grpc/blob/master/tower-grpc/src/request.rs#L44 we could provide a Service that does this mapping for you and takes a fn(&mut tower_grpc::Request) and a fn(&mut tower_grpc::Response) so that you can map things.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants