diff --git a/contrib/emicklei/go-restful.v3/restful.go b/contrib/emicklei/go-restful.v3/restful.go index 1d1e153c2d..10fa7dce4e 100644 --- a/contrib/emicklei/go-restful.v3/restful.go +++ b/contrib/emicklei/go-restful.v3/restful.go @@ -48,7 +48,7 @@ func FilterFunc(configOpts ...Option) restful.FilterFunction { spanOpts = append(spanOpts, httptrace.HeaderTagsFromRequest(req.Request, cfg.headerTags)) span, ctx := httptrace.StartRequestSpan(req.Request, spanOpts...) defer func() { - httptrace.FinishRequestSpan(span, resp.StatusCode(), tracer.WithError(resp.Error())) + httptrace.FinishRequestSpan(span, resp.StatusCode(), nil, tracer.WithError(resp.Error())) }() // pass the span through the request context diff --git a/contrib/emicklei/go-restful/restful.go b/contrib/emicklei/go-restful/restful.go index 164c3fd263..8dc6ff6fc2 100644 --- a/contrib/emicklei/go-restful/restful.go +++ b/contrib/emicklei/go-restful/restful.go @@ -48,7 +48,7 @@ func FilterFunc(configOpts ...Option) restful.FilterFunction { spanOpts = append(spanOpts, httptrace.HeaderTagsFromRequest(req.Request, cfg.headerTags)) span, ctx := httptrace.StartRequestSpan(req.Request, spanOpts...) defer func() { - httptrace.FinishRequestSpan(span, resp.StatusCode(), tracer.WithError(resp.Error())) + httptrace.FinishRequestSpan(span, resp.StatusCode(), nil, tracer.WithError(resp.Error())) }() // pass the span through the request context @@ -61,7 +61,7 @@ func FilterFunc(configOpts ...Option) restful.FilterFunction { func Filter(req *restful.Request, resp *restful.Response, chain *restful.FilterChain) { span, ctx := httptrace.StartRequestSpan(req.Request, tracer.ResourceName(req.SelectedRoutePath())) defer func() { - httptrace.FinishRequestSpan(span, resp.StatusCode(), tracer.WithError(resp.Error())) + httptrace.FinishRequestSpan(span, resp.StatusCode(), nil, tracer.WithError(resp.Error())) }() // pass the span through the request context diff --git a/contrib/gin-gonic/gin/gintrace.go b/contrib/gin-gonic/gin/gintrace.go index 1746c2689a..7a36dc1522 100644 --- a/contrib/gin-gonic/gin/gintrace.go +++ b/contrib/gin-gonic/gin/gintrace.go @@ -54,7 +54,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { opts = append(opts, httptrace.HeaderTagsFromRequest(c.Request, cfg.headerTags)) span, ctx := httptrace.StartRequestSpan(c.Request, opts...) defer func() { - httptrace.FinishRequestSpan(span, c.Writer.Status()) + httptrace.FinishRequestSpan(span, c.Writer.Status(), nil) }() // pass the span through the request context diff --git a/contrib/go-chi/chi.v5/chi.go b/contrib/go-chi/chi.v5/chi.go index 8caddf092a..73706d9765 100644 --- a/contrib/go-chi/chi.v5/chi.go +++ b/contrib/go-chi/chi.v5/chi.go @@ -7,7 +7,6 @@ package chi // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/go-chi/chi.v5" import ( - "fmt" "math" "net/http" @@ -56,11 +55,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler { ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) defer func() { status := ww.Status() - var opts []tracer.FinishOption - if cfg.isStatusError(status) { - opts = []tracer.FinishOption{tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status)))} - } - httptrace.FinishRequestSpan(span, status, opts...) + httptrace.FinishRequestSpan(span, status, cfg.isStatusError) }() // pass the span through the request context diff --git a/contrib/go-chi/chi.v5/chi_test.go b/contrib/go-chi/chi.v5/chi_test.go index b798d7ee6c..e74a03f066 100644 --- a/contrib/go-chi/chi.v5/chi_test.go +++ b/contrib/go-chi/chi.v5/chi_test.go @@ -138,19 +138,10 @@ func TestWithModifyResourceName(t *testing.T) { } func TestError(t *testing.T) { - assertSpan := func(assert *assert.Assertions, spans []mocktracer.Span, code int) { - assert.Len(spans, 1) - if len(spans) < 1 { - t.Fatalf("no spans") - } - span := spans[0] + assertSpan := func(assert *assert.Assertions, span mocktracer.Span, code int) { assert.Equal("http.request", span.OperationName()) assert.Equal("foobar", span.Tag(ext.ServiceName)) - assert.Equal(strconv.Itoa(code), span.Tag(ext.HTTPCode)) - - wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) - assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) } t.Run("default", func(t *testing.T) { @@ -176,7 +167,11 @@ func TestError(t *testing.T) { // verify the errors and status are correct spans := mt.FinishedSpans() - assertSpan(assert, spans, code) + assert.Len(spans, 1) + span := spans[0] + assertSpan(assert, span, code) + wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) + assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) }) t.Run("custom", func(t *testing.T) { @@ -206,7 +201,66 @@ func TestError(t *testing.T) { // verify the errors and status are correct spans := mt.FinishedSpans() - assertSpan(assert, spans, code) + assert.Len(spans, 1) + span := spans[0] + 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() + defer mt.Stop() + + t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500") + + // setup + router := chi.NewRouter() + router.Use(Middleware( + WithServiceName("foobar"), + WithStatusCheck(func(statusCode int) bool { + return statusCode == 404 + }), + )) + code := 404 + // a handler with an error and make the requests + router.Get("/404", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, fmt.Sprintf("%d!", code), code) + }) + r := httptest.NewRequest("GET", "/404", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + response := w.Result() + defer response.Body.Close() + assert.Equal(response.StatusCode, code) + + // verify the errors and status are correct + spans := mt.FinishedSpans() + assert.Len(spans, 1) + span := spans[0] + assertSpan(assert, span, code) + wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) + assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) + + mt.Reset() + + code = 500 + router.Get("/500", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, fmt.Sprintf("%d!", code), code) + }) + r = httptest.NewRequest("GET", "/500", nil) + w = httptest.NewRecorder() + router.ServeHTTP(w, r) + response = w.Result() + defer response.Body.Close() + assert.Equal(response.StatusCode, 500) + + // verify that span does not have error tag + spans = mt.FinishedSpans() + assert.Len(spans, 1) + span = spans[0] + assertSpan(assert, span, 500) + assert.Empty(span.Tag(ext.Error)) }) } diff --git a/contrib/go-chi/chi/chi.go b/contrib/go-chi/chi/chi.go index 370bf06cd2..1a5c2791c4 100644 --- a/contrib/go-chi/chi/chi.go +++ b/contrib/go-chi/chi/chi.go @@ -7,7 +7,6 @@ package chi // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/go-chi/chi" import ( - "fmt" "math" "net/http" @@ -56,11 +55,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler { ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) defer func() { status := ww.Status() - var opts []tracer.FinishOption - if cfg.isStatusError(status) { - opts = []tracer.FinishOption{tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status)))} - } - httptrace.FinishRequestSpan(span, status, opts...) + httptrace.FinishRequestSpan(span, status, cfg.isStatusError) }() // pass the span through the request context diff --git a/contrib/go-chi/chi/chi_test.go b/contrib/go-chi/chi/chi_test.go index 41c9d1c47c..8f0af00b0c 100644 --- a/contrib/go-chi/chi/chi_test.go +++ b/contrib/go-chi/chi/chi_test.go @@ -110,19 +110,10 @@ func TestTrace200(t *testing.T) { } func TestError(t *testing.T) { - assertSpan := func(assert *assert.Assertions, spans []mocktracer.Span, code int) { - assert.Len(spans, 1) - if len(spans) < 1 { - t.Fatalf("no spans") - } - span := spans[0] + assertSpan := func(assert *assert.Assertions, span mocktracer.Span, code int) { assert.Equal("http.request", span.OperationName()) assert.Equal("foobar", span.Tag(ext.ServiceName)) - assert.Equal(strconv.Itoa(code), span.Tag(ext.HTTPCode)) - - wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) - assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) } t.Run("default", func(t *testing.T) { @@ -148,7 +139,11 @@ func TestError(t *testing.T) { // verify the errors and status are correct spans := mt.FinishedSpans() - assertSpan(assert, spans, code) + assert.Len(spans, 1) + span := spans[0] + assertSpan(assert, span, code) + wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) + assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) }) t.Run("custom", func(t *testing.T) { @@ -178,7 +173,66 @@ func TestError(t *testing.T) { // verify the errors and status are correct spans := mt.FinishedSpans() - assertSpan(assert, spans, code) + assert.Len(spans, 1) + span := spans[0] + 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() + defer mt.Stop() + + t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500") + + // setup + router := chi.NewRouter() + router.Use(Middleware( + WithServiceName("foobar"), + WithStatusCheck(func(statusCode int) bool { + return statusCode == 404 + }), + )) + code := 404 + // a handler with an error and make the requests + router.Get("/404", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, fmt.Sprintf("%d!", code), code) + }) + r := httptest.NewRequest("GET", "/404", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + response := w.Result() + defer response.Body.Close() + assert.Equal(response.StatusCode, code) + + // verify the errors and status are correct + spans := mt.FinishedSpans() + assert.Len(spans, 1) + span := spans[0] + assertSpan(assert, span, code) + wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) + assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) + + mt.Reset() + + code = 500 + router.Get("/500", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, fmt.Sprintf("%d!", code), code) + }) + r = httptest.NewRequest("GET", "/500", nil) + w = httptest.NewRecorder() + router.ServeHTTP(w, r) + response = w.Result() + defer response.Body.Close() + assert.Equal(response.StatusCode, 500) + + // verify that span does not have error tag + spans = mt.FinishedSpans() + assert.Len(spans, 1) + span = spans[0] + assertSpan(assert, span, 500) + assert.Empty(span.Tag(ext.Error)) }) } diff --git a/contrib/internal/httptrace/before_handle.go b/contrib/internal/httptrace/before_handle.go index afd40726ed..ba0458f47d 100644 --- a/contrib/internal/httptrace/before_handle.go +++ b/contrib/internal/httptrace/before_handle.go @@ -35,6 +35,8 @@ type ServeConfig struct { FinishOpts []ddtrace.FinishOption // SpanOpts specifies any options to be applied to the request starting span. SpanOpts []ddtrace.StartSpanOption + // isStatusError allows customization of error code determination. + isStatusError func(int) bool } // BeforeHandle contains functionality that should be executed before a http.Handler runs. @@ -58,9 +60,8 @@ func BeforeHandle(cfg *ServeConfig, w http.ResponseWriter, r *http.Request) (htt span, ctx := StartRequestSpan(r, opts...) rw, ddrw := wrapResponseWriter(w) rt := r.WithContext(ctx) - closeSpan := func() { - FinishRequestSpan(span, ddrw.status, cfg.FinishOpts...) + FinishRequestSpan(span, ddrw.status, cfg.isStatusError, cfg.FinishOpts...) } afterHandle := closeSpan handled := false diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 6fa5a43242..92069e03d8 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -62,12 +62,18 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } // FinishRequestSpan finishes the given HTTP request span and sets the expected response-related tags such as the status -// code. Any further span finish option can be added with opts. -func FinishRequestSpan(s tracer.Span, status int, opts ...tracer.FinishOption) { +// code. If not nil, errorFn will override the isStatusError method on httptrace for determining error codes. Any further span finish option can be added with opts. +func FinishRequestSpan(s tracer.Span, status int, errorFn func(int) bool, opts ...tracer.FinishOption) { var statusStr string + var fn func(int) bool + if errorFn == nil { + fn = cfg.isStatusError + } else { + fn = errorFn + } // if status is 0, treat it like 200 unless 0 was called out in DD_TRACE_HTTP_SERVER_ERROR_STATUSES if status == 0 { - if cfg.isStatusError(status) { + if fn(status) { statusStr = "0" s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status))) } else { @@ -75,7 +81,7 @@ func FinishRequestSpan(s tracer.Span, status int, opts ...tracer.FinishOption) { } } else { statusStr = strconv.Itoa(status) - if cfg.isStatusError(status) { + if fn(status) { s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status))) } } diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index 8733a0a343..1d9797e488 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -100,7 +100,7 @@ func TestConfiguredErrorStatuses(t *testing.T) { r := httptest.NewRequest(http.MethodGet, "/test", nil) for i, status := range statuses { sp, _ := StartRequestSpan(r) - FinishRequestSpan(sp, status) + FinishRequestSpan(sp, status, nil) spans := mt.FinishedSpans() require.Len(t, spans, i+1) @@ -130,7 +130,7 @@ func TestConfiguredErrorStatuses(t *testing.T) { r := httptest.NewRequest(http.MethodGet, "/test", nil) sp, _ := StartRequestSpan(r) - FinishRequestSpan(sp, 0) + FinishRequestSpan(sp, 0, nil) spans := mt.FinishedSpans() require.Len(t, spans, 1) assert.Equal(t, "0", spans[0].Tag(ext.HTTPCode)) diff --git a/contrib/labstack/echo.v4/echotrace_test.go b/contrib/labstack/echo.v4/echotrace_test.go index 5f62cf1b25..60b1e1ffa8 100644 --- a/contrib/labstack/echo.v4/echotrace_test.go +++ b/contrib/labstack/echo.v4/echotrace_test.go @@ -232,10 +232,11 @@ func TestErrorHandling(t *testing.T) { func TestStatusError(t *testing.T) { for _, tt := range []struct { - isStatusError func(statusCode int) bool - err error - code string - handler func(c echo.Context) error + isStatusError func(statusCode int) bool + err error + code string + handler func(c echo.Context) error + envServerErrorStatusesVal string }{ { err: errors.New("oh no"), @@ -301,12 +302,44 @@ func TestStatusError(t *testing.T) { return nil }, }, + { + isStatusError: nil, + err: echo.NewHTTPError(http.StatusInternalServerError, "my error message"), + code: "500", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusInternalServerError, "my error message") + }, + envServerErrorStatusesVal: "500", + }, + // integration-level config applies regardless of envvar + { + isStatusError: func(statusCode int) bool { return statusCode == 400 }, + err: echo.NewHTTPError(http.StatusBadRequest, "my error message"), + code: "400", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusBadRequest, "my error message") + }, + envServerErrorStatusesVal: "500", + }, + // envvar impact is discarded if integration-level config has been applied + { + isStatusError: func(statusCode int) bool { return statusCode == 400 }, + err: nil, + code: "500", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusInternalServerError, "my error message") + }, + }, } { t.Run("", func(t *testing.T) { assert := assert.New(t) mt := mocktracer.Start() defer mt.Stop() + if tt.envServerErrorStatusesVal != "" { + t.Setenv(envServerErrorStatuses, tt.envServerErrorStatusesVal) + } + router := echo.New() opts := []Option{WithServiceName("foobar")} if tt.isStatusError != nil { diff --git a/contrib/labstack/echo.v4/option.go b/contrib/labstack/echo.v4/option.go index 2994d42c61..6e9d0e2d61 100644 --- a/contrib/labstack/echo.v4/option.go +++ b/contrib/labstack/echo.v4/option.go @@ -8,7 +8,9 @@ package echo import ( "errors" "math" + "os" + "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace" "gopkg.in/DataDog/dd-trace-go.v1/internal" "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" "gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema" @@ -19,6 +21,9 @@ import ( const defaultServiceName = "echo" +// envServerErrorStatuses is the name of the env var used to specify error status codes on http server spans +const envServerErrorStatuses = "DD_TRACE_HTTP_SERVER_ERROR_STATUSES" + type config struct { serviceName string analyticsRate float64 @@ -40,7 +45,11 @@ type IgnoreRequestFunc func(c echo.Context) bool func defaults(cfg *config) { cfg.serviceName = namingschema.ServiceName(defaultServiceName) cfg.analyticsRate = math.NaN() - cfg.isStatusError = isServerError + if fn := httptrace.GetErrorCodesFromInput(os.Getenv(envServerErrorStatuses)); fn != nil { + cfg.isStatusError = fn + } else { + cfg.isStatusError = isServerError + } cfg.headerTags = globalconfig.HeaderTagMap() cfg.tags = make(map[string]interface{}) cfg.translateError = func(err error) (*echo.HTTPError, bool) { diff --git a/contrib/labstack/echo/echotrace_test.go b/contrib/labstack/echo/echotrace_test.go index 8260bce03d..aa732d1129 100644 --- a/contrib/labstack/echo/echotrace_test.go +++ b/contrib/labstack/echo/echotrace_test.go @@ -316,10 +316,11 @@ func TestErrorHandling(t *testing.T) { func TestStatusError(t *testing.T) { for _, tt := range []struct { - isStatusError func(statusCode int) bool - err error - code string - handler func(c echo.Context) error + isStatusError func(statusCode int) bool + err error + code string + handler func(c echo.Context) error + envServerErrorStatusesVal string }{ { err: errors.New("oh no"), @@ -385,12 +386,44 @@ func TestStatusError(t *testing.T) { return nil }, }, + { + isStatusError: nil, + err: echo.NewHTTPError(http.StatusInternalServerError, "my error message"), + code: "500", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusInternalServerError, "my error message") + }, + envServerErrorStatusesVal: "500", + }, + // integration-level config applies regardless of envvar + { + isStatusError: func(statusCode int) bool { return statusCode == 400 }, + err: echo.NewHTTPError(http.StatusBadRequest, "my error message"), + code: "400", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusBadRequest, "my error message") + }, + envServerErrorStatusesVal: "500", + }, + // envvar impact is discarded if integration-level config has been applied + { + isStatusError: func(statusCode int) bool { return statusCode == 400 }, + err: nil, + code: "500", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusInternalServerError, "my error message") + }, + }, } { t.Run("", func(t *testing.T) { assert := assert.New(t) mt := mocktracer.Start() defer mt.Stop() + if tt.envServerErrorStatusesVal != "" { + t.Setenv(envServerErrorStatuses, tt.envServerErrorStatusesVal) + } + router := echo.New() opts := []Option{WithServiceName("foobar")} if tt.isStatusError != nil { @@ -403,7 +436,7 @@ func TestStatusError(t *testing.T) { router.ServeHTTP(w, r) spans := mt.FinishedSpans() - assert.Len(spans, 1) + require.Len(t, spans, 1) span := spans[0] assert.Equal("http.request", span.OperationName()) assert.Equal(ext.SpanTypeWeb, span.Tag(ext.SpanType)) @@ -413,8 +446,9 @@ func TestStatusError(t *testing.T) { assert.Equal("GET", span.Tag(ext.HTTPMethod)) err := span.Tag(ext.Error) if tt.err != nil { - assert.NotNil(err) - require.NotNil(t, span.Tag(ext.Error)) + if !assert.NotNil(err) { + return + } assert.Equal(tt.err.Error(), err.(error).Error()) } else { assert.Nil(err) diff --git a/contrib/labstack/echo/option.go b/contrib/labstack/echo/option.go index d65a9d2772..a04229c87f 100644 --- a/contrib/labstack/echo/option.go +++ b/contrib/labstack/echo/option.go @@ -7,7 +7,9 @@ package echo import ( "math" + "os" + "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace" "gopkg.in/DataDog/dd-trace-go.v1/internal" "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" "gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema" @@ -16,6 +18,9 @@ import ( const defaultServiceName = "echo" +// envServerErrorStatuses is the name of the env var used to specify error status codes on http server spans +const envServerErrorStatuses = "DD_TRACE_HTTP_SERVER_ERROR_STATUSES" + type config struct { serviceName string analyticsRate float64 @@ -35,7 +40,11 @@ func defaults(cfg *config) { cfg.analyticsRate = math.NaN() } cfg.headerTags = globalconfig.HeaderTagMap() - cfg.isStatusError = isServerError + if fn := httptrace.GetErrorCodesFromInput(os.Getenv(envServerErrorStatuses)); fn != nil { + cfg.isStatusError = fn + } else { + cfg.isStatusError = isServerError + } } // WithServiceName sets the given service name for the system. diff --git a/contrib/urfave/negroni/negroni.go b/contrib/urfave/negroni/negroni.go index 2659d568f7..651188df1a 100644 --- a/contrib/urfave/negroni/negroni.go +++ b/contrib/urfave/negroni/negroni.go @@ -56,7 +56,7 @@ func (m *DatadogMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request, ne opts = []tracer.FinishOption{tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status)))} } } - httptrace.FinishRequestSpan(span, status, opts...) + httptrace.FinishRequestSpan(span, status, m.cfg.isStatusError, opts...) }() next(w, r.WithContext(ctx)) diff --git a/contrib/urfave/negroni/negroni_test.go b/contrib/urfave/negroni/negroni_test.go index 2fc7c91340..406ce0d4b3 100644 --- a/contrib/urfave/negroni/negroni_test.go +++ b/contrib/urfave/negroni/negroni_test.go @@ -220,13 +220,10 @@ func TestTrace200(t *testing.T) { } func TestError(t *testing.T) { - assertSpan := func(assert *assert.Assertions, spans []mocktracer.Span, code int) { - assert.Len(spans, 1) - span := spans[0] + assertSpan := func(assert *assert.Assertions, span mocktracer.Span, code int) { assert.Equal("http.request", span.OperationName()) + assert.Equal("negroni.router", span.Tag(ext.ServiceName)) assert.Equal(strconv.Itoa(code), span.Tag(ext.HTTPCode)) - wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) - assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) } t.Run("default", func(t *testing.T) { @@ -256,7 +253,11 @@ func TestError(t *testing.T) { // verify the errors and status are correct spans := mt.FinishedSpans() - assertSpan(assert, spans, code) + assert.Len(spans, 1) + span := spans[0] + assertSpan(assert, span, code) + wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) + assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) }) t.Run("custom", func(t *testing.T) { @@ -287,7 +288,67 @@ func TestError(t *testing.T) { // verify the errors and status are correct spans := mt.FinishedSpans() - assertSpan(assert, spans, code) + assert.Len(spans, 1) + span := spans[0] + 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() + defer mt.Stop() + + t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500") + + // setup + router := negroni.New() + code := 404 + router.Use(Middleware(WithStatusCheck(func(statusCode int) bool { + return statusCode == 404 + }))) + + // a handler with an error and make the requests + mux := http.NewServeMux() + mux.HandleFunc("/404", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, fmt.Sprintf("%d!", code), code) + }) + router.UseHandler(mux) + r := httptest.NewRequest("GET", "/404", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + response := w.Result() + defer response.Body.Close() + assert.Equal(response.StatusCode, code) + + // verify the errors and status are correct + spans := mt.FinishedSpans() + assert.Len(spans, 1) + span := spans[0] + assertSpan(assert, span, code) + wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code)) + assert.Equal(wantErr, span.Tag(ext.Error).(error).Error()) + + mt.Reset() + + code = 500 + mux.HandleFunc("/500", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, fmt.Sprintf("%d!", code), code) + }) + r = httptest.NewRequest("GET", "/500", nil) + w = httptest.NewRecorder() + router.ServeHTTP(w, r) + response = w.Result() + defer response.Body.Close() + assert.Equal(response.StatusCode, 500) + + // verify that span does not have error tag + spans = mt.FinishedSpans() + assert.Len(spans, 1) + span = spans[0] + assertSpan(assert, span, 500) + assert.Empty(span.Tag(ext.Error)) }) }