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.

Sign up for GitHub

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

Jump to bottom

Django StreamingHttpResponse View not working as expected #3736

Closed
marcoaleixo opened this issue Nov 5, 2024 · 8 comments · Fixed by #3798
Closed

Django StreamingHttpResponse View not working as expected #3736

marcoaleixo opened this issue Nov 5, 2024 · 8 comments · Fixed by #3798
Assignees
@antonpirker
Labels
Integration: Django

Comments

@marcoaleixo
Copy link

marcoaleixo commented Nov 5, 2024 •
edited
Loading

How do you use Sentry?

Sentry Saas (sentry.io)

Version

2.18.0

Steps to Reproduce

  1. Have a view like
class StreamingView(APIView):

    def post(self, request, *args, **kwargs):

        span = sentry_sdk.start_span(name=f"Log: {record.msg}", op="log.info", sampled=False)
        print(span)
        span.set_data("value", record.msg)
        span.set_data("filename", f"{record.pathname} at line {record.lineno}")
        span.finish()

        generator_streaming = dummy_function.execute(parameters=parameters)
        
        return StreamingHttpResponse(generator_streaming, content_type='text/event-stream')

Here the span.trace_id will be for instance "1234"

  1. Inside the dummy_function
def execute():
        span = sentry_sdk.start_span(name=f"Log: {record.msg}", op="log.info", sampled=False)
        print(span)
        span.set_data("value", record.msg)
        span.set_data("filename", f"{record.pathname} at line {record.lineno}")
        span.finish()

In this function, the trace_id will be totally different.

I think that this is related the StreamingHttpResponse, because the Sentry is working fine for all my other views.

Feel free to ask more details to help in the investigation.

FROM python:3.9.18-slim-bullseye
Django==4.1
djangorestframework==3.14

I'm executing my server with python manage.py runserver 0.0.0.0:8000
In production we use something like gunicorn api.wsgi -b 0.0.0.0:8081

Expected Result

The trace_id should stay the same in StreamingHttpResponse views and underlying function calls.

Actual Result

The trace_id is different between the view and underlying function calls.
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 5, 2024
@marcoaleixo marcoaleixo changed the title Django StreamingHttpResponse not working as expected Django StreamingHttpResponse View not working as expected Nov 5, 2024
@marcoaleixo
Copy link
Author

marcoaleixo commented Nov 5, 2024 •
edited
Loading

Do we have a way to set the trace_id?
Maybe an easy fix is to get the trace_id in the view level and pass as parameter to my function and do a transaction.set_trace_id(trace_id=trace_id).

But it seems that we don't have a set_trace_id thing at transaction/event level, right?

I know that we can do a Span(trace_id=myid), but passing this to ALL calls downstream is bad. 😅

@antonpirker
Copy link
Member

Hey @marcoaleixo thanks for raising this, I will have a look!

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Nov 7, 2024
@antonpirker antonpirker added Integration: Django Type: Bug Something isn't working labels Nov 7, 2024
@antonpirker
Copy link
Member

antonpirker commented Nov 7, 2024 •
edited
Loading

Indeed!

I have now reproduced this with this demo application:
https://github.com/antonpirker/testing-sentry/blob/main/test-django-rest-framework-streaming/mysite/api/views.py

The first span "Streaming View" is present in the trace, but the second span "Stream Response Generator" is missing (because it has the wrong trace id)
Image

@antonpirker
Copy link
Member

This is a bug. I guess it has something to do with how Django handles streaming views and what we monkey patch in Django. Sentry is probably not patching the functions in Django that are handling the streaming responses.

@marcoaleixo
Copy link
Author

marcoaleixo commented Nov 12, 2024 •
edited
Loading

hey @antonpirker - thanks for taking a look but I have a quick question: how do you prioritize those fixes?
This bug is creating a hole in the observability of one of my main endpoints :/

Just to understand if I will need to use another observability tool until you fix this.
I do have some bandwidth to work on this fix but not sure if I have enough context to work on it.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 12, 2024
@antonpirker
Copy link
Member

Hey! I will have some time today and tomorrow to look at this. I will keep you posted on the progress

@antonpirker
Copy link
Member

I tested now with Uvicorn and Gunicorn.

Using Uvicorn (asgi) it works. The parent-child relationship is not correct, but at least the spans are there:
Image

Using Gunicorn (wsgi) it does not work:
Image

@antonpirker
Copy link
Member

Small update: The problem is that StreamingHttpResponse() finishes right away and this leads to the transaction being closed promptly. The spans generated in the generator are outside of the transactions time span and thus not part of that trace and are lost.

@antonpirker antonpirker mentioned this issue Nov 18, 2024
Merged
@stephanie-anderson stephanie-anderson removed the Type: Bug Something isn't working label Nov 21, 2024
antonpirker added a commit that referenced this issue Nov 25, 2024
@antonpirker
Fix spans for streaming responses in WSGI based frameworks (#3798)
da20623 
Fixes spans in streaming responses when using WSGI based frameworks. Only close the transaction once the response was consumed. This way all the spans created during creation of the response will be recorded with the transaction:

- The transaction stays open until all the streaming blocks are sent to the client. (because of this I had to update the tests, to make sure the tests, consume the response, because the Werkzeug test client (used by Flask and Django and our Strawberry tests) will not close the WSGI response)
- A maximum runtime of 5 minutes for transactions is enforced. (like Javascript does it)
- When using a generator to generate the streaming response, it uses the correct scopes to have correct parent-child relationship of spans created in the generator.

People having Sentry in a streaming application will:
- See an increase in their transaction duration to up to 5 minutes
- Get the correct span tree for streaming responses generated by a generator

Fixes #3736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@antonpirker antonpirker

Labels
Integration: Django
Projects
GitHub Issues with 👀 3
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix spans for streaming responses in WSGI based frameworks getsentry/sentry-python
4 participants
@antonpirker @sl0thentr0py @stephanie-anderson @marcoaleixo