Add option to notify Sentry only after the last retry on resque #2087
Conversation
GlauberrBatista
changed the title
Codecov ReportAll modified lines are covered by tests ✅
... and 50 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
|
Hello, any update on this? |
|
/cc @sl0thentr0py |
sl0thentr0py
force-pushed
the
feature/resque_retry_report_after
branch
from
36f916f
to
27beb99
Compare
sentry-resque/lib/sentry/resque.rb
Outdated
|
|
||
| if Sentry.configuration.resque.report_after_job_retries && defined?(::Resque::Plugins::Retry) == 'constant' && klass.is_a?(::Resque::Plugins::Retry) | ||
| retry_key = klass.redis_retry_key(payload['args']) | ||
| retry_attempt = ::Resque.redis.get(retry_key) |
There was a problem hiding this comment.
the SDK should never do its own database calls by any chance, that's a side effect that breaks all sorts of contracts.
Is there no way of knowing the retry status from the stuff available to this function?
There was a problem hiding this comment.
@sl0thentr0py In fact, there is. I didn't realize it, but we are catching an exception here (so the processed job context is available) and when we use constantize against the class name, resque-retry will capture the job context, so methods retry_limit_reached? and retry_limit_attempt will expose the current "status" of the job.
I just pushed a new commit that fixes it. Hopefully it will match the requirements
sl0thentr0py
force-pushed
the
feature/resque_retry_report_after
branch
from
76fa859
to
2b1ab7d
Compare
|
@sl0thentr0py not a problem and thank you 🙌 |
sentry-sidekiqandsentry-delayed_jobhave the option to report the failure only after the retry process has finished.With
sentry-resquewe can do the same when using theresque-retrygem. The implementation is not as clean asSidekiqandDelayedJob, which expose the attempts in a better way, but still, I was able to make it work.I noticed the configuration option was there but not implemented. I added a conditional inside the exception block to check if the job is an instance of
Resque::Plugins::Retryand if so, it checks the retry key.An odd behavior:
resque-retrycleans the key on the last try, before being caught on the exception block. Thus, when the key no longer exists on Redis, it means the retries were exhausted.I understand this change implies using the
resque-retrygem, but I'm unaware of a similar gem widely used asresque-retry.