contrib/net/http: Make DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING false by…

… default (#3010) Co-authored-by: Dario Castañé <dario.castane@datadoghq.com>
DataDog · Dec 9, 2024 · 1d2c8f1 · 1d2c8f1
1 parent abfec9a
commit 1d2c8f1
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 15 deletions.
diff --git a/contrib/google.golang.org/api/api_test.go b/contrib/google.golang.org/api/api_test.go
@@ -63,7 +63,7 @@ func TestBooks(t *testing.T) {
 	assert.Equal(t, "books.bookshelves.list", s0.Tag(ext.ResourceName))
 	assert.Equal(t, "400", s0.Tag(ext.HTTPCode))
 	assert.Equal(t, "GET", s0.Tag(ext.HTTPMethod))
-	assert.Equal(t, svc.BasePath+"books/v1/users/montana.banana/bookshelves?alt=json&prettyPrint=false", s0.Tag(ext.HTTPURL))
+	assert.Equal(t, svc.BasePath+"books/v1/users/montana.banana/bookshelves", s0.Tag(ext.HTTPURL))
 	assert.Equal(t, "google.golang.org/api", s0.Tag(ext.Component))
 	assert.Equal(t, ext.SpanKindClient, s0.Tag(ext.SpanKind))
 }
@@ -88,7 +88,7 @@ func TestCivicInfo(t *testing.T) {
 	assert.Equal(t, "civicinfo.representatives.representativeInfoByAddress", s0.Tag(ext.ResourceName))
 	assert.Equal(t, "400", s0.Tag(ext.HTTPCode))
 	assert.Equal(t, "GET", s0.Tag(ext.HTTPMethod))
-	assert.Equal(t, svc.BasePath+"civicinfo/v2/representatives?alt=json&prettyPrint=false", s0.Tag(ext.HTTPURL))
+	assert.Equal(t, svc.BasePath+"civicinfo/v2/representatives", s0.Tag(ext.HTTPURL))
 	assert.Equal(t, "google.golang.org/api", s0.Tag(ext.Component))
 	assert.Equal(t, ext.SpanKindClient, s0.Tag(ext.SpanKind))
 }
@@ -115,7 +115,7 @@ func TestURLShortener(t *testing.T) {
 	assert.Equal(t, "urlshortener.url.list", s0.Tag(ext.ResourceName))
 	assert.Equal(t, "400", s0.Tag(ext.HTTPCode))
 	assert.Equal(t, "GET", s0.Tag(ext.HTTPMethod))
-	assert.Equal(t, "https://www.googleapis.com/urlshortener/v1/url/history?alt=json&prettyPrint=false", s0.Tag(ext.HTTPURL))
+	assert.Equal(t, "https://www.googleapis.com/urlshortener/v1/url/history", s0.Tag(ext.HTTPURL))
 	assert.Equal(t, "google.golang.org/api", s0.Tag(ext.Component))
 	assert.Equal(t, ext.SpanKindClient, s0.Tag(ext.SpanKind))
 }
@@ -140,7 +140,7 @@ func TestWithEndpointMetadataDisabled(t *testing.T) {
 	assert.Equal(t, "GET civicinfo.googleapis.com", s0.Tag(ext.ResourceName))
 	assert.Equal(t, "400", s0.Tag(ext.HTTPCode))
 	assert.Equal(t, "GET", s0.Tag(ext.HTTPMethod))
-	assert.Equal(t, svc.BasePath+"civicinfo/v2/representatives?alt=json&prettyPrint=false", s0.Tag(ext.HTTPURL))
+	assert.Equal(t, svc.BasePath+"civicinfo/v2/representatives", s0.Tag(ext.HTTPURL))
 	assert.Equal(t, "google.golang.org/api", s0.Tag(ext.Component))
 	assert.Equal(t, ext.SpanKindClient, s0.Tag(ext.SpanKind))
 }

diff --git a/contrib/net/http/option.go b/contrib/net/http/option.go
@@ -174,7 +174,7 @@ func newRoundTripperConfig() *roundTripperConfig {
 		propagation:   true,
 		spanNamer:     defaultSpanNamer,
 		ignoreRequest: func(_ *http.Request) bool { return false },
-		queryString:   internal.BoolEnv(envClientQueryStringEnabled, true),
+		queryString:   internal.BoolEnv(envClientQueryStringEnabled, false),
 		isStatusError: isClientError,
 	}
 	v := os.Getenv(envClientErrorStatuses)

diff --git a/contrib/net/http/roundtripper_test.go b/contrib/net/http/roundtripper_test.go
@@ -579,19 +579,19 @@ func TestClientQueryString(t *testing.T) {
 		client := &http.Client{
 			Transport: rt,
 		}
-		resp, err := client.Get(s.URL + "/hello/world?querystring=xyz")
+		resp, err := client.Get(s.URL + "/hello/world?API_KEY=1234")
 		assert.Nil(t, err)
 		defer resp.Body.Close()
 		spans := mt.FinishedSpans()
 		assert.Len(t, spans, 1)
 
-		assert.Regexp(t, regexp.MustCompile(`^http://.*?/hello/world\?querystring=xyz$`), spans[0].Tag(ext.HTTPURL))
+		assert.Regexp(t, regexp.MustCompile(`^http://.*?/hello/world$`), spans[0].Tag(ext.HTTPURL))
 	})
-	t.Run("false", func(t *testing.T) {
+	t.Run("true", func(t *testing.T) {
 		mt := mocktracer.Start()
 		defer mt.Stop()
 
-		os.Setenv("DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING", "false")
+		os.Setenv("DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING", "true")
 		defer os.Unsetenv("DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING")
 
 		rt := WrapRoundTripper(http.DefaultTransport)
@@ -604,29 +604,27 @@ func TestClientQueryString(t *testing.T) {
 		spans := mt.FinishedSpans()
 		assert.Len(t, spans, 1)
 
-		assert.Regexp(t, regexp.MustCompile(`^http://.*?/hello/world$`), spans[0].Tag(ext.HTTPURL))
+		assert.Regexp(t, regexp.MustCompile(`^http://.*?/hello/world\?querystring=xyz$`), spans[0].Tag(ext.HTTPURL))
 	})
 	// DD_TRACE_HTTP_URL_QUERY_STRING_DISABLED applies only to server spans, not client
 	t.Run("Not impacted by DD_TRACE_HTTP_URL_QUERY_STRING_DISABLED", func(t *testing.T) {
 		mt := mocktracer.Start()
 		defer mt.Stop()
 
-		os.Setenv("DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING", "true")
-		os.Setenv("DD_TRACE_HTTP_URL_QUERY_STRING_DISABLED", "true")
-		defer os.Unsetenv("DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING")
+		os.Setenv("DD_TRACE_HTTP_URL_QUERY_STRING_DISABLED", "false")
 		defer os.Unsetenv("DD_TRACE_HTTP_URL_QUERY_STRING_DISABLED")
 
 		rt := WrapRoundTripper(http.DefaultTransport)
 		client := &http.Client{
 			Transport: rt,
 		}
-		resp, err := client.Get(s.URL + "/hello/world?querystring=xyz")
+		resp, err := client.Get(s.URL + "/hello/world?API_KEY=1234")
 		assert.Nil(t, err)
 		defer resp.Body.Close()
 		spans := mt.FinishedSpans()
 		assert.Len(t, spans, 1)
 
-		assert.Contains(t, spans[0].Tag(ext.HTTPURL), "/hello/world?querystring=xyz")
+		assert.Regexp(t, regexp.MustCompile(`^http://.*?/hello/world$`), spans[0].Tag(ext.HTTPURL))
 	})
 }