-
Notifications
You must be signed in to change notification settings - Fork 23
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
Improve sentry integration #274
Comments
Just a wild guess: https://github.com/getAlby/lndhub.go/blob/main/main.go#L113 According to the go echo guide you should first init sentry, then add all the middlewares, and then the sentryecho middleware. https://docs.sentry.io/platforms/go/guides/echo/ @kiwiidb Do you think that could potentially be the issue here? Also I don't really get the |
Currently there is no performance integration that can easily be added and would allow you to see query times, response times, etc. There are 2 methods to do that right now:
See the docs for more info |
do we still need this? opentelemetry is now in with datadog, isn't it? |
I just set up Sentry and added the
Since someone above mentioned potential issues with how Sentry is integrated into the code, I thought I'd put this question here instead of opening a new issue. |
Logs are not sent to Sentry, only errors / unhandled exceptions currently. Would you want to see this particular error in Sentry, too? Currently only some of the errors seem to be captured: https://github.com/getAlby/lndhub.go/blob/main/lib/service/checkpayments.go#L77 The issue itself is about integrating more detailed logs about e.g. database queries and such, the general integration should work fine. |
OK, thanks.
Yes, I think most, if not all, errors should be sent, because as an operator, you can ignore specific ones on the Sentry side and only keep the ones you care about. I haven't looked into the new performance tracking features of Sentry yet, but those could also be interesting for improving database queries, indexes and such. |
I found another thing that would be useful to improve: Currently, there is no environment set for Sentry. I would propose to set |
That would certainly make sense. Any chance you want to prepare a PR? |
Just contributing suggestions and feedback for now, since I currently have no cycles free for contributing code here. |
Currently sentry reports the line where we call
CaptureException
and not where the error actually happened.I think it should be possible to improve this that Sentry tells us the line where the error happend.
also to monitor the application performance we should add opentelementry instrumentation for echo.
https://docs.sentry.io/platforms/go/guides/echo/performance/instrumentation/opentelemetry/
https://uptrace.dev/opentelemetry/instrumentations/go-echo.html#echo-instrumentation
https://uptrace.dev/opentelemetry/go-tracing.html
The text was updated successfully, but these errors were encountered: