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(transport): Add service multiplexing/routing #99

Merged
merged 5 commits into from
Oct 29, 2019
Merged

Conversation

LucioFranco
Copy link
Member

This change introduces a new "router" built on top of
transport::Server that allows one to run multiple
gRPC services on the same socket.

Example

Server::builder()
    .add_service(greeter)
    .add_service(echo)
    .serve(addr)
    .await?;

There is also a new multiplex example showcasing
server side service multiplexing and client side
service multiplexing.

BREAKING CHANGES: Server::serve is now crate private
and all services must be added via Server::add_service.
Codegen also returns just a Service now instead of a
MakeService pair.

Closes #29

cc @rlabrecque @seanmonstar @carllerche

Signed-off-by: Lucio Franco [email protected]

This change introduces a new "router" built on top of
`transport::Server` that allows one to run multiple
gRPC services on the same socket.

```rust
Server::builder()
    .add_service(greeter)
    .add_service(echo)
    .serve(addr)
    .await?;
```

There is also a new `multiplex` example showcasing
server side service multiplexing and client side
service multiplexing.

BREAKING CHANGES: `Server::serve` is now crate private
and all services must be added via `Server::add_service`.
Codegen also returns just a `Service` now instead of a
`MakeService` pair.

Closes #29

Signed-off-by: Lucio Franco [email protected]
@@ -13,7 +13,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let client_key = tokio::fs::read("tonic-examples/data/tls/client1.key").await?;
let client_identity = Identity::from_pem(client_cert, client_key);

let tls = ClientTlsConfig::with_openssl()
let tls = ClientTlsConfig::with_rustls()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this supposed to change in this PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I think it was missed in another PR so I thought i might as well change this now. I want all examples with rustls :)

We really only need openssl for interop since the certs that grpc-go uses are too small for the min that rustls supports.

///
/// This will clone the `Server` builder and create a router that will
/// route around different services.
pub fn add_service<S>(&mut self, svc: S) -> Router<S, Unimplemented>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to take &mut self if it immediately clones self?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you can create multiple routers from one Server config was the idea. It takes a &mut because it follows like the others.

tonic/src/transport/server.rs Outdated Show resolved Hide resolved
tonic/src/transport/service/router.rs Show resolved Hide resolved
tonic/src/transport/service/router.rs Outdated Show resolved Hide resolved
Ok(()).into()
}

fn call(&mut self, _req: Request<Body>) -> Self::Future {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think this should record a tracing event as well? that way, users get some visibility into when requests hit the unimplemented route (in logs, metrics, etc...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ummm probably a good idea, but I'd like to add tracing in another PR.

impl<A, B, Request> Routes<A, B, Request> {
pub(crate) fn push<C>(
self,
predicate: impl Fn(&Request) -> bool + Send + Sync + 'static,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth considering a Predicate trait so that users can add more complex filtering? Probably not in this branch though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, once we get a stable release of tokio, im gonna update a bunch of the tower services and will impl this there.

@LucioFranco LucioFranco merged commit 5b4f468 into master Oct 29, 2019
@LucioFranco LucioFranco deleted the lucio/router branch October 29, 2019 20:32
rabbitinspace pushed a commit to satelit-project/tonic that referenced this pull request Jan 1, 2020
* feat(transport): Add service multiplexing/routing

This change introduces a new "router" built on top of
`transport::Server` that allows one to run multiple
gRPC services on the same socket.

```rust
Server::builder()
    .add_service(greeter)
    .add_service(echo)
    .serve(addr)
    .await?;
```

There is also a new `multiplex` example showcasing
server side service multiplexing and client side
service multiplexing.

BREAKING CHANGES: `Server::serve` is now crate private
and all services must be added via `Server::add_service`.
Codegen also returns just a `Service` now instead of a
`MakeService` pair.

Closes hyperium#29

Signed-off-by: Lucio Franco [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server side service level Routing
3 participants