contrib/go-chi: Apply DD_TRACE_HTTP_SERVER_ERROR_STATUSES

contrib/go-chi/chi.v5/chi_test.go

@@ -16,6 +16,7 @@ import (
 	"testing"
 
 	pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec"
+	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/namingschematest"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
@@ -207,6 +208,40 @@ func TestError(t *testing.T) {
 		wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code))
 		assert.Equal(wantErr, span.Tag(ext.Error).(error).Error())
 	})
+	t.Run("envvar", func(t *testing.T) {
+		assert := assert.New(t)
+		t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "200")
+		mt := mocktracer.Start()
+		defer mt.Stop()
+
+		httptrace.ResetCfg()
+
+		router := chi.NewRouter()
+		router.Use(Middleware(
+			WithServiceName("foobar")))
+		code := 200
+		// a handler with an error and make the requests
+		router.Get("/err", func(w http.ResponseWriter, r *http.Request) {
+			http.Error(w, fmt.Sprintf("%d!", code), code)
+		})
+		r := httptest.NewRequest("GET", "/err", nil)
+		w := httptest.NewRecorder()
+		router.ServeHTTP(w, r)
+		response := w.Result()
+		defer response.Body.Close()
+		assert.Equal(response.StatusCode, code)
+
+		spans := mt.FinishedSpans()
+		assert.Len(spans, 1)
+		span := spans[0]
+		if span.Tag(ext.Error) == nil {
+			t.Fatal("Span missing error tags")
+		}
+		assertSpan(assert, span, code)
+		wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code))
+		assert.Equal(wantErr, span.Tag(ext.Error).(error).Error())
+
+	})
 	t.Run("integration overrides global", func(t *testing.T) {
 		assert := assert.New(t)
 		mt := mocktracer.Start()

contrib/go-chi/chi.v5/option.go

@@ -43,7 +43,6 @@ func defaults(cfg *config) {
 		cfg.analyticsRate = globalconfig.AnalyticsRate()
 	}
 	cfg.headerTags = globalconfig.HeaderTagMap()
-	cfg.isStatusError = isServerError
 	cfg.ignoreRequest = func(_ *http.Request) bool { return false }
 	cfg.modifyResourceName = func(s string) string { return s }
 	// for backward compatibility with modifyResourceName, initialize resourceName as nil.

contrib/go-chi/chi/chi_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec"
+	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/namingschematest"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
@@ -179,6 +180,39 @@ func TestError(t *testing.T) {
 		wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code))
 		assert.Equal(wantErr, span.Tag(ext.Error).(error).Error())
 	})
