diff --git a/contrib/internal/httptrace/before_handle.go b/contrib/internal/httptrace/before_handle.go index ba0458f47d..f3be037d4b 100644 --- a/contrib/internal/httptrace/before_handle.go +++ b/contrib/internal/httptrace/before_handle.go @@ -36,7 +36,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 +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, cfg.isStatusError, cfg.FinishOpts...) + 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 index 5aedb032a4..2069e81a83 100644 --- 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 index 79144dc69c..a862fc605a 100644 --- 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 } @@ -86,6 +87,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(fn func(statusCode int) bool) Option { + return func(cfg *config) { + cfg.isStatusError = fn + } +} + // 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 index e00aaa95fe..f85a592861 100644 --- a/contrib/net/http/trace_test.go +++ b/contrib/net/http/trace_test.go @@ -299,6 +299,102 @@ func TestTraceAndServe(t *testing.T) { assert.Equal("/path?", span.Tag(ext.HTTPURL)) assert.Equal("200", span.Tag(ext.HTTPCode)) }) + + t.Run("integrationLevelErrorHandling", 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{ + 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("envLevelErrorHandling", func(t *testing.T) { + mt := mocktracer.Start() + assert := assert.New(t) + defer mt.Stop() + + t.Setenv("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500") + + cfg := &ServeConfig{ + Service: "service", + Resource: "resource", + } + + handler := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) // 500 + } + + r, err := http.NewRequest("GET", "/", nil) + assert.NoError(err) + w := httptest.NewRecorder() + TraceAndServe(http.HandlerFunc(handler), w, r, cfg) + + spans := mt.FinishedSpans() + assert.Len(spans, 1) + assert.Equal("500", spans[0].Tag(ext.HTTPCode)) + assert.Equal("500: Internal Server Error", spans[0].Tag(ext.Error).(error).Error()) + }) + + t.Run("integrationOverridesEnvConfig", 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") + + cfg := &ServeConfig{ + IsStatusError: func(i int) bool { return i == 400 }, + } + + // Test a 400 response, which should be reported as an error + handler400 := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) // 400 + } + + r400, err := http.NewRequest("GET", "/", nil) + assert.NoError(err) + w400 := httptest.NewRecorder() + TraceAndServe(http.HandlerFunc(handler400), w400, r400, cfg) + + 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()) + + // Reset the tracer + mt.Reset() + + // Test a 500 response, which should NOT be reported as an error, + // even though the environment variable says 500 is an error. + handler500 := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) // 500 + } + + r500, err := http.NewRequest("GET", "/", nil) + assert.NoError(err) + w500 := httptest.NewRecorder() + TraceAndServe(http.HandlerFunc(handler500), w500, r500, cfg) + + spans = mt.FinishedSpans() + assert.Len(spans, 1) + assert.Equal("500", spans[0].Tag(ext.HTTPCode)) + // Confirm that the span is NOT marked as an error. + assert.Nil(spans[0].Tag(ext.Error)) + }) } func TestTraceAndServeHost(t *testing.T) {