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

Server side service level Routing #29

Closed
rlabrecque opened this issue Oct 3, 2019 · 5 comments · Fixed by #99
Closed

Server side service level Routing #29

rlabrecque opened this issue Oct 3, 2019 · 5 comments · Fixed by #99
Labels
A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@rlabrecque
Copy link

rlabrecque commented Oct 3, 2019

Tracking issue for request Routing

Tonic should allow a single socket/server to serve multiple services. Routing is the way to get there.

Current progress: https://github.com/hyperium/tonic/blob/master/tonic-interop/src/bin/server.rs#L73

See also: tower-rs/tower-grpc#2

@LucioFranco LucioFranco added the C-enhancement Category: New feature or request label Oct 3, 2019
@LucioFranco
Copy link
Member

@rlabrecque thanks for opening this!

We've had some ideas there are two that stand out to me currently:

  1. Use a macro to generate the code that looks similar to the one in the interop server.
Server::builder()
	.serve(addr, route! {
		"test_service" => TestService::default(),
		"unimplemented_service" => UnimplementedService::default()
	})
	.await?;
let routed_svc = tonic::Router::new(TestService::default())
	.with(UnimplementedService::default())

Server::builder()
	.serve(addr, routed_svc)
	.await?;

This one is more of an idea, and I'm not sure how to accomplish it without a ton of boxing 😢

@LucioFranco LucioFranco added A-tonic E-help-wanted Call for participation: Help is requested to fix this issue. labels Oct 4, 2019
@rlabrecque
Copy link
Author

Just wanted to throw in my use case for this:

We have a pretty decent REST setup in go, and one thing that I'm missing with tower-grpc is routing. grpc-router above is working alright for my needs now, but I don't think it's going to scale to our REST setup.

The biggest part missing would be the ability to add routes later, and then get a list of the routes.

Here's what I'd want to write:

    let router_service = Router::new(
        GreeterService::new(),
        PingService::new(),
        SomeOtherService::new(),
    );

    // Add the APIListService and HealthCheckService after we created the router_service, and pass
    // it the router_service so that we can get a list of all of the services.
    let api_list_service = APIListService::new(&router_service);
    router_service.add(api_list_service);

    let healthcheck_service = HealthCheckService::new(&router_service);
    router_service.add(healthcheck_service);

    let mut server = Server::new(router_service);

    // Bind server to http2 server and continue as normally
    // ...
#[derive(Clone, Debug)]
pub struct APIListService {
    services: Vec<std::string::String>
}

impl APIListService {
    pub fn new(router: &Router) {
        server::APIListServer::new(APIListService { services: router.get_routes() } )
    }
}

Copied from: tower-rs/tower-grpc#2 (comment)

@LucioFranco
Copy link
Member

@rlabrecque what is your use case around needing to add services at a later point?

@nytopop
Copy link

nytopop commented Oct 17, 2019

As I understand it, so that the api_list_service can:

  • have access to all the services registered before it
  • be served on the same server
  • not expose itself, or the healthcheck_service in api list reqs

Although this could be done without involving the router like the following (although it's not super ergonomic):

let greeter = GreeterService::new();
let ping = PingService::new();
let other = SomeOtherService::new();
let api_list = ApiListService::new()
    .with::<GreeterService>()
    .with::<PingService>()
    .with::<SomeOtherService>();
let healthcheck = HealthCheckService::new()
    .with::<GreeterService>()
    .with::<PingService>()
    .with::<SomeOtherService>()
    .with::<ApiListService>();

// boxed version
let router = Router::new()
    .with(greeter)
    .with(ping)
    .with(other)
    .with(api_list)
    .with(healthcheck);

// static version
let router = router! {greeter, ping, other, api_list, healthcheck};

let mut server = Server::new(router);

// Bind server to http2 server and continue as normally
// ...

So long as every service could expose its own routes. I've written a quick PoC that allows that via a trait and associated consts, something like:

pub mod router {
    // every MakeService gets a generated impl for:
    pub trait Routed {
        const PREFIX: &'static str;
        const ROUTES: &'static [&'static str];
    }
}

@LucioFranco
Copy link
Member

@nytopop That makes sense that was what I was kinda thinking. We can totally add that trait and implement it for codegen. Each async trait impl can include that.

LucioFranco added a commit that referenced this issue Oct 28, 2019
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 luciofranco14@gmail.com
LucioFranco added a commit that referenced this issue Oct 29, 2019
* 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 #29

Signed-off-by: Lucio Franco luciofranco14@gmail.com
rabbitinspace pushed a commit to satelit-project/tonic that referenced this issue 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 luciofranco14@gmail.com
brentalanmiller pushed a commit to brentalanmiller/tonic that referenced this issue Oct 6, 2023
Before this PR, this phrasing appears to have been copied from
the [rust project](https://github.com/rust-lang/rust/tree/f0fe716dbcbf2363ab8f929325d32a17e51039d0#license),
but it does not appear that any of your code is BSD licensed.
Also, it is a little ambigious if there are portions that are not
covered by MIT/Apache.

This patch instead draws it's phrasing from
[futures-rs](https://github.com/rust-lang-nursery/futures-rs). Does
this phrasing align  more with what you intended?

Closes hyperium#29
brentalanmiller pushed a commit to brentalanmiller/tonic that referenced this issue Oct 6, 2023
* Remove futures-core

* Upgrade Tokio 0.3

* clean code

* Fix ci

* Fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tonic C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants