fix: run dns resolution in the same tracing-span as the caller #134
Conversation
This makes it possible to trace the entire request, including DNS resolution. The use case for this is to be able to suppress specific requests from being traced in a situation where the request is used to transmit logs to a remote server. This would result in an infinite loop of logs being sent to the remote server. tokio-rs/tokio#6659 has more details.
|
Ping |
examples/client.rs
Outdated
| @@ -28,7 +29,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> { | |||
| .uri(url) | |||
| .body(Empty::<bytes::Bytes>::new())?; | |||
|
|
|||
| let resp = client.request(req).await?; | |||
| let span = info_span!("request", uri = %req.uri()); | |||
| let resp = client.request(req).instrument(span).await?; | |||
There was a problem hiding this comment.
Could we remove this too?
I agree it's useful when debugging, but I worry it distracts from the initial usage of someone just wanting to get the client working.
|
This is causing: tokio-rs/tracing#3223 (comment) Took me a while to track this down. But reverting this change solves this issue. If you are interested in resolving this, I can think of 2 courses of action:
(2) This would break people depending on |
|
At this point, I'm happy to accept a PR reverting the whole thing. Providing a slightly more accurate trace isn't worth panics. |
I think this would be the right short term approach, but wrong as a long term idea. I think it's important if this is causing panics to revert first and make it possible to work on a replacement that doesn't. I haven't dug into the tracing bug to understand it well enough to see if there's another approach that would work. It might be worth checking with the tracing devs for any obvious workarounds as a third option. |
|
I suggest discussion continue at hyperium/hyper#3870 |
|
@joshka I completely agree. It's an extremely unfortunate sequence of events that:
That's why in #179 I say it can be once again reverted, once Because of this, I'm motivated to make a simple test/dummy repo showing the panic vs the expected behavior. Just spend the last 30 min making it here: https://github.com/arpadav/tracing-span-bug And then I / others can iterate on the |
This makes it possible to trace the entire request, including DNS
resolution.
The use case for this is to be able to suppress specific requests from
being traced in a situation where the request is used to transmit logs
to a remote server. This would result in an infinite loop of logs being
sent to the remote server. tokio-rs/tokio#6659
has more details.