diff --git a/contrib/go-chi/chi.v5/chi_test.go b/contrib/go-chi/chi.v5/chi_test.go index e74a03f066..80a62feef6 100644 --- a/contrib/go-chi/chi.v5/chi_test.go +++ b/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,41 @@ 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() + + // re-run config defaults based on new DD_TRACE_HTTP_SERVER_ERROR_STATUSES value + 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() diff --git a/contrib/go-chi/chi.v5/option.go b/contrib/go-chi/chi.v5/option.go index 6a67d7694a..7a82bfdc89 100644 --- a/contrib/go-chi/chi.v5/option.go +++ b/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. @@ -97,10 +96,6 @@ func WithStatusCheck(fn func(statusCode int) bool) Option { } } -func isServerError(statusCode int) bool { - return statusCode >= 500 && statusCode < 600 -} - // WithIgnoreRequest specifies a function to use for determining if the // incoming HTTP request tracing should be skipped. func WithIgnoreRequest(fn func(r *http.Request) bool) Option { diff --git a/contrib/go-chi/chi/chi_test.go b/contrib/go-chi/chi/chi_test.go index 8f0af00b0c..ecb077a478 100644 --- a/contrib/go-chi/chi/chi_test.go +++ b/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,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() + + // re-run config defaults based on new DD_TRACE_HTTP_SERVER_ERROR_STATUSES value + 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() diff --git a/contrib/go-chi/chi/option.go b/contrib/go-chi/chi/option.go index b78ca8f87c..9ddd340eab 100644 --- a/contrib/go-chi/chi/option.go +++ b/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. diff --git a/contrib/internal/httptrace/config.go b/contrib/internal/httptrace/config.go index 23ee0113d5..57d9a05c98 100644 --- a/contrib/internal/httptrace/config.go +++ b/contrib/internal/httptrace/config.go @@ -38,6 +38,11 @@ type config struct { isStatusError func(statusCode int) bool } +// ResetCfg sets local variable cfg back to its defaults (mainly useful for testing) +func ResetCfg() { + cfg = newConfig() +} + func newConfig() config { c := config{ queryString: !internal.BoolEnv(envQueryStringDisabled, false), diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index 1d9797e488..22de28c62d 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -91,10 +91,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 - defer func() { cfg = oldConfig }() - cfg = newConfig() + // re-run config defaults based on new DD_TRACE_HTTP_SERVER_ERROR_STATUSES value + ResetCfg() statuses := []int{0, 200, 400, 500} r := httptest.NewRequest(http.MethodGet, "/test", nil) @@ -123,10 +121,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 - defer func() { cfg = oldConfig }() - cfg = newConfig() + // re-run config defaults based on new DD_TRACE_HTTP_SERVER_ERROR_STATUSES value + ResetCfg() r := httptest.NewRequest(http.MethodGet, "/test", nil) sp, _ := StartRequestSpan(r)