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

Integration for FastAPI #829

Closed
untitaker opened this issue Sep 22, 2020 · 25 comments · Fixed by #1495
Closed

Integration for FastAPI #829

untitaker opened this issue Sep 22, 2020 · 25 comments · Fixed by #1495
Assignees
Labels
enhancement New feature or request New Integration Integrating with a new framework or library

Comments

@untitaker
Copy link
Member

FastAPI is just Starlette which is just ASGI. We have an ASGI integration that should work fine. However, it would be nice to have deeper integration than we currently provide, particularly around Performance monitoring.

Please vote with 👍 on this post if you're interested in this. I would also like to hear what currently doesn't work out well with the ASGI middleware applied to your FastAPI app.

@untitaker untitaker added the New Integration Integrating with a new framework or library label Sep 22, 2020
@KrunchMuffin

This comment has been minimized.

@untitaker
Copy link
Member Author

@KrunchMuffin you're running into #860, fixed in 0.19.1. I'm collapsing your comment because it's unrelated.

@adg-mh
Copy link

adg-mh commented May 28, 2021

RE: what doesn't work well. I'm trying to work out how to best attach some additional context (user information, from a session) to a sentry event. The user information is per request. Can the middleware accommodate this use case currently?

Edit: As @untitaker pointed out, this should already work as expected. I missed in the docs here the part which reads "Each request has a separate scope. Changes to the scope within a view, for example setting a tag, will only apply to events sent as part of the request being handled."

@untitaker
Copy link
Member Author

untitaker commented May 28, 2021 via email

@ricky-sb
Copy link

particularly around Performance monitoring

What aspect of performance monitoring is missing with the SentryAsgiMiddleware() class? I see that it's tracking transactions, that should give us full performance monitoring, right?

Is there an example of the "gold standard"? For example, is the Django integration more feature-filled?

@untitaker
Copy link
Member Author

untitaker commented May 29, 2021 via email

@lsmith77
Copy link

lsmith77 commented Oct 4, 2021

The Django integration is feature-complete and our gold standard for sure. The main feature that is missing from the ASGI and WSGI Middleware is that the transaction name is not set automatically because the Middleware cannot have knowledge of request routing. But having this information is critical for the product. So you need to set it manually for now.

On Sat, May 29, 2021, 14:13 Ricky @.***> wrote: particularly around Performance monitoring What aspect of performance monitoring is missing with the SentryAsgiMiddleware() class? I see that it's tracking transactions, that should give us full performance monitoring, right? Is there an example of the "gold standard"? For example, is the Django integration more feature-filled? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#829 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGMPRKGDSWGVGBLCWZTA5TTQDK7DANCNFSM4RVQVJNA .

@untitaker can you expand on what we need to do to set this manually?

@untitaker
Copy link
Member Author

@iker-barriocanal iker-barriocanal added the enhancement New feature or request label Oct 14, 2021
@github-actions
Copy link

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@aimfeld
Copy link

aimfeld commented Dec 28, 2021

The setup for ASGI in the documentation didn't work for me, but this worked: https://issueexplorer.com/issue/getsentry/sentry-python/1215

@Olegt0rr
Copy link

@aimfeld,
the same behaviour

it seems as a reason for another issue :)

@juanbenitopr
Copy link

Another thing is not working as expected (at least for me) is the release health dashboard, the support team told me was because the autodetection of the transactions, is it already fixed? or this will be fixed with this integration?

@sl0thentr0py
Copy link
Member

@juanbenitopr we added session tracking for ASGI only in 1.5.5, so maybe upgrade and try out the latest version and it should work.

@juanbenitopr
Copy link

@sl0thentr0py thank you! I will try it :)

@rahulgupta92
Copy link

rahulgupta92 commented Apr 1, 2022

The setup for ASGI in the documentation didn't work for me, but this worked: https://issueexplorer.com/issue/getsentry/sentry-python/1215

