search

evalphobia / logrus_sentry

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

Retry up to N times in case of 429 response #34

Open flimzy opened 7 years ago

flimzy commented 7 years ago

This fixes #31 .

Additional discussion may be warranted before a merge, though, in case the default behavior should possibly be changed.

As of now, I simply expose a hook.Retries field, which can be set to non-zero to enable retries. Otherwise, behavior remains the same as before.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.004%) to 83.702% when pulling 9cd97e90487c91322e66b5520ced4504ecdec68c on flimzy:retry into 3d7f0590b8bf893d3423f355be8dea7078fb3371 on evalphobia:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.004%) to 83.702% when pulling b37bcddfca32bb45e88c0e7af8c59ce8d04ced27 on flimzy:retry into 3d7f0590b8bf893d3423f355be8dea7078fb3371 on evalphobia:master.

mcpherrinm commented 7 years ago

It seems to me that you'd want to do some kind of exponential backoff here. If Sentry thinks it's overloaded, hammering it repeatedly seems like the wrong thing to do.

evalphobia commented 7 years ago

@flimzy Thank you for contributing. And sorry for my lateness. 429 error is the kind of error for rate limit, right?

As @mcpherrinm mentioned, I think it needs some waiting time to guarantee the effectiveness. For example, HTTP library h2non/gentleman has plugin for retry and it implements waiting time to retry. It use millisec scale, but Sentry's rate limit seems calcutlates the events on each minute, so the scale should be more than seconds.