From 1d2c8f140f28680b37b1d7f83f0590eab67a2e13 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Mon, 9 Dec 2024 14:31:41 -0500 Subject: [PATCH] contrib/net/http: Make DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING false by default (#3010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dario Castañé --- contrib/google.golang.org/api/api_test.go | 8 ++++---- contrib/net/http/option.go | 2 +- contrib/net/http/roundtripper_test.go | 18 ++++++++---------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/contrib/google.golang.org/api/api_test.go b/contrib/google.golang.org/api/api_test.go index a9b8f54347..d5104b9690 100644 --- 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 index 22e63ee3b0..79144dc69c 100644 --- 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 index 4002647fe6..a15904fb7f 100644 --- 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)) }) }