-
Notifications
You must be signed in to change notification settings - Fork 579
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
Use server span when determining status from code #1973
Use server span when determining status from code #1973
Conversation
Switches to `SpanStatusFromHTTPStatusCodeAndSpanKind` with a span kind of `SpanKindServer` so that 4xx status codes don't get labelled as errors. Since these codes represent client errors, they make it difficult to differentiate bad server behavior from bad client behavior.
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.
Seems reasonable. Can you please add a test and CHANGELOG entry for this new behavior?
Sure thing, thanks for looking! |
I pushed the CHANGELOG entry but it's not clear to me how to test the result. Do you have any advice for checking the state of a span after the handler and middleware have returned? |
You'll probably need to create a test case similar to this: opentelemetry-go-contrib/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go Line 35 in 826e886
The |
Codecov Report
@@ Coverage Diff @@
## main #1973 +/- ##
=====================================
Coverage 69.4% 69.4%
=====================================
Files 135 135
Lines 6114 6114
=====================================
+ Hits 4244 4247 +3
+ Misses 1742 1739 -3
Partials 128 128
|
Adds a test to verify that the span created by a request that returns status "not found" is not an error span.
…ver-span-for-status
Thanks for the info. Added that test and caught the branch up with main. |
Switches to
SpanStatusFromHTTPStatusCodeAndSpanKind
with a span kindof
SpanKindServer
so that 4xx status codes don't get labelled aserrors. Since these codes represent client errors, they make it
difficult to differentiate bad server behavior from bad client behavior.