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

Django integration doesn't work for async projects #796

Open
reuben opened this issue Apr 28, 2023 · 4 comments
Open

Django integration doesn't work for async projects #796

reuben opened this issue Apr 28, 2023 · 4 comments
Labels
python Issues related to our Python SDK

Comments

@reuben
Copy link

reuben commented Apr 28, 2023

I don't have time to make a proper PR soonish but here's a version of the middleware which works for both sync and async contexts:

from asgiref.sync import iscoroutinefunction
from django.conf import settings

from datetime import datetime
import time

from django.conf import settings

from readme_metrics.Metrics import Metrics
from readme_metrics.ResponseInfoWrapper import ResponseInfoWrapper
from readme_metrics import MetricsApiConfig


from django.utils.decorators import sync_and_async_middleware


@sync_and_async_middleware
def metrics_middleware(get_response):
    # One-time configuration and initialization goes here.
    get_response = get_response
    config = settings.README_METRICS_CONFIG
    assert isinstance(config, MetricsApiConfig)
    metrics_core = Metrics(config)

    def preamble(request):
        try:
            request.rm_start_dt = datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")
            request.rm_start_ts = int(time.time() * 1000)
            if request.headers.get("Content-Length") or request.body:
                request.rm_content_length = request.headers["Content-Length"] or "0"
                request.rm_body = request.body or ""
        except Exception as e:
            # Errors in the Metrics SDK should never cause the application to
            # throw an error. Log it but don't re-raise.
            config.LOGGER.exception(e)

    def handle_response(request, response):
        try:
            try:
                body = response.content.decode("utf-8")
            except UnicodeDecodeError:
                body = "[NOT VALID UTF-8]"
            response_info = ResponseInfoWrapper(
                response.headers,
                response.status_code,
                content_type=None,
                content_length=None,
                body=body,
            )
            metrics_core.process(request, response_info)
        except Exception as e:
            # Errors in the Metrics SDK should never cause the application to
            # throw an error. Log it but don't re-raise.
            config.LOGGER.exception(e)

    if iscoroutinefunction(get_response):

        async def middleware(request):
            print("async metrics middleware")
            preamble(request)
            response = await get_response(request)
            handle_response(request, response)
            return response

    else:

        def middleware(request):
            print("sync metrics middleware")
            preamble(request)
            response = get_response(request)
            handle_response(request, response)
            return response

    return middleware

Relevant documentation: https://docs.djangoproject.com/en/4.2/topics/http/middleware/#async-middleware

In addition, readme_metrics.Metrics.Metrics tries to access request.environ which is WSGI specific. For my use case, I just replaced it with a patched version which uses request.META instead. For your package I imagine you'll want to handle both WSGI and ASGI gracefully.

@reuben
Copy link
Author

reuben commented Apr 28, 2023

Actually, more changes are needed. The grouping function also needs to be called differently depending on sync vs async context. (Plus PayloadBuilder is also using request.environ).

@djmango
Copy link

djmango commented Sep 19, 2023

I've integrated your patches as well as a few others to make it a drop-in solution, see my fork https://github.com/djmango/metrics-sdks

One thing is that for my use case I don't need sync requests at all, and to simplify things and avoid major rewrite of this package I made the Django integration of my fork async/ASGI only. This can and should certainly expand to work with both ASGI and WSGI, but I don't have the bandwidth today. Open to contribs.

Because of that a PR to master isn't necessarily the most appropriate step today, feel free to just install from my branch if your needs are similar.

pip install git+https://github.com/djmango/metrics-sdks.git@master#subdirectory=packages/python
or
poetry add git+https://github.com/djmango/metrics-sdks.git@master#subdirectory=packages/python --extras=Django

@noah-vetcove
Copy link

Any plans on supporting this new async world? @erunion @domharrington

I'm looking to incorporate this django middleware into our project but we run Uvicorn and are facing this same incompatibility

@erunion erunion added the python Issues related to our Python SDK label Jul 26, 2024
@gratcliff
Copy link
Member

Hi all! Dropping in to say that we're planning on support ASGI in the coming months. Will add a more detailed description in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Issues related to our Python SDK
Projects
None yet
Development

No branches or pull requests

5 participants