From 4cb1bdb754795a3f58296efb34d03c4265b4261c Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Fri, 3 Nov 2023 13:48:09 -0700 Subject: [PATCH 1/3] (sentry-sidekiq): Fixed a deprecation warning in error handler --- CHANGELOG.md | 1 + CONTRIBUTING.md | 9 ++++++++- sentry-sidekiq/Gemfile | 2 +- sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb | 8 +++++++- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 247adb3e4..3760c6b5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Fix breadcrumbs with `warn` level not being ingested [#2150](https://github.com/getsentry/sentry-ruby/pull/2150) - Fixes [#2145](https://github.com/getsentry/sentry-ruby/issues/2145) - Don't send negative line numbers in profiles [#2158](https://github.com/getsentry/sentry-ruby/pull/2158) +- Fixed a deprecation in `sidekiq-ruby` error handler [#2160](https://github.com/getsentry/sentry-ruby/pull/2160) ## 5.12.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e59d9e1e9..3619eb371 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,8 +19,15 @@ And if you have any questions, please feel free to reach out on [Discord]. ## Contribute To Individual Gems +- Install the dependencies of a specific gem by running `bundle` in it's subdirectory. I.e: + ```bash + cd sentry-sidekiq + bundle install + ``` +- Install any additional dependencies. `sentry-sidekiq` assumes you have `redis` running. - Use `bundle exec rake` to run tests. - - In `sentry-rails`, you can use `RAILS_VERSION=version` to specify the Rails version to test against. Default is `6.1` + - In `sentry-rails`, you can use `RAILS_VERSION=version` to specify the Rails version to test against. Default is `7.0` + - In `sentry-sidekiq`, you can use `SIDEKIQ_VERSION=version` to specify what version of Sidekiq to install when you run `bundle install`. Default is `7.0` - Use example apps under the `example` or `examples` folder to test the change. (Remember to change the DSN first) - To learn more about `sentry-ruby`'s structure, you can read the [Sentry SDK spec] diff --git a/sentry-sidekiq/Gemfile b/sentry-sidekiq/Gemfile index 8d1a2f82c..776f21219 100644 --- a/sentry-sidekiq/Gemfile +++ b/sentry-sidekiq/Gemfile @@ -19,7 +19,7 @@ gem "loofah", "2.20.0" if RUBY_VERSION.to_f < 2.5 gem "psych", "5.1.0" sidekiq_version = ENV["SIDEKIQ_VERSION"] -sidekiq_version = "6.0" if sidekiq_version.nil? +sidekiq_version = "7.0" if sidekiq_version.nil? sidekiq_version = Gem::Version.new(sidekiq_version) gem "sidekiq", "~> #{sidekiq_version}" diff --git a/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb b/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb index e99dc2b83..3f0aca2db 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb @@ -5,7 +5,13 @@ module Sidekiq class ErrorHandler WITH_SIDEKIQ_7 = ::Gem::Version.new(::Sidekiq::VERSION) >= ::Gem::Version.new("7.0") - def call(ex, context) + # @param ex [Exception] the exception / error that occured + # @param context [Hash or Array] Sidekiq error context + # @param sidekiq_config [Sidekiq::Config] Sidekiq configuration, + # defaults to Sidekiq's default configuration `Sidekiq.default_configuration` + # Sidekiq will pass the config in starting Sidekiq 7.1.5, see + # https://github.com/sidekiq/sidekiq/pull/6051 + def call(ex, context, sidekiq_config = ::Sidekiq.default_configuration) return unless Sentry.initialized? context_filter = Sentry::Sidekiq::ContextFilter.new(context) From 630c2855810ba188d78b4234b2221e86c78f1da0 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Sat, 4 Nov 2023 12:59:29 -0700 Subject: [PATCH 2/3] Fix compatibility with Sidekiq 6 --- .../lib/sentry/sidekiq/error_handler.rb | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb b/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb index 3f0aca2db..41fd9e933 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb @@ -7,11 +7,11 @@ class ErrorHandler # @param ex [Exception] the exception / error that occured # @param context [Hash or Array] Sidekiq error context - # @param sidekiq_config [Sidekiq::Config] Sidekiq configuration, - # defaults to Sidekiq's default configuration `Sidekiq.default_configuration` + # @param sidekiq_config [Sidekiq::Config, Hash] Sidekiq configuration, + # Defaults to nil. # Sidekiq will pass the config in starting Sidekiq 7.1.5, see # https://github.com/sidekiq/sidekiq/pull/6051 - def call(ex, context, sidekiq_config = ::Sidekiq.default_configuration) + def call(ex, context, sidekiq_config = nil) return unless Sentry.initialized? context_filter = Sentry::Sidekiq::ContextFilter.new(context) @@ -19,9 +19,12 @@ def call(ex, context, sidekiq_config = ::Sidekiq.default_configuration) scope = Sentry.get_current_scope scope.set_transaction_name(context_filter.transaction_name, source: :task) unless scope.transaction_name + # If Sentry is configured to only report an error _after_ all retries have been exhausted, + # and if the job is retryable, and have not exceeded the retry_limit, + # return early. if Sentry.configuration.sidekiq.report_after_job_retries && retryable?(context) retry_count = context.dig(:job, "retry_count") - if retry_count.nil? || retry_count < retry_limit(context) - 1 + if retry_count.nil? || retry_count < retry_limit(context, sidekiq_config) - 1 return end end @@ -43,7 +46,10 @@ def retryable?(context) retry_option == true || (retry_option.is_a?(Integer) && retry_option.positive?) end - def retry_limit(context) + # @return [Integer] the number of retries allowed for the job + # Tries to fetch the retry limit from the job config first, + # then falls back to Sidekiq's configuration. + def retry_limit(context, sidekiq_config) limit = context.dig(:job, "retry") case limit @@ -52,7 +58,11 @@ def retry_limit(context) when TrueClass max_retries = if WITH_SIDEKIQ_7 - ::Sidekiq.default_configuration[:max_retries] + # Sidekiq 7.1.5+ passes the config to the error handler, so we should use that. + # Sidekiq 7.0 -> 7.1.5 provides ::Sidekiq.default_configuration. + sidekiq_config.is_a?(::Sidekiq::Config) ? + sidekiq_config[:max_retries] : + ::Sidekiq.default_configuration[:max_retries] else ::Sidekiq.options[:max_retries] end From cc60849e3635b7be59eaf310b6f536a066fa91c2 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 13 Nov 2023 14:11:59 +0100 Subject: [PATCH 3/3] Fix changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3760c6b5a..f8688302d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## Unreleased + +### Bug Fixes + +- Fixed a deprecation in `sidekiq-ruby` error handler [#2160](https://github.com/getsentry/sentry-ruby/pull/2160) + ## 5.13.0 ### Features @@ -18,7 +24,6 @@ - Fix breadcrumbs with `warn` level not being ingested [#2150](https://github.com/getsentry/sentry-ruby/pull/2150) - Fixes [#2145](https://github.com/getsentry/sentry-ruby/issues/2145) - Don't send negative line numbers in profiles [#2158](https://github.com/getsentry/sentry-ruby/pull/2158) -- Fixed a deprecation in `sidekiq-ruby` error handler [#2160](https://github.com/getsentry/sentry-ruby/pull/2160) ## 5.12.0