readmeio / metrics-sdks

SDKs and integrations for ReadMe's Metrics platform
https://readme.com/metrics
10 stars 23 forks source link

Django integration doesn't work for async projects #796

Open reuben opened 1 year ago

reuben commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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 commented 7 months ago

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

gratcliff commented 1 month ago

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.