getsentry / sentry-go

The official Go SDK for Sentry (sentry.io)
https://docs.sentry.io/platforms/go/
MIT License
922 stars 213 forks source link

Capture panics with a wrapper program #107

Open rhcarvalho opened 4 years ago

rhcarvalho commented 4 years ago

To this date Go doesn't support a "recover all" mechanism to globally handle panicking goroutines (see https://github.com/golang/go/issues/20161).

This issue tracks investigating and adding to the SDK the capability of capturing panics by means of an external process.

rhcarvalho commented 4 years ago

Note to self: look at how go test handles panic() and go panic() inside of tests. In both cases it can still print:

FAIL    github.com/getsentry/sentry-go  0.008s

Done
rhcarvalho commented 4 years ago

Note to self: have in mind that spawning new processes can have adverse effects in containerized programs (consider the PID 1, zombie processes and reaping).

rhcarvalho commented 4 years ago

We may integrate with https://github.com/mitchellh/panicwrap.

parasssh commented 4 years ago

We may integrate with https://github.com/mitchellh/panicwrap.

I tried this and seems to work ok. Simply calling sentry.CaptureException followed by sentry.Flush() in the panic handler does the trick. The only downside is that the stack-trace will be useless as it gives the trace to the panic handler which is really a monitoring process. However, the panic-handler's input parameter has the stacktrace of exactly where the panic occurred in the application.

parasssh commented 4 years ago

We may integrate with https://github.com/mitchellh/panicwrap.

I tried this and seems to work ok. Simply calling sentry.CaptureException followed by sentry.Flush() in the panic handler does the trick. The only downside is that the stack-trace will be useless as it gives the trace to the panic handler which is really a monitoring process. However, the panic-handler's input parameter has the stacktrace of exactly where the panic occurred in the application.

The above downside is further exacerbated by the Sentry's grouping algorithm. Since all panics will have the same stack trace (the trace in the monitoring process), they are are all grouped together as one issue even though the panics may happen in different places. How can I get past this?

rhcarvalho commented 4 years ago

@parasssh you can configure grouping using fingerprints, and use BeforeSend to configure the Event.Fingerprint.

parasssh commented 4 years ago

First, I'd like to acknowledge that we have found 3-4 bugs since we integrated Sentry. It is definitely adding value for us and our customers. Thank you.

Now, It seems SDK Fingerprinting in the beforeSend is the way to go for the grouping issue . However, that would mean parsing the panic stack trace (which is actually just a message in the event in the way we implemented capturing panics) and fingerprinting off of that instead of the Default Grouping which relies on the Stack Trace which is always the same. However, I wanted to avoid parsing panics.

Alternatively,

Basically, I want Grouping to be done off the panic's stacktrace.

Also, on a related not, for testing purposes, what is the recommended way to send test/experimental events from SDK without polluting the account with such events. I am unable to find a Guide/Recommendation on that.

rhcarvalho commented 4 years ago

First, I'd like to acknowledge that we have found 3-4 bugs since we integrated Sentry. It is definitely adding value for us and our customers. Thank you.

Thank you for the feedback, made my day.

Just so we avoid going off-topic on this issue (Capturing panics with a wrapper program), I'll take your two questions into new issues; should also make it easier for other folks to find and benefit from the answers.

szgupta commented 6 months ago

Hi @cleptric, is there any update on this effort?

cleptric commented 6 months ago

No update