From cd1f9282dbc162a0c8d7b5729a9ba1ba2b29d051 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Tue, 6 Jan 2026 16:01:37 +0200 Subject: [PATCH 01/11] get body using request.GetBody to enable request reuse --- CHANGELOG.md | 1 + .../net/http/otelhttp/transport.go | 14 ++++++-- .../net/http/otelhttp/transport_test.go | 36 +++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index adc7ed2bcaa..0c259f69dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix panic when passing nil `TracerProvider` or `MeterProvider` to `WithTracerProvider` or `WithMeterProvider` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#8323) +- `Transport` in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` now supports reading request body multiple times for subsequent requests that reuse `http.Request`. (#8258) ### Removed diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 514ae6753b7..22120796276 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -117,11 +117,19 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. - // if request body is nil or NoBody, we don't want to mutate the body as it + // GetBody is preferred over direct access to Body if the function is set. + // if resulting body is nil or NoBody or err is not 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 or NoBody. - bw := request.NewBodyWrapper(r.Body, func(int64) {}) - if r.Body != nil && r.Body != http.NoBody { + var body io.ReadCloser + var err error + if r.GetBody != nil { + body, err = r.GetBody() + } else { + body = r.Body + } + bw := request.NewBodyWrapper(body, func(int64) {}) + if err == nil && body != nil && body != http.NoBody { r.Body = bw } diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index d4d20b2bfa5..5615769c65e 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -1114,3 +1114,39 @@ func BenchmarkTransportRoundTrip(b *testing.B) { }) } } + +func TestRequestBodyReuse(t *testing.T) { + var bodies []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + + defer r.Body.Close() + bodies = append(bodies, string(body)) + + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + transport := NewTransport(http.DefaultTransport) + client := http.Client{Transport: transport} + + body := bytes.NewBuffer([]byte("hello")) + + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, body) + require.NoError(t, err) + + for range 2 { + resp, err := client.Do(req) + assert.NoError(t, err, "failed to perform http req") + + _, err = io.Copy(io.Discard, resp.Body) + require.NoError(t, err) + + err = resp.Body.Close() + require.NoError(t, err) + } + + require.Len(t, bodies, 2, "expected exactly 2 req bodies") + assert.Equal(t, bodies[0], bodies[1], "req bodies do not match") +} From 02efc3fa2fe7911241f10c15ad128667379369d2 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian <100681941+adomaskizogian@users.noreply.github.com> Date: Tue, 6 Jan 2026 17:03:32 +0200 Subject: [PATCH 02/11] Fix comment for clarity on body mutation --- instrumentation/net/http/otelhttp/transport.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 22120796276..66584f709ae 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -118,7 +118,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. // GetBody is preferred over direct access to Body if the function is set. - // if resulting body is nil or NoBody or err is not nil, we don't want to mutate the body as it + // if resulting body is nil or is NoBody or err is not 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 or NoBody. var body io.ReadCloser From 5daa015fd898dca10070f22a180e40b6b2565ce6 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 10:49:44 +0200 Subject: [PATCH 03/11] terminate request transport early on GetBody error --- .../net/http/otelhttp/transport.go | 6 +- .../net/http/otelhttp/transport_test.go | 99 ++++++++++++++++++- 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 66584f709ae..3c74c9b09a2 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -128,6 +128,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { } else { body = r.Body } + bw := request.NewBodyWrapper(body, func(int64) {}) if err == nil && body != nil && body != http.NoBody { r.Body = bw @@ -136,7 +137,10 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { span.SetAttributes(t.semconv.RequestTraceAttrs(r)...) t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) - res, err := t.rt.RoundTrip(r) + var res *http.Response + if err == nil { + res, err = t.rt.RoundTrip(r) + } // Defer metrics recording function to record the metrics on error or no error. defer func() { diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index 5615769c65e..ce8a88fef59 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -1147,6 +1147,101 @@ func TestRequestBodyReuse(t *testing.T) { require.NoError(t, err) } - require.Len(t, bodies, 2, "expected exactly 2 req bodies") - assert.Equal(t, bodies[0], bodies[1], "req bodies do not match") + require.Len(t, bodies, 2, "should have recorded 2 request bodies") + assert.Equal(t, bodies[0], bodies[1], "request bodies should match") +} + +func TestGetBodyError(t *testing.T) { + t.Run("request not made when GetBody returns error", func(t *testing.T) { + requestMade := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requestMade = true + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + r, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, http.NoBody) + require.NoError(t, err) + + getBodyErr := errors.New("GetBody error") + r.GetBody = func() (io.ReadCloser, error) { + return nil, getBodyErr + } + + transport := NewTransport(http.DefaultTransport) + client := http.Client{Transport: transport} + + resp, err := client.Do(r) + require.Error(t, err) + assert.ErrorIs(t, err, getBodyErr) + assert.Nil(t, resp) + assert.False(t, requestMade, "request should not have been made to server") + }) + + t.Run("metrics collected when GetBody returns error", func(t *testing.T) { + ctx := t.Context() + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + defer func() { + err := provider.Shutdown(ctx) + assert.NoError(t, err) + }() + + r, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://errors.com", http.NoBody) + require.NoError(t, err) + + getBodyErr := errors.New("GetBody error") + r.GetBody = func() (io.ReadCloser, error) { + return nil, getBodyErr + } + + transport := NewTransport(http.DefaultTransport, WithMeterProvider(provider)) + client := http.Client{Transport: transport} + + resp, err := client.Do(r) + require.Error(t, err) + assert.ErrorIs(t, err, getBodyErr) + assert.Nil(t, resp) + + var rm metricdata.ResourceMetrics + err = reader.Collect(ctx, &rm) + require.NoError(t, err) + assert.Len(t, rm.ScopeMetrics, 1) + + var metricNames []string + for _, m := range rm.ScopeMetrics[0].Metrics { + metricNames = append(metricNames, m.Name) + } + assert.ElementsMatch(t, []string{"http.client.request.body.size", "http.client.request.duration"}, metricNames) + }) + + t.Run("span updated with error when GetBody returns error", func(t *testing.T) { + ctx := t.Context() + spanRecorder := tracetest.NewSpanRecorder() + provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(spanRecorder)) + + r, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://errors.com", http.NoBody) + require.NoError(t, err) + + getBodyErr := errors.New("GetBody error") + r.GetBody = func() (io.ReadCloser, error) { + return nil, getBodyErr + } + + transport := NewTransport(http.DefaultTransport, WithTracerProvider(provider)) + client := http.Client{Transport: transport} + + resp, err := client.Do(r) + require.Error(t, err) + assert.ErrorIs(t, err, getBodyErr) + assert.Nil(t, resp) + + spans := spanRecorder.Ended() + require.Len(t, spans, 1, "should have exactly one span") + + span := spans[0] + assert.False(t, span.EndTime().IsZero(), "span should be ended") + assert.Equal(t, codes.Error, span.Status().Code, "span should have error status code") + assert.Equal(t, getBodyErr.Error(), span.Status().Description, "span status should contain error message") + }) } From 8500ccecc16ad7848cf06ed074e7d59a99ebb775 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 10:54:19 +0200 Subject: [PATCH 04/11] re-order RoundTrip inject/set attributes for clarity on err path --- instrumentation/net/http/otelhttp/transport.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 3c74c9b09a2..6a5e544410d 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -117,6 +117,8 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. + span.SetAttributes(t.semconv.RequestTraceAttrs(r)...) + // GetBody is preferred over direct access to Body if the function is set. // if resulting body is nil or is NoBody or err is not nil, we don't want to mutate the body as it // will affect the identity of it in an unforeseeable way because we assert @@ -134,11 +136,9 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r.Body = bw } - span.SetAttributes(t.semconv.RequestTraceAttrs(r)...) - t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) - var res *http.Response if err == nil { + t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) res, err = t.rt.RoundTrip(r) } From 9d61c93282e7bf852d77f2c6c863852625d3f21d Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 11:01:11 +0200 Subject: [PATCH 05/11] Revert "re-order RoundTrip inject/set attributes for clarity on err path" This reverts commit 8500ccecc16ad7848cf06ed074e7d59a99ebb775. --- instrumentation/net/http/otelhttp/transport.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 6a5e544410d..3c74c9b09a2 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -117,8 +117,6 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. - span.SetAttributes(t.semconv.RequestTraceAttrs(r)...) - // GetBody is preferred over direct access to Body if the function is set. // if resulting body is nil or is NoBody or err is not nil, we don't want to mutate the body as it // will affect the identity of it in an unforeseeable way because we assert @@ -136,9 +134,11 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r.Body = bw } + span.SetAttributes(t.semconv.RequestTraceAttrs(r)...) + t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) + var res *http.Response if err == nil { - t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) res, err = t.rt.RoundTrip(r) } From eb4ee534a967162c6d96743580c9104f355f5955 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 11:01:19 +0200 Subject: [PATCH 06/11] Revert "terminate request transport early on GetBody error" This reverts commit 5daa015fd898dca10070f22a180e40b6b2565ce6. --- .../net/http/otelhttp/transport.go | 6 +- .../net/http/otelhttp/transport_test.go | 99 +------------------ 2 files changed, 3 insertions(+), 102 deletions(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 3c74c9b09a2..66584f709ae 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -128,7 +128,6 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { } else { body = r.Body } - bw := request.NewBodyWrapper(body, func(int64) {}) if err == nil && body != nil && body != http.NoBody { r.Body = bw @@ -137,10 +136,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { span.SetAttributes(t.semconv.RequestTraceAttrs(r)...) t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) - var res *http.Response - if err == nil { - res, err = t.rt.RoundTrip(r) - } + res, err := t.rt.RoundTrip(r) // Defer metrics recording function to record the metrics on error or no error. defer func() { diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index ce8a88fef59..5615769c65e 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -1147,101 +1147,6 @@ func TestRequestBodyReuse(t *testing.T) { require.NoError(t, err) } - require.Len(t, bodies, 2, "should have recorded 2 request bodies") - assert.Equal(t, bodies[0], bodies[1], "request bodies should match") -} - -func TestGetBodyError(t *testing.T) { - t.Run("request not made when GetBody returns error", func(t *testing.T) { - requestMade := false - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requestMade = true - w.WriteHeader(http.StatusOK) - })) - defer server.Close() - - r, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, http.NoBody) - require.NoError(t, err) - - getBodyErr := errors.New("GetBody error") - r.GetBody = func() (io.ReadCloser, error) { - return nil, getBodyErr - } - - transport := NewTransport(http.DefaultTransport) - client := http.Client{Transport: transport} - - resp, err := client.Do(r) - require.Error(t, err) - assert.ErrorIs(t, err, getBodyErr) - assert.Nil(t, resp) - assert.False(t, requestMade, "request should not have been made to server") - }) - - t.Run("metrics collected when GetBody returns error", func(t *testing.T) { - ctx := t.Context() - reader := sdkmetric.NewManualReader() - provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - defer func() { - err := provider.Shutdown(ctx) - assert.NoError(t, err) - }() - - r, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://errors.com", http.NoBody) - require.NoError(t, err) - - getBodyErr := errors.New("GetBody error") - r.GetBody = func() (io.ReadCloser, error) { - return nil, getBodyErr - } - - transport := NewTransport(http.DefaultTransport, WithMeterProvider(provider)) - client := http.Client{Transport: transport} - - resp, err := client.Do(r) - require.Error(t, err) - assert.ErrorIs(t, err, getBodyErr) - assert.Nil(t, resp) - - var rm metricdata.ResourceMetrics - err = reader.Collect(ctx, &rm) - require.NoError(t, err) - assert.Len(t, rm.ScopeMetrics, 1) - - var metricNames []string - for _, m := range rm.ScopeMetrics[0].Metrics { - metricNames = append(metricNames, m.Name) - } - assert.ElementsMatch(t, []string{"http.client.request.body.size", "http.client.request.duration"}, metricNames) - }) - - t.Run("span updated with error when GetBody returns error", func(t *testing.T) { - ctx := t.Context() - spanRecorder := tracetest.NewSpanRecorder() - provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(spanRecorder)) - - r, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://errors.com", http.NoBody) - require.NoError(t, err) - - getBodyErr := errors.New("GetBody error") - r.GetBody = func() (io.ReadCloser, error) { - return nil, getBodyErr - } - - transport := NewTransport(http.DefaultTransport, WithTracerProvider(provider)) - client := http.Client{Transport: transport} - - resp, err := client.Do(r) - require.Error(t, err) - assert.ErrorIs(t, err, getBodyErr) - assert.Nil(t, resp) - - spans := spanRecorder.Ended() - require.Len(t, spans, 1, "should have exactly one span") - - span := spans[0] - assert.False(t, span.EndTime().IsZero(), "span should be ended") - assert.Equal(t, codes.Error, span.Status().Code, "span should have error status code") - assert.Equal(t, getBodyErr.Error(), span.Status().Description, "span status should contain error message") - }) + require.Len(t, bodies, 2, "expected exactly 2 req bodies") + assert.Equal(t, bodies[0], bodies[1], "req bodies do not match") } From 2377ac948d12133a710d0b31414262e9333cef44 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 11:03:08 +0200 Subject: [PATCH 07/11] simplify err handling when getbody returns err --- instrumentation/net/http/otelhttp/transport.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 66584f709ae..4ee77f9aca9 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -122,14 +122,17 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { // 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. var body io.ReadCloser - var err error if r.GetBody != nil { - body, err = r.GetBody() + b, err := r.GetBody() + if err == nil { + body = b + } } else { body = r.Body } + bw := request.NewBodyWrapper(body, func(int64) {}) - if err == nil && body != nil && body != http.NoBody { + if body != nil && body != http.NoBody { r.Body = bw } From 0129686abe76d9830abc9239331e7a972115b555 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 11:12:47 +0200 Subject: [PATCH 08/11] clarify documentation on bodywritter in case of error --- instrumentation/net/http/otelhttp/transport.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 4ee77f9aca9..80e704306ee 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -118,7 +118,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. // GetBody is preferred over direct access to Body if the function is set. - // if resulting body is nil or is NoBody or err is not nil, we don't want to mutate the body as it + // if resulting body is nil or is NoBody or GetBody returns an error, 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. var body io.ReadCloser From d3055f87d14abcf62cd2c2664c22a9933db4ea49 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 13:45:10 +0200 Subject: [PATCH 09/11] call otel.Handle on GetBody returned err --- .../net/http/otelhttp/transport.go | 5 ++- .../net/http/otelhttp/transport_test.go | 42 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 80e704306ee..4b270cf8a8c 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -5,6 +5,7 @@ package otelhttp // import "go.opentelemetry.io/contrib/instrumentation/net/http import ( "context" + "fmt" "io" "net/http" "net/http/httptrace" @@ -124,7 +125,9 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { var body io.ReadCloser if r.GetBody != nil { b, err := r.GetBody() - if err == nil { + if err != nil { + otel.Handle(fmt.Errorf("http.Request GetBody returned an error: %w", err)) + } else { body = b } } else { diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index 5615769c65e..f87acd0f590 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" @@ -1150,3 +1151,44 @@ func TestRequestBodyReuse(t *testing.T) { require.Len(t, bodies, 2, "expected exactly 2 req bodies") assert.Equal(t, bodies[0], bodies[1], "req bodies do not match") } + +type testErrHandler struct { + err error +} + +func (h *testErrHandler) Handle(err error) { h.err = err } + +func TestHandleErrorOnGetBodyError(t *testing.T) { + errHandler := otel.GetErrorHandler() + + testErrHandler := &testErrHandler{} + + otel.SetErrorHandler(testErrHandler) + defer func() { + otel.SetErrorHandler(errHandler) + }() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + r, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, http.NoBody) + require.NoError(t, err) + + getBodyErr := errors.New("GetBody error") + r.GetBody = func() (io.ReadCloser, error) { + return nil, getBodyErr + } + + transport := NewTransport(http.DefaultTransport) + client := http.Client{Transport: transport} + + resp, err := client.Do(r) + require.NotNil(t, resp) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.ErrorContains(t, testErrHandler.err, "http.Request GetBody returned an error: GetBody error") +} From 124549d3654ad6b1a88cbe9aa3f65c547a5a7e72 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian <100681941+adomaskizogian@users.noreply.github.com> Date: Wed, 7 Jan 2026 14:37:25 +0200 Subject: [PATCH 10/11] Apply suggestion from @dmathieu Co-authored-by: Damien Mathieu <42@dmathieu.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b47a7544c6..dd79581846f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix panic when passing nil `TracerProvider` or `MeterProvider` to `WithTracerProvider` or `WithMeterProvider` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#8323) -- `Transport` in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` now supports reading request body multiple times for subsequent requests that reuse `http.Request`. (#8258) +- `Transport` in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` now supports reading request body multiple times for subsequent requests that reuse `http.Request`. (#8352) ### Removed From 0b3beb39e49497b8bdaaee7f12d443f28ca9b081 Mon Sep 17 00:00:00 2001 From: Adomas Kizogian Date: Wed, 7 Jan 2026 15:03:06 +0200 Subject: [PATCH 11/11] fallback to Body on GetBody error --- instrumentation/net/http/otelhttp/transport.go | 8 ++++---- .../net/http/otelhttp/transport_test.go | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 4b270cf8a8c..1d2f4022116 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -84,6 +84,8 @@ func defaultTransportFormatter(_ string, r *http.Request) string { // RoundTrip creates a Span and propagates its context via the provided request's headers // before handing the request to the configured base RoundTripper. The created span will // end when the response body is closed or when a read from the body returns io.EOF. +// If GetBody returns an error, the error is reported via otel.Handle and the request +// continues with the original Body. func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { requestStartTime := time.Now() for _, f := range t.filters { @@ -119,10 +121,10 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. // GetBody is preferred over direct access to Body if the function is set. - // if resulting body is nil or is NoBody or GetBody returns an error, we don't want to mutate the body as it + // If the resulting body is nil or is 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. - var body io.ReadCloser + body := r.Body if r.GetBody != nil { b, err := r.GetBody() if err != nil { @@ -130,8 +132,6 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { } else { body = b } - } else { - body = r.Body } bw := request.NewBodyWrapper(body, func(int64) {}) diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index f87acd0f590..bb03883c682 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -1168,12 +1168,21 @@ func TestHandleErrorOnGetBodyError(t *testing.T) { otel.SetErrorHandler(errHandler) }() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + var receivedBody string + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, err := io.ReadAll(r.Body) + assert.NoError(t, err) + + receivedBody = string(b) + w.WriteHeader(http.StatusOK) })) defer server.Close() - r, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, http.NoBody) + body := "plain text body" + + r, err := http.NewRequestWithContext(t.Context(), http.MethodPost, server.URL, strings.NewReader(body)) require.NoError(t, err) getBodyErr := errors.New("GetBody error") @@ -1190,5 +1199,6 @@ func TestHandleErrorOnGetBodyError(t *testing.T) { require.NoError(t, resp.Body.Close()) assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, body, receivedBody, "should fallback to http.Request body on http.Request GetBody error") assert.ErrorContains(t, testErrHandler.err, "http.Request GetBody returned an error: GetBody error") }