feat: add asgi integration

[sadipgiri]

The PR adds an ASGI middleware to provide tracing. The middleware is based on a community contributed ddtrace-asgi by @florimondmanca.

The middleware can be used to manually instrument requests for any ASGI web framework, e.g. Starlette, FastAPI, Quart, etc.

(Edit by @florimondmanca: Closes florimondmanca/ddtrace-asgi#41)

[Kyle-Verhoog]

Great start @sadipgiri 😄! We're basically there in terms of the core functionality! I caught a few things and have some questions about others.

Some higher-level discussion items that also came to mind:

1. Supporting ASGI 2.0 apps

ASGI 3.0 was released 2019-03-04. The spec mentions that servers (and I assume middleware as well) should be 2.0 compatible in the short-term with plans to phase it out.

2. Auto instrumentation with ASGI supporting integrations?

It's probably just Django and maybe Flask that are integrations we have that also support ASGI. Is there a way in which we can automatically install the middleware? Django's docs demonstrate using ASGI middleware, but not sure if it can be automated.

3. Integration testing with our existing asgi-compatible integrations

Again, it's probably just Django and maybe Flask that support ASGI already. Is it worth doing some testing with these integrations with the ASGI middleware installed to make sure the traces are sound?

[Kyle-Verhoog]

We're definitely going to want to test with more variations of scope.

[sadipgiri]

👋 @Kyle-Verhoog just addressing your big points:

1. Supporting ASGI 2.0 apps

Thank you for pointing this out. We are addressing this using guarantee_single_callable function from asgiref.compatibility which will make sure it's always single callable and adds backwards compatibility for ASGI 2.0 apps to our Middleware. We also vendored it, this is how we did it. Would love to see what you think? 🙏

2. Auto instrumentation with ASGI supporting integrations?
3. Integration testing with our existing asgi-compatible integrations

Yes, these are future works.

[brettlangdon]

1. Supporting ASGI 2.0 apps

Do we have a test case that validates we support ASGI 2.0?

[majorgreys]

Do we have a test case that validates we support ASGI 2.0?

@brettlangdon this is now been added with 698c0c5#diff-83f065c2d43e2ccd12c64fc8ffd0aab0R82

[Kyle-Verhoog]

👍 👍 just a couple smaller things and one bigger thing about context propagation.

Also, can we add an app to https://github.com/Datadog/trace-examples? It'd be helpful in the future to have something to spin up to manually test the integration 🙂

[Kyle-Verhoog]

🙌 great work @sadipgiri @majorgreys!!

[florimondmanca]

Looks ✨ awesome ✨, very exciting!

Looking forward to deprecating https://github.com/florimondmanca/ddtrace-asgi in favor of this official solution. 😄

Left a bunch of geeky nits for what they're worth. Great work!

[florimondmanca]

Went ahead and updated PR description to automatically close florimondmanca/ddtrace-asgi#41 once this is merged :-) (community member was asking whether moving the middleware to the official dd-trace-py project was planned)

[florimondmanca]

Also a note for possible future work related to #1567 (comment):

IIUC this PR uses GET {requested_path} as the resource name.

In florimondmanca/ddtrace-asgi#27 I received feedback about being able to use the route pattern (eg /users/{user_id}) as the resource name instead of (or maybe alongside) the actual requested URL path (eg /users/1234).

How to get this information depends on the framework - it's possible in Starlette/FastAPI (see a proof-of-concept solution in florimondmanca/ddtrace-asgi#28), but maybe not be in other frameworks, etc.

From using ddtrace-asgi in production myself for a while (I don't anymore), I can relate that this is most likely a relevant use case, though it's not clear to me yet how this would be best handled, both from an API and implementation perspective.

My initial idea in florimondmanca/ddtrace-asgi#32 was to provide a more general solution which would be "allow users to arbitrarily modify the span pre/post request" - in other words, "span customization". It would also introduce a notion of "backend" for framework-specific functionality. Proof-of-concept in: florimondmanca/ddtrace-asgi#35.

This would allow solving the "route pattern as resource" use case (see snippet in issue description of florimondmanca/ddtrace-asgi#32 for a Starlette example), but also allow users to do whatever specific processing they'd need.

Looking back at it, the proposed API does look a bit over-engineered, and maybe there are alternative APIs we could think of (eg see snippet in florimondmanca/ddtrace-asgi#27 (comment)).

Still, my intuition is still that a general "customize span pre(/post)-request" API would be better for the project than a very specific solution for the "route pattern as resource" use case. But this could also just be a major case of YAGNI, so…

Anyway, just wanted to hand over the feedback and discussions from florimondmanca/ddtrace-asgi#27 since this might be something you could get as well once this initial tack at ASGI tracing is released and people use it in prod. :-)

[Kyle-Verhoog]

@florimondmanca thanks for hopping in! Yeah resource naming is tricky across all our integrations. Long-term I think it's really something that should be handled by the backend. Until that day comes however, it would fall onto the underlying web framework to add the appropriate resource. The ASGI span would be accessible from web frameworks via tracer.current_root_span() which would allow the resource to be modified downstream by the framework.

[florimondmanca]

👍 on my side :-)

[Kyle-Verhoog]

Adding these back since we want to match the functionality as closely as possible

[jcwilson]

Awesome!