evalphobia / logrus_sentry

sentry hook for logrus
MIT License
194 stars 78 forks source link

logrus.Panic and async hook don't work well together #46

Open glasser opened 7 years ago

glasser commented 7 years ago

When doing a logrus.Fatal or logrus.Panic, it would be good if an async hook got flushed. With Fatal, this is doable because you can use logrus.RegisterExitHandler to call Flush. But you can't do that for Panic. Is it possible to get a Panic out with an async hook?

I'm thinking maybe Fire should ignore hook.asynchronous for the Panic level (and honestly maybe for the Fatal level too, so you don't have to hook up a Flush).

glasser commented 7 years ago

(I guess you can add a second hook that flushes on Panic and Fatal...)

glasser commented 7 years ago

This is my workaround:

type flushSentryHook struct {
    sentryHook *logrus_sentry.SentryHook
}

func (flushSentryHook) Levels() []logrus.Level {
    return []logrus.Level{logrus.PanicLevel, logrus.FatalLevel}
}
func (fsh flushSentryHook) Fire(*logrus.Entry) error {
    fsh.sentryHook.Flush()
    return nil
}

Then, after adding the sentry hook, also run logrus.AddHook(flushSentryHook{sentryHook: hook}).

blaskovicz commented 6 years ago

@glasser thanks for the snippet. I also wrapped your final shutdown flush in a timeout of its own (so that the wg.Done() of the underlying async hook doesn't wait forever).

https://gist.github.com/blaskovicz/8a61d655cb5ebaab53f6645204aa51b8