Skip to content

Commit

Permalink
fix: Do not modify the origin request in RoundTripper (#4033)
Browse files Browse the repository at this point in the history
  • Loading branch information
gastonqiu committed Jul 13, 2023
1 parent 6d9a05e commit 9437f9b
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `go.opentelemetry.io/contrib/instrumentation/gopkg.in/macaron.v1/otelmacaron`
- `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace`
- `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`
- Do not modify the origin request in RoundTripper in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4033)

### Removed

Expand Down
2 changes: 1 addition & 1 deletion instrumentation/net/http/otelhttp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
ctx = httptrace.WithClientTrace(ctx, t.clientTrace(ctx))
}

r = r.WithContext(ctx)
r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request.
span.SetAttributes(semconvutil.HTTPClientRequest(r)...)
t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header))

Expand Down
29 changes: 29 additions & 0 deletions instrumentation/net/http/otelhttp/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,32 @@ func TestTransportProtocolSwitch(t *testing.T) {

assert.Implements(t, (*io.ReadWriteCloser)(nil), res.Body, "invalid body returned for protocol switch")
}

func TestTransportOriginRequestNotModify(t *testing.T) {
prop := propagation.TraceContext{}

ctx := context.Background()
sc := trace.NewSpanContext(trace.SpanContextConfig{
TraceID: trace.TraceID{0x01},
SpanID: trace.SpanID{0x01},
})
ctx = trace.ContextWithRemoteSpanContext(ctx, sc)

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
defer ts.Close()

r, err := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, http.NoBody)
require.NoError(t, err)

expectedRequest := r.Clone(r.Context())

c := http.Client{Transport: NewTransport(http.DefaultTransport, WithPropagators(prop))}
res, err := c.Do(r)
require.NoError(t, err)

t.Cleanup(func() { require.NoError(t, res.Body.Close()) })

assert.Equal(t, expectedRequest, r)
}

0 comments on commit 9437f9b

Please sign in to comment.