From 9437f9bf6a302c0d4728e9047894ba7c1de3bb72 Mon Sep 17 00:00:00 2001
From: gaston chiu <42923070+gastonqiu@users.noreply.github.com>
Date: Thu, 13 Jul 2023 16:37:28 +0800
Subject: [PATCH] fix: Do not modify the origin request in RoundTripper (#4033)

---
 CHANGELOG.md                                  |  1 +
 .../net/http/otelhttp/transport.go            |  2 +-
 .../net/http/otelhttp/transport_test.go       | 29 +++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ceb54a30cea..6699566f901 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go
index 24c8cb264cb..e835cac12e4 100644
--- a/instrumentation/net/http/otelhttp/transport.go
+++ b/instrumentation/net/http/otelhttp/transport.go
@@ -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))
 
diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go
index 7cfe147a809..9c96ebe21a2 100644
--- a/instrumentation/net/http/otelhttp/transport_test.go
+++ b/instrumentation/net/http/otelhttp/transport_test.go
@@ -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)
+}