Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Enable adding the gRPC API to tonic without serving #428

Closed
maximelenoir opened this issue May 25, 2023 · 2 comments · Fixed by #451
Closed

Enable adding the gRPC API to tonic without serving #428

maximelenoir opened this issue May 25, 2023 · 2 comments · Fixed by #451
Labels
S-feature Severity: feature. This is adding a new feature.

Comments

@maximelenoir
Copy link

maximelenoir commented May 25, 2023 •
edited
Loading

What problem are you trying to solve?

I would like the Server to optionally serve the InstrumentServer after it's been added as a service to the tonic Builder. As of now, the Server implementation starts the aggregator, add the service to tonic and start the gRPC service. Ideally, this last step could be optional, leaving the user the choice of adding more services or using this tonic service in a larger set of services: in my case, I would like to transform the tonic service to a tower service and multiplex it with an Axum service.

How should the problem be solved?

Probably by extending the Service API so that the aggregation is started and the InstrumentService is properly added to tonic, returning a tonic Router. I have a fork that I'm using for production here: 700b80a

There can be a few drawbacks with this:

  • The builder can be configured with a listening port / Unix socket so it might feel surprising to have an API that completely ignores it
  • We lose the fine grained control of the aggregator thread handle (either by dropping it in this new API body or by leaking it to the user)

Any alternatives you've considered?

I did not consider another solution yet.

How would users interact with this feature?

No response

Would you like to work on this feature?

maybe
@maximelenoir maximelenoir added the S-feature Severity: feature. This is adding a new feature. label May 25, 2023
@hds
Copy link
Collaborator

hds commented Jun 14, 2023

I've been thinking about this issue and your use case and I think I have a solution.

I'm going to need a bit more time to work through it though.

I just wanted to let you know that this issue hasn't fallen on deaf ears.

hds added a commit that referenced this issue Jul 18, 2023
@hds
feat(subscriber) expose server parts
ba6b86c 
The `ConsoleLayer` builder provides the user with a console layer and
a server, which is used to start the gRPC server.

However, it may be desireable to expose the instrumentation server together
with other services on the same Tonic router. This was requested
explicitly in #428.

Additionally, to add tests which make use of the instrumentation server
(as part of improving test coverage for #450), more flexibility is
needed than what is provided by the current API. Specifically we would
like to connect a client and server via an in memory channel, rather
than a TCP connection.

This change adds an additional method to `console_subscriber::Server`
called `into_parts` which allows the user to access the
`InstrumentServer` directly. A handle which controls the lifetime of the
`Aggregator` is also provided, as the user must ensure that the
aggregator lives at least as long as the instrument server.

To facilitate the addition of functionality which would result in more
"parts" in the future, `into_parts` returns a non-exhaustive struct,
rather than a tuple of parts.

Closes: #428
hds added a commit that referenced this issue Jul 18, 2023
@hds
feat(subscriber) expose server parts
93eb450 
The `ConsoleLayer` builder provides the user with a console layer and
a server, which is used to start the gRPC server.

However, it may be desireable to expose the instrumentation server together
with other services on the same Tonic router. This was requested
explicitly in #428.

Additionally, to add tests which make use of the instrumentation server
(as part of improving test coverage for #450), more flexibility is
needed than what is provided by the current API. Specifically we would
like to connect a client and server via an in memory channel, rather
than a TCP connection.

This change adds an additional method to `console_subscriber::Server`
called `into_parts` which allows the user to access the
`InstrumentServer` directly. A handle which controls the lifetime of the
`Aggregator` is also provided, as the user must ensure that the
aggregator lives at least as long as the instrument server.

To facilitate the addition of functionality which would result in more
"parts" in the future, `into_parts` returns a non-exhaustive struct,
rather than a tuple of parts.

Closes: #428
@hds hds mentioned this issue Jul 18, 2023
Merged
hds added a commit that referenced this issue Jul 18, 2023
@hds
feat(subscriber) expose server parts
758ba9c 
The `ConsoleLayer` builder provides the user with a console layer and
a server, which is used to start the gRPC server.

However, it may be desireable to expose the instrumentation server together
with other services on the same Tonic router. This was requested
explicitly in #428.

Additionally, to add tests which make use of the instrumentation server
(as part of improving test coverage for #450), more flexibility is
needed than what is provided by the current API. Specifically we would
like to connect a client and server via an in memory channel, rather
than a TCP connection.

This change adds an additional method to `console_subscriber::Server`
called `into_parts` which allows the user to access the
`InstrumentServer` directly. A handle which controls the lifetime of the
`Aggregator` is also provided, as the user must ensure that the
aggregator lives at least as long as the instrument server.

To facilitate the addition of functionality which would result in more
"parts" in the future, `into_parts` returns a non-exhaustive struct,
rather than a tuple of parts.

Closes: #428
hawkw pushed a commit that referenced this issue Jul 28, 2023
@hds
feat(subscriber) expose server parts (#451)
f4536af 
The `ConsoleLayer` builder provides the user with a console layer and
a server, which is used to start the gRPC server.

However, it may be desireable to expose the instrumentation server together
with other services on the same Tonic router. This was requested
explicitly in #449 428.

Additionally, to add tests which make use of the instrumentation server
(as part of improving test coverage for #450), more flexibility is
needed than what is provided by the current API. Specifically we would
like to connect a client and server via an in memory channel, rather
than a TCP connection.

This change adds an additional method to `console_subscriber::Server`
called `into_parts` which allows the user to access the
`InstrumentServer` directly. The `Aggregator` is also returned and
must be set to run for at least as long as the instrument server. This
allows the aggregator to be spawned wherever the user wishes.

To facilitate the addition of functionality which would result in more
"parts" in the future, `into_parts` returns a non-exhaustive struct,
rather than a tuple of parts.

Closes: #428
@maximelenoir
Copy link
Author

Wonderful! Thanks a lot for your work

hawkw pushed a commit that referenced this issue Sep 29, 2023
@hds @hawkw
feat(subscriber) expose server parts (#451)
e51ac5a 
The `ConsoleLayer` builder provides the user with a console layer and
a server, which is used to start the gRPC server.

However, it may be desireable to expose the instrumentation server together
with other services on the same Tonic router. This was requested
explicitly in #449 428.

Additionally, to add tests which make use of the instrumentation server
(as part of improving test coverage for #450), more flexibility is
needed than what is provided by the current API. Specifically we would
like to connect a client and server via an in memory channel, rather
than a TCP connection.

This change adds an additional method to `console_subscriber::Server`
called `into_parts` which allows the user to access the
`InstrumentServer` directly. The `Aggregator` is also returned and
must be set to run for at least as long as the instrument server. This
allows the aggregator to be spawned wherever the user wishes.

To facilitate the addition of functionality which would result in more
"parts" in the future, `into_parts` returns a non-exhaustive struct,
rather than a tuple of parts.

Closes: #428
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
S-feature Severity: feature. This is adding a new feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(subscriber) expose server parts tokio-rs/console
2 participants
@hds @maximelenoir