Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate Payload logging configuration #81

Merged
merged 5 commits into from
May 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,21 @@ class UniqueJobWithFilterMethod
end
```

### Logging

To see logging in sidekiq when duplicate payload has been filtered out you can enable on a per worker basis using the sidekiq options. The default value is false

```ruby
class UniqueJobWithFilterMethod
include Sidekiq::Worker
sidekiq_options unique: true,
log_duplicate_payload: true

...

end
```

### Testing

SidekiqUniqueJobs uses mock_redis for inline testing. Due to complaints about having that as a runtime dependency it was made a development dependency so if you are relying on inline testing you will have to add `gem 'mock_redis'` to your Gemfile.
Expand All @@ -112,3 +127,4 @@ SidekiqUniqueJobs uses mock_redis for inline testing. Due to complaints about ha
- https://github.com/eduardosasso
- https://github.com/KensoDev
- https://github.com/adstage-david
- https://github.com/jprincipe
13 changes: 8 additions & 5 deletions lib/sidekiq_unique_jobs/middleware/client/strategies/unique.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,32 @@ def self.elegible?
true
end

def self.review(worker_class, item, queue, redis_pool = nil)
new(worker_class, item, queue, redis_pool).review { yield }
def self.review(worker_class, item, queue, redis_pool = nil, log_duplicate_payload = false)
new(worker_class, item, queue, redis_pool, log_duplicate_payload).review { yield }
end

def initialize(worker_class, item, queue, redis_pool = nil)
def initialize(worker_class, item, queue, redis_pool = nil, log_duplicate_payload = false)
@worker_class = SidekiqUniqueJobs.worker_class_constantize(worker_class)
@item = item
@queue = queue
@redis_pool = redis_pool
@log_duplicate_payload = log_duplicate_payload
end

def review
item['unique_hash'] = payload_hash
unless unique_for_connection?
Sidekiq.logger.warn "payload is not unique #{item}"
if @log_duplicate_payload
Sidekiq.logger.warn "payload is not unique #{item}"
end
return
end
yield
end

private

attr_reader :item, :worker_class, :redis_pool, :queue
attr_reader :item, :worker_class, :redis_pool, :queue, :log_duplicate_payload

# rubocop:disable MethodLength
def unique_for_connection?
Expand Down
6 changes: 5 additions & 1 deletion lib/sidekiq_unique_jobs/middleware/client/unique_jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def call(worker_class, item, queue, redis_pool = nil)
@redis_pool = redis_pool

if unique_enabled?
strategy.review(worker_class, item, queue, redis_pool) { yield }
strategy.review(worker_class, item, queue, redis_pool, log_duplicate_payload?) { yield }
else
yield
end
Expand All @@ -30,6 +30,10 @@ def unique_enabled?
worker_class.get_sidekiq_options['unique'] || item['unique']
end

def log_duplicate_payload?
worker_class.get_sidekiq_options['log_duplicate_payload'] || item['log_duplicate_payload']
end

def strategy
STRATEGIES.detect(&:elegible?)
end
Expand Down
Binary file removed spec/lib/.sidekiq_testing_enabled_spec.rb.swp
Binary file not shown.
20 changes: 20 additions & 0 deletions spec/lib/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,25 @@ class QueueWorkerWithFilterProc < QueueWorker

expect(actual_expires_at).to be_within(2).of(expected_expires_at)
end

it 'logs duplicate payload when config turned on' do
expect(Sidekiq.logger).to receive(:warn).with(/^payload is not unique/)

QueueWorker.sidekiq_options unique: true, log_duplicate_payload: true

2.times { Sidekiq::Client.push('class' => QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) }
result = Sidekiq.redis { |c| c.llen('queue:customqueue') }
expect(result).to eq 1
end

it 'does not log duplicate payload when config turned off' do
expect(Sidekiq.logger).to_not receive(:warn).with(/^payload is not unique/)

QueueWorker.sidekiq_options unique: true, log_duplicate_payload: false

2.times { Sidekiq::Client.push('class' => QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) }
result = Sidekiq.redis { |c| c.llen('queue:customqueue') }
expect(result).to eq 1
end
end
end