diff --git a/Gemfile b/Gemfile index fadcf15..7f4f5e9 100644 --- a/Gemfile +++ b/Gemfile @@ -3,6 +3,3 @@ source 'https://rubygems.org' gemspec - -gem 'rubocop-rake' -gem 'rubocop-rspec' diff --git a/README.md b/README.md index d9c3f47..4b96a8f 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,10 @@ Sidekiq::Logstash.configure do |config| config.custom_options = lambda do |payload| payload['my_custom_field'] = 'my_custom_value' end + + # by default, sidekiq-logstash removes the default error handler + # to keep it, simply set this to true + config.keep_default_error_handler = true end ``` diff --git a/lib/sidekiq/logstash.rb b/lib/sidekiq/logstash.rb index 5b1ea43..ce72193 100644 --- a/lib/sidekiq/logstash.rb +++ b/lib/sidekiq/logstash.rb @@ -22,21 +22,18 @@ def self.configure def self.setup(_opts = {}) # Calls Sidekiq.configure_server to inject logics Sidekiq.configure_server do |config| - if defined?(Sidekiq::ExceptionHandler::Logger) - # Remove default Sidekiq error_handler that logs errors - # NOTE: this only exists up until Sidekiq 6.4.x - config.error_handlers.delete_if { |h| h.is_a?(Sidekiq::ExceptionHandler::Logger) } + # Remove default, noisy error handler, + # unless LogStash.configuration.keep_default_error_handler is set to true + unless configuration.keep_default_error_handler + config.error_handlers.delete(Sidekiq::Config::ERROR_HANDLER) + # Insert a no-op error handler to prevent Sidekiq from logging to STDOUT + # because of empty error_handlers (see link). + # https://github.com/mperham/sidekiq/blob/02153c17360e712d9a94c08406fe7c057c4d7635/lib/sidekiq/config.rb#L258 + config.error_handlers << proc {} end # Add logstash support - if config.respond_to?(:[]=) - # Only available from Sidekiq 6.5 - config[:job_logger] = Sidekiq::LogstashJobLogger - else - # This is deprecated and will be removed in Sidekiq 7.0 - config.options[:job_logger] = Sidekiq::LogstashJobLogger - end - + config[:job_logger] = Sidekiq::LogstashJobLogger # Set custom formatter for Sidekiq logger config.logger.formatter = Sidekiq::Logging::LogstashFormatter.new end diff --git a/lib/sidekiq/logstash/configuration.rb b/lib/sidekiq/logstash/configuration.rb index b539648..d03677d 100644 --- a/lib/sidekiq/logstash/configuration.rb +++ b/lib/sidekiq/logstash/configuration.rb @@ -4,7 +4,7 @@ module Sidekiq module Logstash # Class that allows to configure the gem behaviour. class Configuration - attr_accessor :custom_options, :filter_args, :job_start_log + attr_accessor :custom_options, :filter_args, :job_start_log, :keep_default_error_handler def initialize @filter_args = [] diff --git a/sidekiq-logstash.gemspec b/sidekiq-logstash.gemspec index a833cd9..8711253 100644 --- a/sidekiq-logstash.gemspec +++ b/sidekiq-logstash.gemspec @@ -22,7 +22,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.5.0' spec.add_dependency 'logstash-event', '~> 1.2' - spec.add_runtime_dependency 'sidekiq', '>= 6.0', '<7' + spec.add_dependency 'sidekiq', '~> 7.0' spec.add_development_dependency 'factory_bot', '~> 6.1' spec.add_development_dependency 'rake', '>= 10' diff --git a/spec/sidekiq/logstash_spec.rb b/spec/sidekiq/logstash_spec.rb index 45491f8..e0d6702 100644 --- a/spec/sidekiq/logstash_spec.rb +++ b/spec/sidekiq/logstash_spec.rb @@ -13,16 +13,22 @@ def process(worker, params = [], encrypt: false) let(:buffer) { StringIO.new } let(:logger) { Logger.new(buffer) } let(:job) { build(:job) } - let(:processor_options) { Sidekiq } - let(:processor) { ::Sidekiq::Processor.new(processor_options) } + let(:logstash_config) { Sidekiq::Logstash::Configuration.new } + let(:sidekiq_config) { Sidekiq::Config.new } + let(:processor) { ::Sidekiq::Processor.new(sidekiq_config.default_capsule) } let(:log_message) { JSON.parse(buffer.string) } let(:log_messages) { buffer.string.split("\n").map { |log| JSON.parse(log) } } + let(:mock_redis) { double(:Redis) } + let(:custom_config) { nil } before do + # Reset error handlers because they're globally stored across instances of Sidekiq::Config + Sidekiq::Config::DEFAULTS[:error_handlers] = [] + allow(Sidekiq).to receive(:default_configuration).and_return(sidekiq_config) + allow(Sidekiq::Logstash).to receive(:configuration).and_return(logstash_config) logger.formatter = Sidekiq::Logging::LogstashFormatter.new - Sidekiq.logger = logger - processor_options[:job_logger] = Sidekiq::LogstashJobLogger - processor_options[:fetch] = ::Sidekiq::BasicFetch.new(processor_options) + sidekiq_config.logger = logger + Sidekiq::Logstash.setup end it 'has a version number' do @@ -101,16 +107,22 @@ def process(worker, params = [], encrypt: false) end context 'when job raises a error' do - it 'logs the exception with job retry' do - mock_redis = double(:Redis) - allow(Sidekiq).to receive(:redis).and_yield(mock_redis) + before do + allow(processor.capsule).to receive(:redis) + end - expect(mock_redis).to receive(:zadd).with('retry', any_args).once + it 'logs a single line, silencing the default error handler' do + expect { process(SpecWorker, [true]) } + .to raise_error(RuntimeError) + .and not_output.to_stdout + end + + it 'logs the exception with job retry' do expect { process(SpecWorker, [true]) }.to raise_error(RuntimeError) - expect(log_messages.last['error_message']).to eq('You know nothing, Jon Snow.') - expect(log_messages.last['error']).to eq('RuntimeError') - expect(log_messages.last['error_backtrace'].split("\n").first).to include('workers/spec_worker.rb:7') + expect(log_message['error_message']).to eq('You know nothing, Jon Snow.') + expect(log_message['error']).to eq('RuntimeError') + expect(log_message['error_backtrace'].split("\n").first).to include('workers/spec_worker.rb:7') end it 'logs the exception without job retry' do @@ -118,9 +130,9 @@ def process(worker, params = [], encrypt: false) expect { process(SpecWorker, [true]) }.to raise_error(RuntimeError) - expect(log_messages.last['error_message']).to eq('You know nothing, Jon Snow.') - expect(log_messages.last['error']).to eq('RuntimeError') - expect(log_messages.last['error_backtrace'].split("\n").first).to include('workers/spec_worker.rb:7') + expect(log_message['error_message']).to eq('You know nothing, Jon Snow.') + expect(log_message['error']).to eq('RuntimeError') + expect(log_message['error_backtrace'].split("\n").first).to include('workers/spec_worker.rb:7') end end @@ -144,4 +156,16 @@ def process(worker, params = [], encrypt: false) expect(log_messages.last['job_status']).to eq('done') end end + + context 'with keep_default_error_handler' do + let(:logstash_config) do + Sidekiq::Logstash::Configuration.new.tap do |config| + config.keep_default_error_handler = true + end + end + + it 'leaves the default error handler in place' do + expect(sidekiq_config.error_handlers).to include(Sidekiq::Config::ERROR_HANDLER) + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d459538..be6e972 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,4 +14,4 @@ require 'factory_bot' require 'support/factory_bot' -Sidekiq::Logstash.setup +RSpec::Matchers.define_negated_matcher :not_output, :output