async-graphql: Tracing extension doesn't work properly when used with async code

Expected Behavior

Should build correct spans tree

Actual Behavior

Inconsistent span trees, some spans are missing parent. Don’t think span.with_subscriber(|(id, d)| d.enter(id)) works in multi threaded runtime

Steps to Reproduce the Problem

Clone this repo https://github.com/ibralu/async-graphql-tracing. I’ve used starwars example with sleep on resolvers
Start jaeger

docker run -d --name jaeger \
  -e COLLECTOR_ZIPKIN_HOST_PORT=:9411 \
  -p 5775:5775/udp \
  -p 6831:6831/udp \
  -p 6832:6832/udp \
  -p 5778:5778 \
  -p 16686:16686 \
  -p 14268:14268 \
  -p 14250:14250 \
  -p 9411:9411 \
  jaegertracing/all-in-one:1.22

Test with query

{
  hero(episode: EMPIRE) {
    id
    name
    friends {
      id
      name
    }
    appearsIn
  }
}

Specifications

Version: 2.6
Platform: macOS
Subsystem: 11.2

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 37 (8 by maintainers)

Commits related to this issue

Fix tracing extension doesn't work with async code. ##448 — committed to async-graphql/async-graphql by sunli829 3 years ago
Fix tracing extension doesn't work with async code. ##448 — committed to async-graphql/async-graphql by sunli829 3 years ago

Most upvoted comments

I have implemented a new extension mechanism on the master branch, which is better than the previous implementation, and I believe this problem has been completely solved. If you have time, please help me test it, thank you. 🙂

sunli829 on Apr 4, 2021

Good news, I have made a lot of progress! 🙂

sunli829 on Apr 3, 2021

I made a lot of changes, and now the Tracing extension is finally working great!

sunli829 on Mar 26, 2021

I understand the problem you encountered. Because of the lack of the current span, tracing::{warn,trace,info...} can’t work at all. I did neglect this problem, thank you for telling me! I will carefully consider how to solve this problem. 🙂

sunli829 on Apr 2, 2021

async fn execute(&self, ctx: &ExtensionContext<'_>, next: NextExtension<'_>) -> Response;

Hells yes! ^_^

D1plo1d on Apr 4, 2021

Tested it out with my project. Works exactly as I expect it to, so far. I will keep testing it and will let you know if there are any issues.

menixator on Apr 4, 2021

I think this issue has been resolved, if you find that it is still incorrect, please feel free to open it.

sunli829 on Apr 2, 2021

Released in v2.6.3

sunli829 on Mar 22, 2021