-
Notifications
You must be signed in to change notification settings - Fork 377
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
Add config support for "on_error" in Elasticsearch tracing #4066
Add config support for "on_error" in Elasticsearch tracing #4066
Conversation
This mirrors the functionality that is already present in many other instrumenters. Reasons for wanting this: You might want to ignore some errors in some expected scenarios such as Elasticsearch::Transport::Transport::Errors::NotFound or Elasticsearch::Transport::Transport::Errors::Conflict
9398422
to
4e02912
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4066 +/- ##
==========================================
+ Coverage 97.77% 97.78% +0.01%
==========================================
Files 1353 1353
Lines 81785 81806 +21
Branches 4141 4143 +2
==========================================
+ Hits 79963 79997 +34
+ Misses 1822 1809 -13 ☔ View full report in Codecov by Sentry. |
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.
Minor feedback on tense.
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.
Well done 👏🏼
I've updated PR body and add Change log entry section
This mirrors the functionality that is already present in many other instrumenters.
Reasons for wanting this:
One might want to ignore some errors in some expected scenarios such as
Elasticsearch::Transport::Transport::Errors::NotFound
or
Elasticsearch::Transport::Transport::Errors::Conflict
What does this PR do?
Adds
on_error
configuration support for Elasticsearch tracing and forwards it toTracing.trace
Motivation:
We have flows where
NotFound
exceptions are expected and we make sure to handle them in the application.We don't want them to show up as errors in the dashboard though.
Change log entry
Yes. Add configuration option
on_error
to Elasticsearch tracing(Added by @Strech)
How to test the change?
Run provided tests with
bundle exec rake test:elasticsearch