feat(tonic): make it easier to add tower middleware to servers

[davidpdrsn]

This is an attempt at making tower easier to use with tonic. It only touches the server parts because I think adding tower middleware to a client is already easy.

The main limitations of the current setup I think are:

1. Tonic's router requires that all services implement NamedService. This is big downside because it means middleware defined in tower cannot be used since tower doesn't know anything about tonic.
2. Users who want to do middleware kind of things, that interceptors don't support, have to know that tower exists and works with tonic.
3. Tower middleware must use http::Request and http::Response to work with tonic. Thats quite surprising since tonic already has its own request and response types.

What I have so far solves 1 and 2. I'm not quite sure 3 has a good solution but might be possible to write a middleware that converts things back and forth behind the scenes.

Using tower now

With this patch adding tower middleware to a tonic server looks like:

let greeter = MyGreeter::default();
let svc = GreeterServer::new(greeter);

// some large layer who's output service _doesn't_
// implement `NamedService`
let layer = tower::ServiceBuilder::new()
    // good old tower middlewares
    .timeout(Duration::from_secs(30))
    .concurrency_limit(42)
    // some custom middleware we wrote
    .layer(MyMiddlewareLayer::default())
    .into_inner();

Server::builder()
    // wrap all services in the layer
    .layer(layer)
    .add_service(svc.clone())
    // also supports optional services
    .add_optional_service(Some(svc))
    .serve(addr)
    .await?;

So Server and Router becomes generic over a tower layer. The layer starts out as Identity but can be changed with the Server::layer method. The layer will be applied before handing the service off to hyper via MakeSvc.

Server::layer will show up in the docs and gives us a place to mention and demo using tower to add middleware. That should hopefully increase awareness a bit.

TODO

Things to fix should be decide to move forward with this.

• Write docs for eveything
• Update interceptor client and server examples
• Remove old intercept API?
• Don't derive Debug for InterceptorFn and InterceptedService
• Update generated with_interceptor constructors to use InterceptedService
• Don't use BoxBody as bounds for services in add_service and add_optional_service as that requires Error = Status.

Fixes #372 #499 #70
Ref #69

[davidpdrsn]

Update: Interceptors can now be added with tonic::service::interceptor_fn:

fn intercept(req: Request<()>) -> Result<Request<()>, Status> {
    Ok(req)
}

Server::builder()
    .layer(tonic::service::interceptor_fn(intercept))
    .add_service(svc)
    .serve(addr)
    .await?;

And since interceptor_fn just returns a tower::Layer they can be composed just like any other tower service.

Edit: Might have gone a bit overboard but I actually went ahead and implemented the old intercept API on top of the new tower interceptors. Still a breaking change of course but I don't think there is much reason to keep both APIs around.

[davidpdrsn]

Converting this back to a draft. Wanna see if I can find a solution to

Don't use BoxBody as bounds for services in add_service and add_optional_service as that requires Error = Status.

That means you currently cannot use middleware that change the body type which quite a few in tower-http does, namely Trace.

[LucioFranco]

Everything looks good to me, I see you still have a few things to check but approving since I trust you to complete those before merging! I love this PR!

[LucioFranco]

should tonic re-export? Or even better could the channel build expose a way to get the servicebuilder?

[davidpdrsn]

I considered that as well but opted not to. Since ServiceBuilder isn't very useful on its own its not clear which other things from tower we would also have to re-export.

I might consider adding some convenience stuff to channel builders but I think I'll do that as a separate PR.

[LucioFranco]

gh issue for this?

[davidpdrsn]

yes!

[drauschenbach]

Doesn't this refactor break the interceptor/client.rs and authentication/client.rs examples?

In my derived example:

error[E0308]: mismatched types
  --> src/mymodule.rs:27:8
   |
27 |     Ok(client)
   |        ^^^^^^ expected struct `Channel`, found struct `tonic::codegen::InterceptedService`
   |
   = note: expected struct `MyClient<Channel>`
              found struct `MyClient<tonic::codegen::InterceptedService<Channel, fn(tonic::Request<()>) -> Result<tonic::Request<()>, tonic::Status> {intercept}>>`

[davidpdrsn]

@drauschenbach indeed it does. That's why it was released as part of tonic 0.5, the previous version being 0.4.x