+	t.Run("envvar", func(t *testing.T) {
+		assert := assert.New(t)
+		t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "200")
+		mt := mocktracer.Start()
+		defer mt.Stop()
+
+		httptrace.ResetCfg()
+
+		router := chi.NewRouter()
+		router.Use(Middleware(
+			WithServiceName("foobar")))
+		code := 200
+		// a handler with an error and make the requests
+		router.Get("/err", func(w http.ResponseWriter, r *http.Request) {
+			http.Error(w, fmt.Sprintf("%d!", code), code)
+		})
+		r := httptest.NewRequest("GET", "/err", nil)
+		w := httptest.NewRecorder()
+		router.ServeHTTP(w, r)
+		response := w.Result()
+		defer response.Body.Close()
+		assert.Equal(response.StatusCode, code)
+
+		spans := mt.FinishedSpans()
+		assert.Len(spans, 1)
+		span := spans[0]
+		if span.Tag(ext.Error) == nil {
+			t.Fatal("Span missing error tags")
+		}
+		assertSpan(assert, span, code)
+		wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code))
+		assert.Equal(wantErr, span.Tag(ext.Error).(error).Error())
+	})
 	t.Run("integration overrides global", func(t *testing.T) {
 		assert := assert.New(t)
 		mt := mocktracer.Start()

contrib/go-chi/chi/option.go

@@ -41,7 +41,6 @@ func defaults(cfg *config) {
 		cfg.analyticsRate = globalconfig.AnalyticsRate()
 	}
 	cfg.headerTags = globalconfig.HeaderTagMap()
-	cfg.isStatusError = isServerError
 	cfg.ignoreRequest = func(_ *http.Request) bool { return false }
 	cfg.resourceNamer = func(r *http.Request) string {
 		resourceName := chi.RouteContext(r.Context()).RoutePattern()
@@ -99,10 +98,6 @@ func WithStatusCheck(fn func(statusCode int) bool) Option {
 	}
 }
 
-func isServerError(statusCode int) bool {
-	return statusCode >= 500 && statusCode < 600
-}
-
 // WithHeaderTags enables the integration to attach HTTP request headers as span tags.
 // Warning:
 // Using this feature can risk exposing sensitive data such as authorization tokens to Datadog.

contrib/internal/httptrace/config.go

@@ -6,6 +6,7 @@
 package httptrace
 
 import (
+	"fmt"
 	"os"
 	"regexp"
 	"strconv"
@@ -38,6 +39,12 @@ type config struct {
 	isStatusError     func(statusCode int) bool
 }
 
+func ResetCfg() config {
+	oldCfg := cfg
+	cfg = newConfig()
+	return oldCfg
+}
+
 func newConfig() config {
 	c := config{
 		queryString:       !internal.BoolEnv(envQueryStringDisabled, false),
@@ -47,6 +54,7 @@ func newConfig() config {
 	}
 	v := os.Getenv(envServerErrorStatuses)
 	if fn := GetErrorCodesFromInput(v); fn != nil {
+		fmt.Println("MTOFF: is 200 error?", fn(200))
 		c.isStatusError = fn
 	}
 	if s, ok := os.LookupEnv(envQueryStringRegexp); !ok {

contrib/internal/httptrace/httptrace.go

@@ -67,8 +67,10 @@ func FinishRequestSpan(s tracer.Span, status int, errorFn func(int) bool, opts .
 	var statusStr string
 	var fn func(int) bool
 	if errorFn == nil {
+		fmt.Println("MTOFF: errorFn is nil")
 		fn = cfg.isStatusError
 	} else {
+		fmt.Println("MTOFF: errorFn is not nil")
 		fn = errorFn
 	}
 	// if status is 0, treat it like 200 unless 0 was called out in DD_TRACE_HTTP_SERVER_ERROR_STATUSES
@@ -82,6 +84,7 @@ func FinishRequestSpan(s tracer.Span, status int, errorFn func(int) bool, opts .
 	} else {
 		statusStr = strconv.Itoa(status)
 		if fn(status) {
+			fmt.Println("MTOFF: FN 200 IS ERR")
 			s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status)))
 		}
 	}

contrib/internal/httptrace/httptrace_test.go

@@ -92,9 +92,8 @@ func TestConfiguredErrorStatuses(t *testing.T) {
 		os.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "199-399,400,501")
 
 		// reset config based on new DD_TRACE_HTTP_SERVER_ERROR_STATUSES value
-		oldConfig := cfg
+		oldConfig := ResetCfg()
 		defer func() { cfg = oldConfig }()
-		cfg = newConfig()
 
 		statuses := []int{0, 200, 400, 500}
 		r := httptest.NewRequest(http.MethodGet, "/test", nil)
@@ -124,9 +123,8 @@ func TestConfiguredErrorStatuses(t *testing.T) {
 		os.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "0")
 
 		// reset config based on new DD_TRACE_HTTP_SERVER_ERROR_STATUSES value
-		oldConfig := cfg
+		oldConfig := ResetCfg()
 		defer func() { cfg = oldConfig }()
-		cfg = newConfig()
 
 		r := httptest.NewRequest(http.MethodGet, "/test", nil)
 		sp, _ := StartRequestSpan(r)