warp icon indicating copy to clipboard operation
warp copied to clipboard

`recover` handlers seem to not be executed in the tracing context added by a `warp::trace` filter

Open jszwedko opened this issue 2 years ago • 2 comments

Version

├── async-graphql-warp v5.0.7
│   └── warp v0.3.5
├── warp v0.3.5 (*)

Platform

❯ uname -a Darwin COMP-J4C4P27K9Q 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000 arm64

Description

Hey all!

It looks like the warp::trace filter doesn't propagate the span information to recover handlers:

If we look at:

https://github.com/vectordotdev/vector/blob/a9c8dc88ce7c35b75ab3d1bf903aca0a6feaee53/src/sources/util/http/prelude.rs#L161-L172

The log that is eventually emitted lacks span information that is added here:

https://github.com/vectordotdev/vector/blob/a9c8dc88ce7c35b75ab3d1bf903aca0a6feaee53/src/sources/util/http/prelude.rs#L158

The span context is present in the request handlers.

Is that expected? Or am I maybe missing something?

jszwedko avatar May 05 '23 19:05 jszwedko

It looks like you have the trace layer wrapping svc, and then combine that with ping, and then add a recover. So the recover is outside the trace layer.

seanmonstar avatar May 06 '23 09:05 seanmonstar

Aha, yeah, I see. Thanks for pointing that out @seanmonstar . I tried moving it below the .or(ping) but it gave me an odd error:

error: implementation of `Reply` is not general enough
   --> src/sources/util/http/prelude.rs:77:12
    |
77  |           Ok(Box::pin(async move {
    |  ____________^
78  | |             let span = Span::current();
79  | |             let mut filter: BoxedFilter<()> = match method {
80  | |                 HttpMethod::Head => warp::head().boxed(),
...   |
196 | |             Ok(())
197 | |         }))
    | |__________^ implementation of `Reply` is not general enough
    |
    = note: `&'0 str` must implement `Reply`, for any lifetime `'0`...
    = note: ...but `Reply` is actually implemented for the type `&'static str`

I'll keep playing with it. Thanks for the tip! I'll close this once I'm able to get it working.

jszwedko avatar May 08 '23 14:05 jszwedko