fix(asgi): set http status when exception raised

[majorgreys]

Description

When exception is raised by app call, the span for the ASGI request should have an HTTP status code set. This can be customized by a user of the library to update spans based on the application's exceptions.

Checklist

• Entry added to CHANGELOG.md.
• Tests provided; and/or
• Description of manual testing performed and explanation is included in the code and/or PR.
• Library documentation is updated.
• Corp site documentation is updated (link to the PR).

[Kyle-Verhoog]

Is the response code guaranteed to be 500 by the server if an error occurs? Just want to be sure that we're not going set a 500 status code when another response code could be returned.

[raphaelauv]

I have exactly this problem I had to intern the middleware.py and edit it to remove the span.finish() from the finally :

try:
    await self.app(scope, receive, send_with_tracing)
except BaseException as exc:
    span.set_traceback()
    raise exc from None
span.finish()

Since I have a Wrapper managing the status to raise my custom error code ( 500 or 503 ... )

from .middleware_ddtrace_asgi import TraceMiddleware
from ddtrace.ext import http as http_tags
from starlette.responses import Response
from starlette.types import Receive, Scope, Send
from fastapi.requests import Request

class ExceptionTraceMiddleware(TraceMiddleware):

    def __init__(self, **kwds):
        super().__init__(**kwds)

    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
        if scope["type"] != "http":
            return await self.app(scope, receive, send)

        request = Request(scope=scope, receive=receive)
        try:
            await super(ExceptionTraceMiddleware, self).__call__(scope, receive, send)
        except errors.ApiError as exc:  # only our case
            response: Response = handle_api_error(request, exc)
            span = self.tracer.current_span()
            if span:
                span.set_tag(http_tags.STATUS_CODE, response.status_code)
                span.finish()
            await response.__call__(scope, receive, send)

[majorgreys]

@raphaelauv By the looks of this code, I am guessing you are using the ddtrace-asgi library. But I think your solution is in line with what I have contributed here since the tag has to be set before the span is finished.

[raphaelauv]

Off course I'm using ddtrace-asgi :

I had to intern the middleware.py and edit ... !

and take a look to the definition of my custom class , it's extending TraceMiddleware.

And No your solution is not equivalent because your putting 500 in all case and do not provide any mechanism to custom that value in case of error/exception.

[majorgreys]

Thank you for highlighting this. I've added a static method on the middleware that can be overridden by a subclass to allow custom handling of any exceptions raised by the application.

https://github.com/DataDog/dd-trace-py/pull/1645/files#diff-61d5c837e13077f7c9d719d4cd911a59R79-R82

Hopefully that addresses your concerns.

[raphaelauv]

Maybe people would like to use the request that generated the error.

It's the default behavior in fastAPI for a custom error handler

something like :

def handle_exception_span(request, exc, span): 

[majorgreys]

In this generic ASGI middleware, we do not have access to a request/response interface as you would find in something like with a Starlette middleware's dispatch method.

We are working now on support for a Starlette middleware, where we could certainly provide library users a way to accomplish this.

[raphaelauv]

Okay it make sense , thank you for your answer.

[Kyle-Verhoog]

IMO I'd prefer to pass a function to the init of the middleware rather than having to subclass TraceMiddleware. Especially since the method is static, there's really no reason to have it on the class. Otherwise looks good to me!