WIP: http/net error codes

DataDog · Dec 5, 2024 · cef2e37 · cef2e37
1 parent 67e7c56
commit cef2e37
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 6 deletions.
diff --git a/contrib/internal/httptrace/before_handle.go b/contrib/internal/httptrace/before_handle.go
@@ -6,6 +6,7 @@
 package httptrace
 
 import (
+	"fmt"
 	"net/http"
 
 	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
@@ -36,7 +37,7 @@ type ServeConfig struct {
 	// 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
+	IsStatusError func(int) bool
 }
 
 // BeforeHandle contains functionality that should be executed before a http.Handler runs.
@@ -61,7 +62,10 @@ 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, cfg.isStatusError, cfg.FinishOpts...)
+		if cfg.IsStatusError != nil && cfg.IsStatusError(ddrw.status) {
+			span.SetTag(ext.Error, fmt.Errorf("%d: %s", ddrw.status, http.StatusText(ddrw.status)))
+		}
+		FinishRequestSpan(span, ddrw.status, cfg.IsStatusError, cfg.FinishOpts...)
 	}
 	afterHandle := closeSpan
 	handled := false

diff --git a/contrib/net/http/http.go b/contrib/net/http/http.go
@@ -60,10 +60,11 @@ func (mux *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	copy(so, mux.cfg.spanOpts)
 	so = append(so, httptrace.HeaderTagsFromRequest(r, mux.cfg.headerTags))
 	TraceAndServe(mux.ServeMux, w, r, &ServeConfig{
-		Service:  mux.cfg.serviceName,
-		Resource: resource,
-		SpanOpts: so,
-		Route:    route,
+		Service:       mux.cfg.serviceName,
+		Resource:      resource,
+		SpanOpts:      so,
+		Route:         route,
+		IsStatusError: mux.cfg.isStatusError,
 	})
 }
 

diff --git a/contrib/net/http/option.go b/contrib/net/http/option.go
@@ -35,6 +35,7 @@ type config struct {
 	finishOpts    []ddtrace.FinishOption
 	ignoreRequest func(*http.Request) bool
 	resourceNamer func(*http.Request) string
+	isStatusError func(int) bool
 	headerTags    *internal.LockMap
 }
 
@@ -51,6 +52,7 @@ func defaults(cfg *config) {
 		cfg.analyticsRate = globalconfig.AnalyticsRate()
 	}
 	cfg.serviceName = namingschema.ServiceName(defaultServiceName)
+	cfg.isStatusError = func(status int) bool { return status >= 500 && status < 600 }
 	cfg.headerTags = globalconfig.HeaderTagMap()
 	cfg.spanOpts = []ddtrace.StartSpanOption{tracer.Measured()}
 	if !math.IsNaN(cfg.analyticsRate) {
@@ -86,6 +88,14 @@ func WithHeaderTags(headers []string) Option {
 	}
 }
 
+// WithStatusCheck sets a span to be an error if the passed function
+// returns true for a given status code.
+func WithStatusCheck(checker func(statusCode int) bool) Option {
+	return func(cfg *config) {
+		cfg.isStatusError = checker
+	}
+}
+
 // WithAnalytics enables Trace Analytics for all started spans.
 func WithAnalytics(on bool) MuxOption {
 	return func(cfg *config) {

diff --git a/contrib/net/http/trace_test.go b/contrib/net/http/trace_test.go
@@ -10,6 +10,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"testing"
 	"time"
 
@@ -299,6 +300,82 @@ func TestTraceAndServe(t *testing.T) {
 		assert.Equal("/path?<redacted>", span.Tag(ext.HTTPURL))
 		assert.Equal("200", span.Tag(ext.HTTPCode))
 	})
+
+	t.Run("isStatusError", func(t *testing.T) {
+		mt := mocktracer.Start()
+		assert := assert.New(t)
+		defer mt.Stop()
+
+		handler := func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusBadRequest)
+		}
+		r, err := http.NewRequest("GET", "/", nil)
+		assert.NoError(err)
+		w := httptest.NewRecorder()
+		TraceAndServe(http.HandlerFunc(handler), w, r, &ServeConfig{
+			Service:       "service",
+			Resource:      "resource",
+			IsStatusError: func(i int) bool { return i >= 400 },
+		})
+
+		spans := mt.FinishedSpans()
+		assert.Len(spans, 1)
+		assert.Equal("400", spans[0].Tag(ext.HTTPCode))
+		assert.Equal("400: Bad Request", spans[0].Tag(ext.Error).(error).Error())
+	})
+
+	t.Run("isStatusErrorEnv", func(t *testing.T) {
+		mt := mocktracer.Start()
+		assert := assert.New(t)
+		defer mt.Stop()
+
+		// Set environment variable to treat 500 as an error
+		t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500")
+
+		handler := func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusBadRequest)
+		}
+		r, err := http.NewRequest("GET", "/", nil)
+		assert.NoError(err)
+		w := httptest.NewRecorder()
+		TraceAndServe(http.HandlerFunc(handler), w, r, &ServeConfig{
+			Service:       "service",
+			Resource:      "resource",
+			IsStatusError: func(i int) bool { return i >= 400 },
+		})
+
+		spans := mt.FinishedSpans()
+		assert.Len(spans, 1)
+		assert.Equal("400", spans[0].Tag(ext.HTTPCode))
+		assert.Equal("400: Bad Request", spans[0].Tag(ext.Error).(error).Error())
+	})
+
+	t.Run("isStatusErrorClientError", func(t *testing.T) {
+		mt := mocktracer.Start()
+		assert := assert.New(t)
+		defer mt.Stop()
+
+		// Set environment variable to treat 500-510 as client errors
+		os.Setenv("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES", "500-510")
+		defer os.Unsetenv("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES") // Ensure cleanup after the test
+
+		handler := func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusBadRequest)
+		}
+		r, err := http.NewRequest("GET", "/", nil)
+		assert.NoError(err)
+		w := httptest.NewRecorder()
+		// Define ServeConfig with custom IsStatusError logic
+		TraceAndServe(http.HandlerFunc(handler), w, r, &ServeConfig{
+			Service:       "service",
+			Resource:      "resource",
+			IsStatusError: func(i int) bool { return i >= 400 && i < 500 }, // Treat only 400-499 as client errors
+		})
+		spans := mt.FinishedSpans()
+		assert.Len(spans, 1)
+		assert.Equal("400", spans[0].Tag(ext.HTTPCode))
+		assert.Equal("400: Bad Request", spans[0].Tag(ext.Error).(error).Error())
+	})
 }
 
 func TestTraceAndServeHost(t *testing.T) {