contrib/internal/httptrace: Add DD_TRACE_SET_HTTP_ERROR_DISABLED env var

Signed-off-by: kazukousen <[email protected]>

contrib/internal/httptrace/config.go

@@ -22,6 +22,8 @@ const (
 	envQueryStringRegexp = "DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP"
 	// envTraceClientIPEnabled is the name of the env var used to specify whether or not to collect client ip in span tags
 	envTraceClientIPEnabled = "DD_TRACE_CLIENT_IP_ENABLED"
+	// envSetHTTPErrorDisabled is the name of the env var used to disabled to set HTTP 5xx error to span tags
+	envSetHTTPErrorDisabled = "DD_TRACE_SET_HTTP_ERROR_DISABLED"
 )
 
 // defaultQueryStringRegexp is the regexp used for query string obfuscation if `envQueryStringRegexp` is empty.
@@ -31,13 +33,15 @@ type config struct {
 	queryStringRegexp *regexp.Regexp // specifies the regexp to use for query string obfuscation.
 	queryString       bool           // reports whether the query string should be included in the URL span tag.
 	traceClientIP     bool
+	setHTTPError      bool
 }
 
 func newConfig() config {
 	c := config{
 		queryString:       !internal.BoolEnv(envQueryStringDisabled, false),
 		queryStringRegexp: defaultQueryStringRegexp,
 		traceClientIP:     internal.BoolEnv(envTraceClientIPEnabled, false),
+		setHTTPError:      !internal.BoolEnv(envSetHTTPErrorDisabled, false),
 	}
 	if s, ok := os.LookupEnv(envQueryStringRegexp); !ok {
 		return c

contrib/internal/httptrace/config_test.go

@@ -6,7 +6,6 @@
 package httptrace
 
 import (
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -16,6 +15,8 @@ func TestConfig(t *testing.T) {
 	defaultCfg := config{
 		queryString:       true,
 		queryStringRegexp: defaultQueryStringRegexp,
+		traceClientIP:     false,
+		setHTTPError:      true,
 	}
 	for _, tc := range []struct {
 		name string
@@ -38,40 +39,44 @@ func TestConfig(t *testing.T) {
 			name: "disable-query",
 			env:  map[string]string{envQueryStringDisabled: "true"},
 			cfg: config{
+				queryString:       false,
 				queryStringRegexp: defaultQueryStringRegexp,
+				traceClientIP:     false,
+				setHTTPError:      true,
 			},
 		},
 		{
 			name: "disable-query-obf",
 			env:  map[string]string{envQueryStringRegexp: ""},
 			cfg: config{
-				queryString: true,
+				queryString:       true,
+				queryStringRegexp: nil,
+				traceClientIP:     false,
+				setHTTPError:      true,
 			},
 		},
+		{
+			name: "disable-set-http-error",
+			env:  map[string]string{envSetHTTPErrorDisabled: "true"},
+			cfg: config{
+				queryString:       true,
+				queryStringRegexp: defaultQueryStringRegexp,
+				traceClientIP:     false,
+				setHTTPError:      false,
+			},
+		},
+		{
+			name: "disable-set-http-error-obf",
+			env:  map[string]string{envSetHTTPErrorDisabled: ""},
+			cfg:  defaultCfg,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			defer cleanEnv()()
 			for k, v := range tc.env {
-				os.Setenv(k, v)
+				t.Setenv(k, v)
 			}
 			c := newConfig()
-			require.Equal(t, tc.cfg.queryStringRegexp, c.queryStringRegexp)
-			require.Equal(t, tc.cfg.queryString, c.queryString)
+			require.Equal(t, tc.cfg, c)
 		})
 	}
 }
-
-func cleanEnv() func() {
-	env := map[string]string{
-		envQueryStringDisabled: os.Getenv(envQueryStringDisabled),
-		envQueryStringRegexp:   os.Getenv(envQueryStringRegexp),
-	}
-	for k := range env {
-		os.Unsetenv(k)
-	}
-	return func() {
-		for k, v := range env {
-			os.Setenv(k, v)
-		}
-	}
-}

contrib/internal/httptrace/httptrace.go

@@ -71,7 +71,7 @@ func FinishRequestSpan(s tracer.Span, status int, opts ...tracer.FinishOption) {
 		statusStr = strconv.Itoa(status)
 	}
 	s.SetTag(ext.HTTPCode, statusStr)
-	if status >= 500 && status < 600 {
+	if cfg.setHTTPError && status >= 500 && status < 600 {
 		s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status)))
 	}
 	s.Finish(opts...)

contrib/internal/httptrace/httptrace_test.go

@@ -18,6 +18,7 @@ import (
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/normalizer"
 
 	"github.com/DataDog/appsec-internal-go/netip"
@@ -146,6 +147,62 @@ func TestTraceClientIPFlag(t *testing.T) {
 	}
 }
 
+func TestSetHTTPErrorFlag(t *testing.T) {
+	mt := mocktracer.Start()
+	defer mt.Stop()
+
+	type testCase struct {
+		name               string
+		setHTTPErrorEnvVal string
+		expectSetHTTPError bool
+	}
+
+	for _, tc := range []testCase{
+		{
+			name:               "Set HTTP Error Disabled Env set to true",
+			setHTTPErrorEnvVal: "true",
+			expectSetHTTPError: false,
+		},
+		{
+			name:               "Set HTTP Error Disabled Env set to false",
+			setHTTPErrorEnvVal: "false",
+			expectSetHTTPError: true,
+		},
+		{
+			name:               "Set HTTP Error Disabled Env unset",
+			setHTTPErrorEnvVal: "",
+			expectSetHTTPError: true,
+		},
+		{
+			name:               "Set HTTP Error Disabled Env set to non-boolean value",
+			setHTTPErrorEnvVal: "asdadsasd",
+			expectSetHTTPError: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv(envSetHTTPErrorDisabled, tc.setHTTPErrorEnvVal)
+
+			// reset config based on new DD_TRACE_SET_HTTP_ERROR_DISABLED value
+			cfg = newConfig()
+
+			s := tracer.StartSpan(namingschema.NewHTTPServerOp().GetName(), nil)
+			FinishRequestSpan(s, http.StatusServiceUnavailable, nil)
+
+			spans := mt.FinishedSpans()
+			targetSpan := spans[0]
+
+			assert.Equal(t, "503", targetSpan.Tag(ext.HTTPCode))
+			if tc.expectSetHTTPError {
+				assert.Equal(t, "503: Service Unavailable", targetSpan.Tag(ext.Error).(error).Error())
+			} else {
+				assert.NotContains(t, targetSpan.Tags(), ext.Error)
+			}
+			mt.Reset()
+		})
+	}
+
+}
+
 func TestURLTag(t *testing.T) {
 	type URLTestCase struct {
 		name, expectedURL, host, port, path, query, fragment string