Make the tracer a global resource #27
Conversation
Time to move to std futures
While the old style of per-Tracer span tracking was conceptually clean, in practice it was just a bunch of extra work to thread Tracers around everywhere that needed them. Like `log`, an application realistically has a single trace consumer so the API can move away from objects to functions.
| @@ -1,6 +1,5 @@ | |||
| [workspace] | |||
| members = [ | |||
| "futures-zipkin", | |||
There was a problem hiding this comment.
but there's still a standard approach to maintaining the correct spans across async things?
There was a problem hiding this comment.
Yeah - it's in the zipkin crate directly as the OpenSpan::bind method.
zipkin/src/test.rs
Outdated
| let _ = crate::set_tracer(AlwaysSampler, TestReporter, Endpoint::builder().build()); | ||
| SPANS.with(|s| s.borrow_mut().clear()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn detach() { |
There was a problem hiding this comment.
worth additionally quickly exercising the bind/attach stuff?
There was a problem hiding this comment.
Will do - we're a bit test-deficient right now.
sfackler
force-pushed
the
global-tracer
branch
from
5adf007 to
2c63de7
Compare
| } | ||
| } | ||
|
|
||
| pin_project! { |
There was a problem hiding this comment.
(read through https://doc.rust-lang.org/beta/std/pin/index.html but still a bit fuzzy on this stuff)
is the following roughly correct?
the generated .project() just returns a 'projection' of Bind like
struct ProjectedBind<T> {
span: OpenSpan<Detached>,
future: Pin<T>
}and then this all seems pretty innocuous because we're not doing anything fishy in drop
also I guess previously you had mentioned that pin-project was pretty heavyweight - the -lite moves the needle enough to just use it?
There was a problem hiding this comment.
Right yeah. The rough idea with projections is that when you have a Pin<&mut MyType>, you need to decide if you want to be able to "project" the Pin down into some or all of MyType's fields. If you do, then there are certain requirements with what you can do with those fields.
pin-project handles all of those safety requirements for you by having you flag the fields you want to pin or not pin, but it's a procedural macro so needs to pull in all of the infrastructure for those. In practice that's not really a problem since you're almost certainly going to be using some other proc macro like #[derive(Serialize)] or whatever, but since this is a library we don't want to make that choice.
pin-project-lite does the same thing as pin-project, but as a macro_rules macro so it doesn't have any dependencies. It doesn't handle as many cases, but luckily it's good enough for what we need.
There was a problem hiding this comment.
The projected struct actually looks like this:
struct ProjectedBind<'a, T> {
span: &'a mut OpenSpan<Detached>,
future: Pin<&'a mut T>,
}
While the old style of per-Tracer span tracking was conceptually clean,
in practice it was just a bunch of extra work to thread Tracers around
everywhere that needed them. Like
log, an application realisticallyhas a single trace consumer so the API can move away from objects to
functions.
I've also folded in an upgrade to the 2018 edition and moved futures support to the zipkin crate itself.