fix(tracing): propagate span context

[alpeb]

Hi Sean! 👋

The tracing span context set in consumers of the TokioExecutor is sometimes lost (if the task is run in a different thread). This change adds a call to in_current_span() to preserve it.

As a side-effect, it adds a dependency on tracing for the tokio feature. Should I instead allow the new behavior only if tracing is enabled? Or use a separate TracingExecutor for this?

[seanmonstar]

Interesting, I remember something like this a while ago. I think an issue when doing this automatically is that some tasks that are spawned live longer than the initial context. For example, if a new connection is created when trying to serve a request, and then that connection is put in a pool and used for later requests, the context is no longer the same.

[olix0r]

if a new connection is created when trying to serve a request, and then that connection is put in a pool and used for later requests, the context is no longer the same.

My understanding is that this isn't a concern for HTTP/2--connection establishment occurs separately from request dispatch; IIUC, the HTTP/1 connection pooling behavior was changed in the 1.0 refactor... what would be the proper way to audit this? Presumably we don't want to look for uses of executor as much as we want to look into HTTP/1 connection reuse?

I'm especially interested in making this change so that libraries like kube-rs (cc @clux) can get this behavior without us having to go submit patches to every library that builds a client...

[seanmonstar]

I'm happy to include something like this. Are you saying that if we made this a wrapping TracingExecutor<E>, that wouldn't work with the libraries you depend on?

We could alternatively just use #[cfg(feature = "tracing")] on blocks inside the function.

[olix0r]

Are you saying that if we made this a wrapping TracingExecutor, that wouldn't work with the libraries you depend on?

It would require upstream changes, but I think that would be an acceptable middle-ground (at least we can document the trade offs in the util crate).

We could alternatively just use #[cfg(feature = "tracing")] on blocks inside the function.

I like this approach because it means that applications can control the behavior independently of the underlying libraries; but maybe it makes sense to force library authors to make a choice? I can see both sides of that.

[seanmonstar]

Alright, let's do that one: use #[cfg(feature)] and #[cfg(not(feature))] blocks inside the function, and don't enable the dependency automatically. I think a tracing feature will need to be added that would enable it.

[seanmonstar]

Thanks!