guardian / kinesis-logback-appender

LOGBack Appender for writing data into a Kinesis Stream
Apache License 2.0
40 stars 25 forks source link

Add link to dotcom approach of avoiding blocking #26

Closed philmcmahon closed 5 years ago

philmcmahon commented 6 years ago

We've taken this approach for all discussion apps, seems worth highlighting

sihil commented 6 years ago

How does this compare to the approach suggested in #24 with an AsyncAppender wrapper?

sihil commented 6 years ago

https://logback.qos.ch/manual/appenders.html#AsyncAppender says that it is automatically lossy, so could replace the circuit breaking feature as well. Bonus is that it is only lossy for INFO / TRACE / DEBUG messages.

Interestingly, this is the default config of Play when in PROD (see https://www.playframework.com/documentation/2.6.x/SettingsLogger#Default-configuration)

philmcmahon commented 6 years ago

Sounds good, and would reduce the implementation complexity. Any thoughts @QuarpT ?

QuarpT commented 6 years ago

Are you certain the AsyncAppender doesn't block on error messages when the queue is full @philmcmahon @sihil ?

sihil commented 6 years ago

You need to make sure you set neverBlock to true to drop all messages once the queue fills up: https://logback.qos.ch/manual/appenders.html#asyncNeverBlock

See https://github.com/guardian/ophan/blob/master/shared-lib/src/main/scala/ophan/util/ElkLogging.scala#L69 for the example in Ophan.

sihil commented 5 years ago

Closing this in favour of the AsyncAppender approach.