Skip to content
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

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

Open
jszwedko opened this issue May 5, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@jszwedko
Copy link

jszwedko commented May 5, 2023

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 jszwedko added the bug Something isn't working label May 5, 2023
@seanmonstar
Copy link
Owner

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.

@jszwedko
Copy link
Author

jszwedko commented May 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants