diff --git a/.gitignore b/.gitignore index 0cb6eeb..616c060 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ /pkg/ /spec/reports/ /tmp/ +sidekiq-instrument-* \ No newline at end of file diff --git a/lib/sidekiq/instrument/mixin.rb b/lib/sidekiq/instrument/mixin.rb index 4305054..189839a 100644 --- a/lib/sidekiq/instrument/mixin.rb +++ b/lib/sidekiq/instrument/mixin.rb @@ -13,10 +13,17 @@ def worker_dog_options(worker) end def max_retries(worker) - retries = worker.class.get_sidekiq_options['retry'] || Sidekiq[:max_retries] - return Sidekiq[:max_retries] if retries.to_s.eql?("true") - return 0 if retries.eql?("false") - retries + retries = fetch_worker_retry(worker) + case retries.to_s + when "true" + Sidekiq[:max_retries] + when "false" + 0 + when "" + Sidekiq[:max_retries] + else + retries + end end private @@ -29,6 +36,10 @@ def class_name(worker) worker.class.name.gsub('::', '_') end + def fetch_worker_retry(worker) + worker.class.get_sidekiq_options['retry'] + end + def underscore(string) string.gsub(/::/, '/'). gsub(/([A-Z]+)([A-Z][a-z])/,'\1_\2'). diff --git a/lib/sidekiq/instrument/version.rb b/lib/sidekiq/instrument/version.rb index 818a3ba..9c7fa30 100644 --- a/lib/sidekiq/instrument/version.rb +++ b/lib/sidekiq/instrument/version.rb @@ -1,5 +1,5 @@ module Sidekiq module Instrument - VERSION = '0.7.0' + VERSION = '0.7.1' end end diff --git a/spec/sidekiq-instrument/server_middleware_spec.rb b/spec/sidekiq-instrument/server_middleware_spec.rb index 719d3f5..8e04d76 100644 --- a/spec/sidekiq-instrument/server_middleware_spec.rb +++ b/spec/sidekiq-instrument/server_middleware_spec.rb @@ -28,7 +28,6 @@ context 'when an initial job succeeds' do before do Sidekiq[:max_retries] = 0 - allow_any_instance_of(MyWorker).to receive(:get_sidekiq_options).and_return({ 'retry': 'true'}) end it 'increments StatsD dequeue counter' do @@ -85,7 +84,6 @@ before do Sidekiq[:max_retries] = 1 allow_any_instance_of(MyWorker).to receive(:perform).and_raise('foo') - allow_any_instance_of(MyWorker).to receive(:get_sidekiq_options).and_return({ 'retry': 'true'}) # This makes the job look like a retry since we can't access the job argument allow_any_instance_of(Sidekiq::Instrument::ServerMiddleware).to receive(:current_retries).and_return(0) @@ -114,7 +112,6 @@ before do Sidekiq[:max_retries] = 0 allow_any_instance_of(MyWorker).to receive(:perform).and_raise('foo') - allow_any_instance_of(MyWorker).to receive(:get_sidekiq_options).and_return({ 'retry': 'false'}) end it 'increments the StatsD error counter' do @@ -141,36 +138,79 @@ end end - context 'job has retries left' do - before do - Sidekiq[:max_retries] = 1 - allow_any_instance_of(MyWorker).to receive(:get_sidekiq_options).and_return({ 'retry': 'true'}) + context 'the worker has retries disabled' do + shared_examples 'it does not attempt to track retries' do |retry_value| + before do + Sidekiq[:max_retries] = 1 + allow(MyWorker).to receive(:get_sidekiq_options).and_return({ "retry" => retry_value, "queue" => 'default' }) + end + + it 'does not increments the DogStatsD enqueue.retry counter' do + expect( + Sidekiq::Instrument::Statter.dogstatsd + ).to receive(:increment).with('sidekiq.dequeue', expected_dog_options).once + expect( + Sidekiq::Instrument::Statter.dogstatsd + ).not_to receive(:increment).with('sidekiq.enqueue.retry', expected_dog_options) + expect(Sidekiq::Instrument::Statter.dogstatsd).not_to receive(:time) + expect( + Sidekiq::Instrument::Statter.dogstatsd + ).to receive(:increment).with('sidekiq.error', expected_dog_options).once + + begin + MyWorker.perform_async + rescue StandardError + nil + end + end end - it 'increments the DogStatsD enqueue.retry counter' do - expect( - Sidekiq::Instrument::Statter.dogstatsd - ).to receive(:increment).with('sidekiq.dequeue', expected_dog_options).once - expect( - Sidekiq::Instrument::Statter.dogstatsd - ).to receive(:increment).with('sidekiq.enqueue.retry', expected_dog_options).once - expect(Sidekiq::Instrument::Statter.dogstatsd).not_to receive(:time) - expect( - Sidekiq::Instrument::Statter.dogstatsd - ).to receive(:increment).with('sidekiq.error', expected_dog_options).once + it_behaves_like 'it does not attempt to track retries', false - begin - MyWorker.perform_async - rescue StandardError - nil + it_behaves_like 'it does not attempt to track retries', 'false' + + it_behaves_like 'it does not attempt to track retries', 0 + end + + context 'the current job has retries left to attempt' do + shared_examples 'it tracks the retries with DogStatsD' do |retry_value| + before do + Sidekiq[:max_retries] = 2 + allow(MyWorker).to receive(:get_sidekiq_options).and_return({ "retry" => retry_value, "queue" => 'default' }) + end + + it 'does not increments the DogStatsD enqueue.retry counter' do + expect( + Sidekiq::Instrument::Statter.dogstatsd + ).to receive(:increment).with('sidekiq.dequeue', expected_dog_options).once + expect( + Sidekiq::Instrument::Statter.dogstatsd + ).to receive(:increment).with('sidekiq.enqueue.retry', expected_dog_options).once + expect(Sidekiq::Instrument::Statter.dogstatsd).not_to receive(:time) + expect( + Sidekiq::Instrument::Statter.dogstatsd + ).to receive(:increment).with('sidekiq.error', expected_dog_options).once + + begin + MyWorker.perform_async + rescue StandardError + nil + end end end + + it_behaves_like 'it tracks the retries with DogStatsD', 'true' + + it_behaves_like 'it tracks the retries with DogStatsD', true + + it_behaves_like 'it tracks the retries with DogStatsD', 5 + + it_behaves_like 'it tracks the retries with DogStatsD', nil end - context 'on its last retry' do + context 'the job is on its last retry attempt' do before do Sidekiq[:max_retries] = 1 - allow_any_instance_of(MyWorker).to receive(:get_sidekiq_options).and_return({ 'retry': 'true'}) # This makes the job look like a retry since we can't access the job argument allow_any_instance_of(Sidekiq::Instrument::ServerMiddleware).to receive(:current_retries).and_return(1)