Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from

Conversation

davidpdrsn
Copy link
Member

@davidpdrsn davidpdrsn commented May 16, 2021

Edit: Superseded by #651


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 becomes generic over a tower layer. The layer starts out as Identity but can be changed with the Server::layer method. All added services will be wrapped in that layer's service.

The way I get around the NamedService limitation is with a new middleware called Named (please bikeshed the name). Named has two generic type parameters:

  • One from the original service before it had our layer applied. This service will implement NamedService and is used when implementing NamedService for Named.
  • The second type is the wrapped service itself. This wont implement NamedService but thats fine since we get that name from the first type.

So a full type for Named would be something like Named<GreeterServer<MyGreeter>, Timeout<GreeterServer<MyGreeter>>>. Where the name comes from GreeterServer<MyGreeter> but the actual service used to handle requests is Timeout<GreeterServer<MyGreeter>>.

This is also nice because 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.

The downside to this approach is that it complicates the types quite a bit and the type errors you get when things don't align are pretty bad. But thats hard to avoid when doing anything non trivial with tower anyway 🤷

@davidpdrsn
Copy link
Member Author

Superseded by #651

@davidpdrsn davidpdrsn closed this May 16, 2021
@davidpdrsn davidpdrsn deleted the david/tower-server-integration branch May 16, 2021 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant