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

Support after-retry reporting to sentry-sidekiq #1532

Merged
merged 4 commits into from
Aug 16, 2021
Merged

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Aug 15, 2021

This PR adds a new config.sidekiq.report_after_job_retries option (default: false).

When activated, sentry-sidekiq will wait for a job's retries to be exhausted before reporting the exception. Worker without the retry option will not be affected.

Closes #781

@st0012 st0012 added this to the 4.7.0 milestone Aug 15, 2021
@st0012 st0012 self-assigned this Aug 15, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 15, 2021

Codecov Report

Merging #1532 (7b4f589) into master (bcd728b) will increase coverage by 0.52%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1532      +/-   ##
==========================================
+ Coverage   98.21%   98.73%   +0.52%     
==========================================
  Files         218      125      -93     
  Lines       10624     6879    -3745     
==========================================
- Hits        10434     6792    -3642     
+ Misses        190       87     -103     
Impacted Files Coverage Δ
sentry-sidekiq/lib/sentry-sidekiq.rb 91.30% <100.00%> (+0.39%) ⬆️
sentry-sidekiq/lib/sentry/sidekiq/configuration.rb 100.00% <100.00%> (ø)
sentry-sidekiq/lib/sentry/sidekiq/error_handler.rb 100.00% <100.00%> (ø)
...c/sentry/sidekiq/sentry_context_middleware_spec.rb 100.00% <100.00%> (ø)
sentry-sidekiq/spec/sentry/sidekiq_spec.rb 100.00% <100.00%> (ø)
sentry-raven/spec/raven/raven_spec.rb
...entry-raven/lib/raven/breadcrumbs/sentry_logger.rb
...en/spec/raven/integrations/rails/activejob_spec.rb
sentry-raven/lib/raven/breadcrumbs.rb
...try-raven/lib/raven/interfaces/single_exception.rb
... and 92 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcd728b...7b4f589. Read the comment docs.

When set to true, `sentry-sidekiq` only reports exceptions when the job
has been fully retried.
@st0012 st0012 merged commit 952e9c9 into master Aug 16, 2021
@st0012 st0012 deleted the implement-#781 branch August 16, 2021 03:50
@@ -11,6 +11,15 @@ def call(ex, context)
scope = Sentry.get_current_scope
scope.set_transaction_name(context_filter.transaction_name) unless scope.transaction_name

retry_option = context.dig(:job, "retry")

if Sentry.configuration.sidekiq.report_after_job_retries && retry_option.is_a?(Integer)
Copy link

@semenyukdmitry semenyukdmitry Sep 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@st0012 it looks like retry_option is a boolean here; when would it be an integer?

Never mind, I realized the job itself has to be configured with sidekiq_options retry: SOME_INTEGER for this feature to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't send an exception until retries are exhausted if the exception is retryable with Sidekiq
4 participants