-
Notifications
You must be signed in to change notification settings - Fork 428
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/net/http: fix status reporting for empty replies #1140
Conversation
When no data is written, in an HTTP handler, our wrapped response writer fails to capture a status code, resulting it the StatusCode tag being missing from the span. We should detect this situation and record a 200 OK, which is the default code for an empty response.
@@ -81,6 +81,6 @@ func wrapResponseWriter(w http.ResponseWriter, span ddtrace.Span) http.ResponseW | |||
w = mw | |||
} | |||
|
|||
return w | |||
return w, mw |
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.
Why not simply change the type of the return value from http.ResponseWriter
to *responseWriter
? It should still work, because it still implements http.ResponseWriter
. This way you're just returning the same thing twice under different types. Or, have I missed something?
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.
Unfortunately, w
is a http.ResponseWriter
since we're inventing them on the fly by combining things into unnamed struct types. At the end, w
is not a *responseWriter
, but a struct containing one, so we have to return the *responseWriter
independently. I agree it's kind of ugly, but I'm not sure a better way without more major refactoring.
Co-authored-by: Gabriel Aszalos <gabriel.aszalos@gmail.com>
…ataDog/dd-trace-go into knusbaum/improve-http-status-reporting
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.
👍 this looks like a nice improvement
When no data is written, in an HTTP handler, our wrapped response writer
fails to capture a status code, resulting it the StatusCode tag being
missing from the span. We should detect this situation and record a 200 OK,
which is the default code for an empty response.