-
Notifications
You must be signed in to change notification settings - Fork 609
add sidekiq_notice_only_once configuration to reduce noise from retriable sidekiq failures #3296
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
base: dev
Are you sure you want to change the base?
Conversation
|
Giulio Giraldi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
…jobs are notified only once.
ab23c30 to
5877d7a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @DonGiulio, thanks for opening this PR! Sorry to hear about the additional noise from the Sidekiq failures.
Before we release a change like this, we would like to add some tests for it. Would you be willing to write some tests?
Alternatively, our team can write tests. We don't have the capacity to do this at this time, but we can leave your PR open until we do.
Hello, Thanks for the feedback, I added some tests, I hope it's enough |
|
Thank you! I'll take a look at the tests today. One other block on merging this PR is the CLA. Could you take a look at the CLA concerns for Giulio Giraldi? Let me know if there's anything we can do to help. |
I'm having troubles approving the CLA, both committers are myself. I signed with one account, I seem unable to sign again with the different name, but same account. I'm happy to amend the author if you tell me what I should put there to be able to complete it |
I took a look at the git logs for your branch and I think the problem is because the commits were made with two different email addresses. One with a gmail and the other with possibly a work email? GitHub recognizes the gmail, but not the other email. You could amend your commits to use a different author (I tried the suggestions in this post and they worked), add the second email to your GitHub account, or try to sign the CLA with the second email. |
Overview
instrumentation for sidekiq, configures the server to call
NewRelic::Agent.notice_erroron any error that occurs, this means there will be a call also for each failed retry, generating a lot of noise.(any temporary error that will be retried will be reported).
This can be annoying.
Adding sidekiq_notice_only_once configuration, by configuring it the user can decide not to report exceptions that will be retried, but only report an error on a job before it goes to the dead queue.
Thus reducing noise from temporary issues. (logging is advised)
Submitter Checklist:
Testing
The agent includes a suite of unit and functional tests which should be used to
verify your changes don't break existing functionality. These tests will run with
GitHub Actions when a pull request is made. More details on running the tests locally can be found
here for our unit tests,
and here for our functional tests.
For most contributions it is strongly recommended to add additional tests which
exercise your changes.
Reviewer Checklist