Feature request: Sentry integration

[dsferruzza]

Hi!

There are some cases where PostgREST can return a HTTP 500 response, for example when a stored procedure raises an exception (on purpose or not). In general (in apps I develop), I tend to deal with these unhandled exceptions by adding an integration to Sentry: it is an open source and SaaS product that ingests exception events (augmented by the integration to provide more context), aggregates them and notifies maintainers. (In my opinion, it is almost inconceivable to operate a real-world production system without this kind of tools; it is so useful 💜.)

Do you think this could be relevant to integrate Sentry in PostgREST, so that, if a Sentry DSN was provided in PostgREST's configuration, any failure (either from PostgreSQL or PostgREST itself) would be sent to Sentry?

---

Additional notes:

1. There seems to be a relatively up-to-date package in Hackage to play with Sentry: https://hackage.haskell.org/package/raven-haskell
2. I believe this could also be achievable in the reverse proxy (for example, with OpenResty + raven-lua) but with a poor access to context (for example, it would be harder to report which user was authenticated)

[steve-chavez]

Hey @dsferruzza.

I see the convenience of the Sentry integration. We've had a similar request on gitter chat and the recommendation was to use a log collector(logstash/fluentd) to send the logs to Sentry from there.

That being said we've just refined our logging(#1604). We could see how that plays out with raven-haskell.

A PR would be welcomed :). I haven't used raven-haskell and perhaps the change is simple.

[dsferruzza]

Hi!

Using a custom log collector that sends errors to Sentry could indeed work, and provide value.
This might be a better approach than doing this in the reverse proxy (even if more complex to setup, I guess) because it would not be on the critical path and might have more detailed error messages.

But from my perspective, the main point of having a native integration is to be able to provide more context in Sentry errors (for example, which user was authenticated, or maybe what queries did PostgREST already send to the database).

For example, a year ago I developed a tiny tool called Sentry Process which allows you to run any CLI process and report to Sentry if it does not exit with code 0. It works, it saved my ass in situations where I had to operate a closed-source binary that did not integrate Sentry natively, but reports are very poor because they can only leverage external outputs whereas internal context is unreachable (no stacktraces, …).

That being said, raven-haskell sure might help (if not, the Sentry protocol does not seem very complex…) but I have no idea how simple or complex it could be in PostgREST to access any context that could help in error reports from the scope where Sentry reports would be sent.
In other words, if you could serialize and append these context items in every log output, I would be pretty confident that providing a Sentry integration is actually quite easy 🤞

Does this make sense to you?
You know this codebase (I really don't), does this seem achievable?

[evelant]

@dsferruzza I'm interested to know if you ever found a solution for this. I'm using Supabase so I don't have a traditional server between my app and postgres. I landed here searching for a way to add some tracing or error logging since supabase uses postgrest as a primary interface for the db.

[dsferruzza]

@dsferruzza I'm interested to know if you ever found a solution for this. I'm using Supabase so I don't have a traditional server between my app and postgres. I landed here searching for a way to add some tracing or error logging since supabase uses postgrest as a primary interface for the db.

Unfortunately no, I do not have a solution 😐

[jbstewart]

I would also welcome integration with Sentry!

[taimoorzaeem]

This seems like a really cool feature to have on PostgREST, let's make this happen 🚀. @steve-chavez Here's my initial implementation proposal:

Implementation

I am thinking of adding a configuration say sentry-dsn which when set, would send critical (startup and db connection) and error (status >= 500) events to sentry via raven-haskell service. This should disregard whatever level log-level is set on.

Testing

Since, we don't have any sentry test DSN, I am thinking of testing this feature via a mock DSN. We can do this in our spec-tests by implementing something similar to this using haskell. This way we could test sentry integration without an active test DSN.

Let me know your thoughts!

[steve-chavez]

Implementation

@taimoorzaeem Is it possible to do the sentry integration using the observation module as described on #3140 (comment) ? (this one is for OTel, but same idea applies). We need to ensure this new logging is out of the fast path so performance is not impacted, plus adding sentry support this way would be more maintainable.

[steve-chavez]

Additionally, it seems that Sentry now uses OpenTelemetry under the hood? https://docs.sentry.io/platforms/javascript/guides/node/opentelemetry/

So maybe adding OpenTelemetry would also solve this? Or does native sentry integration (with raven-haskell) has other features? 🤔

Maybe someone that has more knowledge of Sentry can comment.

[wolfgangwalther]

Additionally, it seems that Sentry now uses OpenTelemetry under the hood? https://docs.sentry.io/platforms/javascript/guides/node/opentelemetry/

So maybe adding OpenTelemetry would also solve this? Or does native sentry integration (with raven-haskell) has other features? 🤔

Maybe someone that has more knowledge of Sentry can comment.

I think that only applies to tracing, but not necessarily to error logging. I use Sentry, but I still don't have a clue.. so I might be wrong :)

[taimoorzaeem]

@taimoorzaeem Is it possible to do the sentry integration using the observation module as described on #3140 (comment) ? (this one is for OTel, but same idea applies).

Yes, I think it would be possible, but I can't know for sure until I begin the implementation (issues come up unexpectedly). I guess we can start working on Sentry once #3140 get merged.

We need to ensure this new logging is out of the fast path so performance is not impacted, plus adding sentry support this way would be more maintainable.

Agreed.

[steve-chavez]

I think that only applies to tracing, but not necessarily to error logging. I use Sentry, but I still don't have a clue.. so I might be wrong :)

Cool, then I guess we're fine in adding dedicated support for Sentry.

I am thinking of adding a configuration say sentry-dsn

This config sounds good. I was thinking of adding a prefix, but can't think of a good one.

I guess we can start working on Sentry once #3140 get merged.

@taimoorzaeem Hm, waiting on #3140 shouldn't be needed. You can add a Sentry module and integrate it with AppState.

[taimoorzaeem]

@steve-chavez Without a "real" sentry DSN, I couldn't figure out a way to test this feature where we can be absolutely sure that the logs are being sent to sentry. raven-haskell is pretty limited and does not provide a way to test the integration. Any ideas?

[steve-chavez]

Maybe we can test it at the Github CI level? Not sure if this would help https://github.com/getsentry/sentry-github-actions-app

(Idea would be to add a secret on the repo, then check the output of a CI job that links to sentry)

[taimoorzaeem]

@steve-chavez Yes, this could work if we can have Sentry for PostgREST by signing up here? https://sentry.io/for/open-source/