@aimfeld I am also not able to integrate Sentry into FastAPI using the ASGIMiddleware solution. Can you please share the other link(or code) which worked for you as the solution link you shared is broken?

@lsmith77
Copy link

lsmith77 commented Apr 1, 2022

for the record this works for me.

from sentry_sdk.integrations.asgi import SentryAsgiMiddleware

app = FastAPI()

# Uncaught exceptions (like `raise Exception`) should propagate correctly
# to Sentry's error handler
# Middleware will also enable Sentry performance monitoring to work as expected
try:
    sentry_sdk.init(
        dsn=settings.sentry_dsn,
        traces_sample_rate=settings.sentry_traces_sample_rate,
        sample_rate=settings.sentry_sample_rate,
        release=version,
        environment=settings.platform_environment,
    )

    app.add_middleware(SentryAsgiMiddleware)
except Exception:
    # pass silently if the Sentry integration failed
    pass

@Kaelten
Copy link

Kaelten commented Apr 12, 2022

For me, when I added the SentryAsgiMiddleware to my fastapi app it resulted in exceptions being thrown on app shutdown. Transaction status was also not set properly due the generalist nature of the middleware (there's a comment in there about how it'd be hard to support)

I ended up using the code from it and a now abandoned open source project to write a custom middleware for Starlette that's based on starlette.middleware.base.BaseHTTPMiddleware. While doing this I was also able to update the associated event processor to work atop starlette.requests.Request instead of the raw ASGI scope. In most cases this allowed reuse of framework code to capture things like the url and headers versus needing custom helpers.

In addition there are two custom error types that FastAPI uses HTTPException and RequestValidationError that would need custom handlers added to the application to capture those issues in sentry. These have been previously mentioned, but it is important to note them since they get swallowed by the framework and do not propagate up to the middleware.

@ruslaniv
Copy link

This issue was created almost two years ago, is there any progress or even a desire for building native Sentry / FastAPI integration?

FastAPI arguably is in top 3 of python web frameworks (based on Github stars, 44K stars just behind Django and Flask) so I think it would be quite popular.

@smeubank
Copy link
Member

perfect timing @ruslaniv! We want to start work on the FastAPI integration next quarter. And have some project kickoff planning this afternoon.

@antonpirker
Copy link
Member

Just a quick update, @sl0thentr0py has started working on this. As FastAPI is a wrapper around Starlette he started doing a Startlette Integration that will be the foundation of the FastAPI integration. Have a look here: #1441

@RamiAwar
Copy link

RamiAwar commented Jul 7, 2022

@sl0thentr0py Note that the SentryASGI Middleware seems to be swallowing Starlette ServerErrors. Had some trouble figuring this out myself, since I was using Streaming Responses and was only seeing RuntimeErrors. Turns out it was because of Sentry's middlware. Check this thread out for more information: encode/starlette#1739

@antonpirker
Copy link
Member

Hey @RamiAwar !
Thanks for the pointer, I will take this into account when I am testing our new Starlette (the base of FastAPI) Integration next week!

@RamiAwar
Copy link

Hey @RamiAwar ! Thanks for the pointer, I will take this into account when I am testing our new Starlette (the base of FastAPI) Integration next week!

Just to clarify even further for the sake of completeness:

Sentry DOES log the error that happens as part of the StreamingResponse, but the server log does not: Server log only logs a Runtime error and uncaught exception, without specifying what it was.

It's pretty weird to replicate. Happened only with Pydantic validation errors that happened inside the generator passed into the streaming response. Did not happen with generic exceptions.

@smeubank
Copy link
Member

initial release will be with manual integration, so we can do some testing in the wild before auto-enabling for all users

@vladanpaunovic vladanpaunovic linked a pull request Jul 19, 2022 that will close this issue
@antonpirker
Copy link
Member

This has now been released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request New Integration Integrating with a new framework or library
Projects
None yet
Development

Successfully merging a pull request may close this issue.