tarpc
tarpc copied to clipboard
Creating a context for each call is tedious
Adding context::current()
to every RPC call isn't too fun. What if we added an option to tarpc::service
such that each Context
parameter was tucked under the hood?
Thanks for the suggestion! I'm sympathetic to this point of view, but the problem is that the best default for a context would do things that context::current()
doesn't do:
- a context's trace context should inherit the caller's trace ID and have the caller's span as the parent span.
- the deadline should be the caller's deadline minus now().
I would love for this to be the behavior of context::current()
. If it were, I'd have fewer qualms about having it done implicitly. The problem is I am not aware of a good request-local library. Thread locals are insufficient because a request handler can spawn multiple futures that run on different threads (or indeed spawn regular threads in serving a request).
Other options:
- You could also alias
context::current
, e.g.use tarpc::context::current as cc;
- Could you have an extension trait impl'd by the client that adds rpc methods that set up a context implicitly?
The extension trait (with #[async_trait]
) is what I'm doing now. It works, but just wanted to offer the feedback, because in hindsight, I think I'd rather not give the user that level of control initially. If they needed it, I'd want to add it incrementally (pay for what you use).
The decreasing deadline doesn't feel like a critical invariant. Instead, I'd expect the timeout would generally need to increase with the number of network hops.
It seems like the concerns are related to the situation where a server makes rpc calls to another server. In the scenario where that's not the case (which is how I use it) using context::current
for everything seems fine. To handle this scenario, could the service macro generate a second client struct implementation like HelloClientDefaultContext
(there's probably a better name) that could be used if you're fine with a default context.
Edit: Maybe SimpleHelloClient
is a better name
@drewkett apologies for never responding to your suggestion! I could potentially see value in an opt-in simple client — I wouldn't want it to be generated by default since it's more codegen that some users won't need.
One alternative would be an alias for context::current
, like:
use tarpc::context::current as c;
then you could write something like client.hello(c(), "drewkett")
?