litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.62k stars 382 forks source link

Enhancement: Add `Sentry` integration for 2.0 #1514

Closed JacobCoffee closed 3 months ago

JacobCoffee commented 1 year ago

Summary

We have a sentry integration for 1.x. We need to update that integration, but we did not write any docs.

Basic Example

Please follow the Sentry Integration Guide with special emphasis on the docs

Per the Sentry Integration Guide:

Write the docs. Answer the following questions:

Drawbacks and Impact

N/A

Unresolved questions

  1. How do we handle the renaming of the current integration per Sentry guidelines?

Fund with Polar

Goldziher commented 1 year ago

I dont think this is an issue for this repository. Sentry is a for-profit company, and we should not contribute to them out of our own charity. Please open an issue on the sentry side of the fence, and they can take it from there - or not.

janluke commented 1 year ago

@provinzkraut Does the current integration for Starlite need major modifications or it's just a matter of renaming starlite with litestar?

janluke commented 1 year ago

I asked the question because we need this (at the company I work for) and we're considering contributing, I just wanted to have an idea on the effort needed, considering that we're new to Litestar and never contributed to Sentry. Thanks.

peterschutt commented 1 year ago

I'm a heavy user of sentry also, so would like to see this done. I'd have to look at the code more thoroughly to comment, but I'd imagine there will be some fairly significant changes required as starlite -> litestar was a bit of a jump.

JacobCoffee commented 1 year ago

It would be welcome and we could help with the transition when needed.

bpereto commented 1 year ago

I tried to adapt the implementation mentioned in https://github.com/getsentry/sentry-python/blob/f570a9966252920bdb221101d596eb029497b0e9/sentry_sdk/integrations/starlite.py#L51

https://gist.github.com/bpereto/8b49893f4a19740c19bf336af94aa95b#file-sentry-py

I can tell that the errors do show up :) but I struggled with the retrieve_user_from_scope.

kedod commented 8 months ago

@bpereto Do you need a hand with this one?

JacobCoffee commented 8 months ago

@kedod you might work with @guacs as well. There is a thread in the development discord channel.

Happy to have anyone help or maybe even just take this if they have the capacity!

bpereto commented 8 months ago

@bpereto Do you need a hand with this one?

I'm not sure how the internals of litestar work, but my implementation attempt seems do it's job.

maybe you know the framework better and how to implement it correctly?

Im unsure if the references are the newest versions.

guacs commented 8 months ago

@kedod if you want to take this, please feel free :)

rafalkrupinski commented 6 months ago

I was expecting adding SentryAsgiMiddleware to the list of middlewares to have at least some effect, but no.

peterschutt commented 6 months ago

The logging integration gives you something, but obviously not ideal as no spans etc.

I spoke with one of their engineers at pycon, and he mentioned they might be exposing some new apis that would help us to build an integration without monkey-patching the application (which would be my preference). I'll be watching that space, but as a sentry user myself, this is a problem I'd like to see solved too.

rafalkrupinski commented 6 months ago

OK, looks like I forgot to configure DSN :D ASGI integration actually does something.

If one was to contribute to the integration:

I like Litestar code, but I know nothing about sentry internals.

scherbakovx commented 3 months ago

Looks like Litestar integration is coming soon: https://github.com/getsentry/sentry-python/pull/3358

JacobCoffee commented 3 months ago

This is done externally in https://github.com/getsentry/sentry-python/pull/3358