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

implement report_after_job_retries in ActiveJob adapter #2499

Open
modosc opened this issue Dec 17, 2024 · 4 comments · May be fixed by #2500
Open

implement report_after_job_retries in ActiveJob adapter #2499

modosc opened this issue Dec 17, 2024 · 4 comments · May be fixed by #2500

Comments

@modosc
Copy link
Contributor

modosc commented Dec 17, 2024

we are using sentry-rails to report errors from ActiveJob (which is running on sidekiq). we want the ability to only report on errors when retries are exhausted. there doesn't seem to be a way to do this directly in the ActiveJob integration (sentry-sidekiq, sentry-delayed_job, and sentry-resque all support this configuration option).

is it possible to implement this directly in the ActiveJob adapter?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 17, 2024
@sl0thentr0py
Copy link
Member

Yes, we should implement :report_after_job_retries for active job too, feel free to make a PR if you want it quicker!

@modosc
Copy link
Contributor Author

modosc commented Dec 18, 2024

@sl0thentr0py i gave it a shot in #2500 but ran into several issues:

  1. tests work locally but not in CI or in CI but not locally, similar to Fix ActiveJob tests for Rails 7.2 and 8.0 #2487
  2. rails 5.0 didn't support retry_on and i'm unclear on how you expect that to be handled (print a deprecation warning? throw an error?).

anyway - i'm closing the pr out but i'll make sure the branch still exists. i think the approach is valid (and it seems to work) but i can't spend anymore time figuring out the tests.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 18, 2024
@sl0thentr0py
Copy link
Member

@modosc thanks a lot! I will reopen the PR and get it into a mergeable state myself. You don't have to do further work on it.

@modosc
Copy link
Contributor Author

modosc commented Jan 11, 2025

@sl0thentr0py curious if there's any progress on your end? we really need this functionality to quiet our production alerting and the fact that this is a monorepo means we can't easily just use my fork.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for: Product Owner
Development

Successfully merging a pull request may close this issue.

2 participants