From e79577d696648847f9129aa1f390ae47ca905304 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Fri, 11 Nov 2022 10:23:40 -0500 Subject: [PATCH] treat http.NoBody the same as a nil body (#2983) --- CHANGELOG.md | 1 + instrumentation/net/http/otelhttp/handler.go | 8 ++++---- .../net/http/otelhttp/handler_test.go | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cf4495e0b9..3b624b21b85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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] diff --git a/instrumentation/net/http/otelhttp/handler.go b/instrumentation/net/http/otelhttp/handler.go index 506e7c06755..fd91e4162da 100644 --- a/instrumentation/net/http/otelhttp/handler.go +++ b/instrumentation/net/http/otelhttp/handler.go @@ -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 diff --git a/instrumentation/net/http/otelhttp/handler_test.go b/instrumentation/net/http/otelhttp/handler_test.go index c14c7d750fd..1a2683d348f 100644 --- a/instrumentation/net/http/otelhttp/handler_test.go +++ b/instrumentation/net/http/otelhttp/handler_test.go @@ -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) +}