-
Notifications
You must be signed in to change notification settings - Fork 435
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
contrib/labstack/{echo, echo.v4}: improve error detection #1000
Conversation
Manually tested, but need to add unit tests for this. |
Done, ready. |
Moving to 1.35.0. Hope that's ok. Otherwise ping me. |
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.
LGTM! Please mind the code freeze.
LGTM 👍 I'm looking forward to merging. Is there anything I can do ? |
This adds improved error detection to the echo integrations. Previously, any returned error was recorded as an error in the echo span. This patch causes the integration to inspect the returned error to determine if it is a echo.HTTPError which it can use to discriminate between real errors (5xx and up) or non-errors (4xx and below) Fixes #987
8f109ee
to
61a12f3
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.
Overall looks good, just two quick questions
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.
Looks good!
This adds improved error detection to the echo integrations. Previously,
any returned error was recorded as an error in the echo span. This patch
causes the integration to inspect the returned error to determine if it is
a echo.HTTPError which it can use to discriminate between real errors (5xx
and up by default) or non-errors (below 5xx by default)
This also adds a WithStatusCheck Option which allows users to configure
which errors/statuses are reported as errors.
Fixes #987