evalphobia / logrus_sentry / 95

81%

Pull #47

travis-ci

Stop double-buffering in async hook

The underlying Raven Sentry client already is asynchronous with an adjustable
buffer size. Unlike the buffer in logrus_sentry, Raven drops packets when the
buffer is filled (and calls an optional DropHandler when it does so).

The existing implementation of async hooks does not drop packets when its buffer
fills, which means that the so-called "async" hook becomes synchronous if the
buffer fills, and the Raven packet-dropping logic most likely never has a chance
to do its tricks.  In our service, we discovered that bugs that caused errors to
spew to Sentry via logrus_sentry quickly hit this buffer's limit and caused our
service to stall, despite the supposedly "async" nature of the hook.

This commit changes logrus_sentry's async mode to be truly asynchronous: it
delivers packets to Raven, which either passes them to its background worker or
drops them if its buffer is full.  It then starts a new goroutine per packet to
log any errors and coordinate with the WaitGroup for flush.

The previous version of the code set a 1 second Timeout for async mode, which
effectively just allowed the worker goroutine to send the next packet to Raven
after 1 second.  Because we no longer have a separate worker goroutine, this
is no longer necessary.  A comment also mentions how to set a timeout for the
underlying HTTP POST, which achieves a similar goal of allowing Flush to finish
in a bounded amount of time.

Note that Raven's buffer size (100) is smaller than logrus_sentry's old buffer
size (8192), so users may want to increase raven.MaxQueueBuffer before
contructing the hook to avoid dropping packets.

22 of 22 new or added lines in 1 file covered. (100.0%)

330 of 385 relevant lines covered (85.71%)

61.55 hits per line