Skip to content
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

otelhttp: Treat http.NoBody the same as a nil body #2983

Merged
merged 1 commit into from Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixed

- Set the status_code span attribute even if the HTTP handler hasn't written anything. (#2822)
- Do not wrap http.NoBody in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`, which fixes handling of that special request body. (#2983)

## [1.11.1/0.36.4/0.5.2]

Expand Down
8 changes: 4 additions & 4 deletions instrumentation/net/http/otelhttp/handler.go
Expand Up @@ -164,10 +164,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

var bw bodyWrapper
// if request body is nil we don't want to mutate the body as it will affect
// the identity of it in an unforeseeable way because we assert ReadCloser
// fulfills a certain interface and it is indeed nil.
if r.Body != nil {
// if request body is nil or NoBody, we don't want to mutate the body as it
// will affect the identity of it in an unforeseeable way because we assert
// ReadCloser fulfills a certain interface and it is indeed nil or NoBody.
if r.Body != nil && r.Body != http.NoBody {
bw.ReadCloser = r.Body
bw.record = readRecordFunc
r.Body = &bw
Expand Down
20 changes: 20 additions & 0 deletions instrumentation/net/http/otelhttp/handler_test.go
Expand Up @@ -59,3 +59,23 @@ func TestHandlerReadingNilBodySuccess(t *testing.T) {
h.ServeHTTP(rr, r)
assert.Equal(t, 200, rr.Result().StatusCode)
}

// This use case is important as we make sure the body isn't mutated
// when it is NoBody.
func TestHandlerReadingNoBodySuccess(t *testing.T) {
h := otelhttp.NewHandler(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Body != http.NoBody {
_, err := io.ReadAll(r.Body)
assert.NoError(t, err)
}
}), "test_handler",
)

r, err := http.NewRequest(http.MethodGet, "http://localhost/", http.NoBody)
require.NoError(t, err)

rr := httptest.NewRecorder()
h.ServeHTTP(rr, r)
assert.Equal(t, 200, rr.Result().StatusCode)
}