-
Notifications
You must be signed in to change notification settings - Fork 2
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
[im-2290] Instrument timeout exceptions #110
Conversation
@@ -22,6 +22,9 @@ def call(request_env) | |||
rescue Routemaster::Errors::BaseError => e | |||
increment_response_count(response_tags(e.env)) | |||
raise e | |||
rescue Faraday::TimeoutError => e |
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.
(If the circuit is open we won't report any metrics)
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.
Yep, but we would know how many timeouts we received before the breaker was tripped. We anyway don't push any metric once a circuit trips. We would have to record circuitbox metrics for that information.
@@ -22,6 +22,9 @@ def call(request_env) | |||
rescue Routemaster::Errors::BaseError => e | |||
increment_response_count(response_tags(e.env)) | |||
raise e | |||
rescue Faraday::TimeoutError => e |
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.
Should we add connection too?
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.
Wdym? Routemaster::Errors::BaseError
and Faraday::TimeoutError
are the only exceptions that are monitored by CircuitBox
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.
I meant Faraday::ConnectionFailed
, but sorry, forgot that you made this change only for CB.
DESCRIPTION
JIRA Ticket: im-2290
CHANGELOG:
Instrument timeout exceptions. There are a few tests failing on the PR, but these are unrelated to my changes and have been failing on the CI for some time now.