pydantic / logfire

Uncomplicated Observability for Python and beyond! 🪵🔥
https://logfire.pydantic.dev/docs/
MIT License
2.17k stars 66 forks source link

Django instrumentation silently doesn't work for ASGI requests #472

Open alexmojaki opened 1 month ago

alexmojaki commented 1 month ago

A user couldn't get the Django instrumentation working (https://pydanticlogfire.slack.com/archives/C06EDRBSAH3/p1722460460137619?thread_ts=1722440635.372479&cid=C06EDRBSAH3). With difficulty we realised that it was because they were using ASGI. In opentelemetry/instrumentation/django/middleware/otel_middleware.py you can see that ASGI requests are silently ignored when opentelemetry-instrumentation-asgi isn't installed.

Firstly, we should document this loudly.

Then for people who miss this point, we should either:

  1. Try to detect ASGI requests and warn the user if needed. Not sure how easy this is.
  2. Add a mandatory is_asgi parameter to instrument_django to force the user to tell us the situation, then raise an error if needed.
  3. Always install opentelemetry-instrumentation-asgi with the django extra.
  4. Remove the django extra and replace it with django-wsgi and django-asgi.

@samuelcolvin what do you think?

Kludex commented 2 days ago

Remove the django extra and replace it with django-wsgi and django-asgi.

We can also just add django-async.

Kludex commented 2 days ago

The first step here is to document this.

alexmojaki commented 1 day ago

Remove the django extra and replace it with django-wsgi and django-asgi.

We can also just add django-async.

The idea is that the user has to be aware of the distinction and make an explicit choice. If they don't check docs carefully they may easily think that pip install logfire[django] should just work, they won't get an error at any point.