-
Notifications
You must be signed in to change notification settings - Fork 202
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
Consider making tracing optional #402
Comments
Thanks, you make a good point! I'd happily accept a PR; otherwise, I'll see if I can get to this in the next few weeks. |
I've looked into it but |
Hm, yeah that's true—thinking about this again, I think the tracing crate is already decoupled from specific tracers. I expect the core tracing crate, for use by other libraries, is intended to be lightweight enough to not need to be optional.
|
Yeah, so regarding the extra dependencies and compilation overhead, this is going to require significant efforts. One workaround could be writing a dummy module that implements Regarding the runtime and RPC overhead, it looks like tokio tracing won't do anything when there is no subscriber. However, there is a trace_context regardless, so maybe it's just a matter of making this optional when no subscriber is registered? |
Maybe the issue is just that passing around Contexts is too expensive (it's copying like 25 bytes or something)? Do you have any performance profiles of this overhead being a problem? It could potentially be refactored to be heap-allocated and then passed around by pointer, to save like 17 bytes... |
It's not a big overhead but I wish there was a way to send things context-free especially for things like inter-process communication where it's not really warranted. I would say it is more problematic with regards to all the dependencies and compilation time it brings. In particular, it makes this project de facto licensed under |
One way to send things context-free would be to use a transport that converts the context to a traceless type before sending a message, and then just creates a default trace context on the other end? Or is the pain more about having to create the context for each RPC, like #310? |
Yes that could work, but this doesn't solve the fact that all |
While tracing is nice, it adds several dependencies and some overhead to the RPCs.
Ideally this should be a configurable feature so that projects can choose whether to include it or not.
The text was updated successfully, but these errors were encountered: