diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..ea773bd --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,37 @@ +name: CI +on: [pull_request, push] +jobs: + rubocop: + strategy: + fail-fast: true + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.1 + bundler-cache: true + - run: bundle exec rubocop + rspec: + strategy: + fail-fast: false + matrix: + ruby: ['3.1', '3.2', '3.3'] + runs-on: ubuntu-latest + services: + redis: + image: redis + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + steps: + - uses: actions/checkout@v2 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - run: bundle exec rake \ No newline at end of file diff --git a/.gitignore b/.gitignore index b04a8c8..1c05247 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,6 @@ # rspec failure tracking .rspec_status + +# Ignore Gemfile.lock +Gemfile.lock \ No newline at end of file diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..469aab1 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,115 @@ +require: + - rubocop-performance + - rubocop-rspec + +inherit_mode: + merge: + - Exclude + +AllCops: + NewCops: enable + TargetRubyVersion: 3.1 + Exclude: + - "db/**/*" + - "bin/**/*" + - "tmp/**/*" + - "log/**/*" + - "vendor/**/*" + - "spec/rails_helper.rb" + +Layout/FrozenStringLiteralComment: + Enabled: true + +Layout/ArgumentAlignment: + EnforcedStyle: with_fixed_indentation + +Layout/ArrayAlignment: + EnforcedStyle: with_fixed_indentation + +Layout/EmptyLineBetweenDefs: + AllowAdjacentOneLineDefs: true + +Layout/EndAlignment: + EnforcedStyleAlignWith: variable + +Layout/FirstArgumentIndentation: + EnforcedStyle: consistent + +Layout/FirstArrayElementIndentation: + EnforcedStyle: consistent + +Layout/FirstHashElementIndentation: + EnforcedStyle: consistent + +Layout/MultilineMethodCallIndentation: + EnforcedStyle: indented + +Layout/MultilineOperationIndentation: + EnforcedStyle: indented + +Layout/ParameterAlignment: + EnforcedStyle: with_fixed_indentation + +Layout/SpaceBeforeBrackets: + Enabled: false + + +Lint/UnusedMethodArgument: + AllowUnusedKeywordArguments: true + +Lint/AmbiguousBlockAssociation: + Exclude: + - 'spec/**/*' + + +Naming/FileName: + EnforcedStyle: snake_case + Exclude: + - 'lib/sidekiq-poison-pill-remedy.rb' + +Naming/VariableNumber: + EnforcedStyle: snake_case + + +RSpec/MultipleExpectations: + Max: 20 + +RSpec/NestedGroups: + Max: 10 + +RSpec/ExampleLength: + Max: 15 + Exclude: + - 'spec/sidekiq_poison_pill_remedy_spec.rb' + +RSpec/VerifiedDoubles: + Exclude: + - 'spec/**/*.rb' + + +Metrics/AbcSize: + Max: 15 + Exclude: + - 'lib/sidekiq_poison_pill_remedy.rb' + + +Metrics/MethodLength: + Max: 20 + +Metrics/BlockLength: + Exclude: + - 'lib/sidekiq_poison_pill_remedy.rb' + +Layout/LineLength: + Max: 125 + + +Style/StringLiterals: + EnforcedStyle: double_quotes + ConsistentQuotesInMultiline: true + +Style/Documentation: + Enabled: false + +RSpec/AnyInstance: + Enabled: false diff --git a/Gemfile b/Gemfile index 03278df..9b5a7c8 100644 --- a/Gemfile +++ b/Gemfile @@ -6,5 +6,13 @@ source "https://rubygems.org" gemspec gem "rake", "~> 13.0" +gem "sentry-ruby" -gem "rspec", "~> 3.0" +group :development, :test do + gem "rspec", "~> 3.0" + gem "rspec-sidekiq" + gem "rubocop", "~> 1.40", require: false + gem "rubocop-performance" + gem "rubocop-rake" + gem "rubocop-rspec" +end diff --git a/README.md b/README.md index cf430d3..7e90303 100644 --- a/README.md +++ b/README.md @@ -1,24 +1,31 @@ -# Sidekiq::Poison::Pill::Remedy +# SidekiqPoisonPillRemedy -TODO: Delete this and the text below, and describe your gem - -Welcome to your new gem! In this directory, you'll find the files you need to be able to package up your Ruby library into a gem. Put your Ruby code in the file `lib/sidekiq/poison/pill/remedy`. To experiment with that code, run `bin/console` for an interactive prompt. +The Sidekiq Poison Pill Remedy gem enhances Sidekiq's job processing by automatically handling and rescheduling failed jobs (poison pills) with integrated logging and error tracking through Sentry, ultimately improving reliability and performance optimization. ## Installation -TODO: Replace `UPDATE_WITH_YOUR_GEM_NAME_IMMEDIATELY_AFTER_RELEASE_TO_RUBYGEMS_ORG` with your gem name right after releasing it to RubyGems.org. Please do not do it earlier due to security reasons. Alternatively, replace this section with instructions to install your gem from git if you don't plan to release to RubyGems.org. +Add this line to your application's Gemfile: + +`gem 'sidekiq-poison-pill-remedy'` -Install the gem and add to the application's Gemfile by executing: +And then execute: - $ bundle add UPDATE_WITH_YOUR_GEM_NAME_IMMEDIATELY_AFTER_RELEASE_TO_RUBYGEMS_ORG +`$ bundle install` -If bundler is not being used to manage dependencies, install the gem by executing: +Or install it yourself as: - $ gem install UPDATE_WITH_YOUR_GEM_NAME_IMMEDIATELY_AFTER_RELEASE_TO_RUBYGEMS_ORG +`$ gem install sidekiq-poison-pill-remedy` ## Usage -TODO: Write usage instructions here +The gem is supposed to be used in the following way when added to the application + +Check Sidekiq super_fetch:[here](https://github.com/sidekiq/sidekiq/wiki/Reliability#using-super_fetch) + +Remedy is supposed to be use like: +`config.super_fetch!(&SidekiqPoisonPillRemedy.remedy)` + +When a job is considered a poison pill by Sidekiq, SidekiqPoisonPillRemedy prevents it from being moved to DeadSet and moves it to a dedicated queue to make sure the job will be processed after capturing team's attention with Sentry notification ## Development @@ -28,7 +35,7 @@ To install this gem onto your local machine, run `bundle exec rake install`. To ## Contributing -Bug reports and pull requests are welcome on GitHub at https://github.com/[USERNAME]/sidekiq-poison-pill-remedy. +Bug reports and pull requests are welcome on GitHub at https://github.com/BookingSync/sidekiq-poison-pill-remedy. ## License diff --git a/bin/console b/bin/console index 72d021f..f836a9f 100755 --- a/bin/console +++ b/bin/console @@ -1,11 +1,11 @@ #!/usr/bin/env ruby # frozen_string_literal: true -require "bundler/setup" -require "sidekiq/poison/pill/remedy" +require 'bundler/setup' +require 'sidekiq/poison/pill/remedy' # You can add fixtures and/or initialization code here to make experimenting # with your gem easier. You can also use a different console, if you like. -require "irb" +require 'irb' IRB.start(__FILE__) diff --git a/lib/sidekiq-poison-pill-remedy.rb b/lib/sidekiq-poison-pill-remedy.rb new file mode 100644 index 0000000..9625472 --- /dev/null +++ b/lib/sidekiq-poison-pill-remedy.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +require "sidekiq_poison_pill_remedy" +require "active_support" +require "active_support/inflector" diff --git a/lib/sidekiq/poison/pill/remedy.rb b/lib/sidekiq/poison/pill/remedy.rb deleted file mode 100644 index 238011c..0000000 --- a/lib/sidekiq/poison/pill/remedy.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require_relative "remedy/version" - -module Sidekiq - module Poison - module Pill - module Remedy - class Error < StandardError; end - # Your code goes here... - end - end - end -end diff --git a/lib/sidekiq/poison/pill/remedy/version.rb b/lib/sidekiq/poison/pill/remedy/version.rb deleted file mode 100644 index 075ca78..0000000 --- a/lib/sidekiq/poison/pill/remedy/version.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module Sidekiq - module Poison - module Pill - module Remedy - VERSION = "0.1.0" - end - end - end -end diff --git a/lib/sidekiq_poison_pill_remedy.rb b/lib/sidekiq_poison_pill_remedy.rb new file mode 100644 index 0000000..bc74ca7 --- /dev/null +++ b/lib/sidekiq_poison_pill_remedy.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module SidekiqPoisonPillRemedy + def self.remedy + proc do |_jobstr, pill| + next unless pill + + job = Sidekiq::DeadSet.new.find_job(pill.jid) + + if job.queue == "poison_pill" + capture_sentry_message( + "#{job.klass} failed in the `#{job.queue}`, this means that it has to be urgently optimized on memory usage", + level: :critical, + job_item: job.item + ) + else + capture_sentry_message( + "#{job.klass} was marked as `poison pill`, please create the job memory optimizations ticket timely", + level: :warning, + job_item: job.item + ) + job.klass.constantize.set(queue: :poison_pill).perform_async(*job.args) + job.delete + end + end + end + + def self.capture_sentry_message(message, level:, job_item:) + if defined?(Sentry) + Sentry.capture_message( + message, + level:, + extra: { job_item: } + ) + end + + Sidekiq.logger.fatal(message) + end +end diff --git a/lib/version.rb b/lib/version.rb new file mode 100644 index 0000000..921083c --- /dev/null +++ b/lib/version.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module SidekiqPoisonPillRemedy + module Version + end + VERSION = "0.1.0" +end diff --git a/sidekiq-poison-pill-remedy.gemspec b/sidekiq-poison-pill-remedy.gemspec index 10c8182..5f23c1c 100644 --- a/sidekiq-poison-pill-remedy.gemspec +++ b/sidekiq-poison-pill-remedy.gemspec @@ -1,24 +1,22 @@ # frozen_string_literal: true -require_relative "lib/sidekiq/poison/pill/remedy/version" +require_relative "lib/version" Gem::Specification.new do |spec| spec.name = "sidekiq-poison-pill-remedy" - spec.version = Sidekiq::Poison::Pill::Remedy::VERSION + spec.version = SidekiqPoisonPillRemedy::VERSION spec.authors = ["Karol Galanciak"] spec.email = ["karol.galanciak@gmail.com"] - spec.summary = "TODO: Write a short summary, because RubyGems requires one." - spec.description = "TODO: Write a longer description or delete this line." - spec.homepage = "TODO: Put your gem's website or public repo URL here." + spec.summary = "Enhances Sidekiq's job processing by automatically handling and rescheduling poison pills" + spec.description = "Enhances Sidekiq's job processing by automatically handling and rescheduling poison pills" + spec.homepage = "https://github.com/BookingSync/sidekiq-poison-pill-remedy" spec.license = "MIT" - spec.required_ruby_version = ">= 3.0.0" - - spec.metadata["allowed_push_host"] = "TODO: Set to your gem server 'https://example.com'" + spec.required_ruby_version = ">= 3.1.0" spec.metadata["homepage_uri"] = spec.homepage - spec.metadata["source_code_uri"] = "TODO: Put your gem's public repo URL here." - spec.metadata["changelog_uri"] = "TODO: Put your gem's CHANGELOG.md URL here." + spec.metadata["source_code_uri"] = "https://github.com/BookingSync/sidekiq-poison-pill-remedy" + spec.metadata["changelog_uri"] = "https://github.com/BookingSync/sidekiq-poison-pill-remedy/blob/master/CHANGELOG.md" # Specify which files should be added to the gem when it is released. # The `git ls-files -z` loads the files in the RubyGem that have been added into git. @@ -35,7 +33,10 @@ Gem::Specification.new do |spec| # Uncomment to register a new dependency of your gem # spec.add_dependency "example-gem", "~> 1.0" + spec.add_dependency "activesupport", ">= 6.1" + spec.add_dependency "sidekiq" # For more information and examples about making a new gem, check out our # guide at: https://bundler.io/guides/creating_gem.html + spec.metadata["rubygems_mfa_required"] = "true" end diff --git a/spec/sidekiq/poison/pill/remedy_spec.rb b/spec/sidekiq/poison/pill/remedy_spec.rb deleted file mode 100644 index d159a00..0000000 --- a/spec/sidekiq/poison/pill/remedy_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Sidekiq::Poison::Pill::Remedy do - it "has a version number" do - expect(Sidekiq::Poison::Pill::Remedy::VERSION).not_to be nil - end - - it "does something useful" do - expect(false).to eq(true) - end -end diff --git a/spec/sidekiq_poison_pill_remedy_spec.rb b/spec/sidekiq_poison_pill_remedy_spec.rb new file mode 100644 index 0000000..d98e4fc --- /dev/null +++ b/spec/sidekiq_poison_pill_remedy_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe SidekiqPoisonPillRemedy do + describe ".remedy" do + subject(:call) { described_class.remedy.call(nil, job) } + + let(:default_queue) { "default" } + let(:poison_pill_queue) { "poison_pill" } + let(:enqueue_job) { MyJob.set(queue: job_queue).perform_async("fail") } + let(:job) { Sidekiq::Queue.new(job_queue).find_job(enqueue_job) } + + before do + Sidekiq::Testing.disable! + Sidekiq::Queue.new(poison_pill_queue).clear + Sidekiq::Queue.new(default_queue).clear + + enqueue_job + + allow_any_instance_of(Sidekiq::DeadSet).to receive(:find_job).with(enqueue_job).and_return(job) + allow(Sentry).to receive(:capture_message).and_call_original + allow(Sidekiq.logger).to receive(:fatal).and_call_original + end + + context "when the job is a poison pill in non-poison pill queue" do + let(:job_queue) { default_queue } + + it "moves job to poison_pill queue and sends error notification" do + expect do + call + end.to change { Sidekiq::Queue.new(default_queue).count }.from(1).to(0) + .and change { Sidekiq::Queue.new(poison_pill_queue).count }.from(0).to(1) + + expect(Sentry).to have_received(:capture_message).with( + "MyJob was marked as `poison pill`, please create the job memory optimizations ticket timely", + level: :warning, + extra: hash_including(:job_item) + ) + expect(Sidekiq.logger).to have_received(:fatal).with( + "MyJob was marked as `poison pill`, please create the job memory optimizations ticket timely" + ) + end + end + + context "when the job is a poison pill in poison pill queue" do + let(:job_queue) { poison_pill_queue } + + it "keep the jobs in posion pill queue and sends error notification" do + expect(job.queue).to eq("poison_pill") + + expect do + call + end.to avoid_changing { Sidekiq::Queue.new(default_queue).count }.from(0) + .and avoid_changing { Sidekiq::Queue.new(poison_pill_queue).count }.from(1) + + expect(Sentry).to have_received(:capture_message).with( + "MyJob failed in the `poison_pill`, this means that it has to be urgently optimized on memory usage", + level: :critical, + extra: hash_including(:job_item) + ) + expect(Sidekiq.logger).to have_received(:fatal).with( + "MyJob failed in the `poison_pill`, this means that it has to be urgently optimized on memory usage" + ) + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 901a1e8..064984c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,15 @@ # frozen_string_literal: true -require "sidekiq/poison/pill/remedy" +require "bundler/setup" +require "sidekiq/testing" +require "rspec-sidekiq" +require "sentry-ruby" +require "sidekiq" +require "sidekiq/api" +require "sidekiq-poison-pill-remedy" +require "support/my_job" + +Dir[File.join(__dir__, "support", "**", "*.rb")].each { |f| require f } RSpec.configure do |config| # Enable flags like --only-failures and --next-failure @@ -12,4 +21,18 @@ config.expect_with :rspec do |c| c.syntax = :expect end + + config.before(:all) do + ENV["REDIS_URL"] ||= "redis://localhost:6379/1" + end + + Sidekiq.configure_server do |sidekiq_config| + sidekiq_config.redis = { url: ENV["REDIS_URL"] || "redis://localhost:6379/0" } + end + + Sidekiq.configure_client do |sidekiq_config| + sidekiq_config.redis = { url: ENV["REDIS_URL"] || "redis://localhost:6379/0" } + end + + RSpec::Matchers.define_negated_matcher :avoid_changing, :change end diff --git a/spec/support/my_job.rb b/spec/support/my_job.rb new file mode 100644 index 0000000..b4661e3 --- /dev/null +++ b/spec/support/my_job.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class MyJob + include Sidekiq::Job + sidekiq_options retry: 3 + + def perform(arg) + raise StandardError, "Job was called with nil argument" if arg.nil? + + puts "Job executed successfully with argument: #{arg}" + end +end