Apply changes to httptrace.BeforeHandle and labstack/echo integratoin

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.
@@ -59,7 +61,7 @@ func BeforeHandle(cfg *ServeConfig, w http.ResponseWriter, r *http.Request) (htt
 	rw, ddrw := wrapResponseWriter(w)
 	rt := r.WithContext(ctx)
 	closeSpan := func() {
-		FinishRequestSpan(span, ddrw.status, nil, cfg.FinishOpts...)
+		FinishRequestSpan(span, ddrw.status, cfg.isStatusError, cfg.FinishOpts...)
 	}
 	afterHandle := closeSpan
 	handled := false

contrib/internal/httptrace/httptrace.go

@@ -62,7 +62,7 @@ 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.
+// 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

contrib/labstack/echo.v4/echotrace_test.go

@@ -337,7 +337,7 @@ func TestStatusError(t *testing.T) {
 			defer mt.Stop()
 
 			if tt.envServerErrorStatusesVal != "" {
-				t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", tt.envServerErrorStatusesVal)
+				t.Setenv(envServerErrorStatuses, tt.envServerErrorStatusesVal)
 			}
 
 			router := echo.New()

contrib/labstack/echo.v4/option.go

@@ -45,8 +45,7 @@ type IgnoreRequestFunc func(c echo.Context) bool
 func defaults(cfg *config) {
 	cfg.serviceName = namingschema.ServiceName(defaultServiceName)
 	cfg.analyticsRate = math.NaN()
-	v := os.Getenv(envServerErrorStatuses)
-	if fn := httptrace.GetErrorCodesFromInput(v); fn != nil {
+	if fn := httptrace.GetErrorCodesFromInput(os.Getenv(envServerErrorStatuses)); fn != nil {
 		cfg.isStatusError = fn
 	} else {
 		cfg.isStatusError = isServerError

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)

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.