Possible bug in sidekiq_threshold implementation

[ujh]

Back in https://github.com/rollbar/rollbar-gem/commit/94baeb1f9662575ad0135fb4fa4a65e28610470e a bug in the reporting of Sidekiq retries was fixed that resulted in the first failure to be reported to Rollbar. I have the feeling that it's back.

Looking at the errors reported in Rollbar I can see that request.params.retry is false and looking at the code in question this probably prevents the retry logic from kicking in.

We are using Sidekiq 6.1.2 and rollbar 3.1.1

[Draiken]

Is this project dead?

[waltjones]

@Draiken What is the issue you're reporting?

[waltjones]

@ujh Rollbar does not use sidekiq_threshold when the Sidekiq worker has set sidekiq_options retry: false. This has been the behavior for a long time, both before and after https://github.com/rollbar/rollbar-gem/commit/94baeb1f9662575ad0135fb4fa4a65e28610470e in 2018.

That said, I can see why it could make sense to not report these since they also skip the Sidekiq Dead set.

Changing the current behavior would affect everyone else who does expect to see these occurrences. A new config flag (e.g. sidekiq_ignore_retry_false) could enable this behavior without impacting existing users.

In current rollbar-gem, adding this to your config will work:

  # adds the sidekiq params to the scope
  config.sidekiq_use_scoped_block 

  # config option to ignore occurrences
  config.before_process << proc do |options|
    return 'ignored' if options.dig(:scope, :request, :params, 'retry') == false
  end

For more info on before_process, see https://docs.rollbar.com/docs/ruby#section-ignoring-items

Alternately, settingsidekiq_options retry: 0 will allow sidekiq_threshold to be used as expected, but I'm guessing you don't want Sidekiq to retain these in the Dead set and that's why you're using false.