From bc82905372e5ab9fb2da891480fd0a53c13167ad Mon Sep 17 00:00:00 2001 From: Nick Brandt Date: Thu, 19 Sep 2024 17:55:59 -0400 Subject: [PATCH] Support configurable error codes in contrib/net/http --- contrib/internal/httptrace/httptrace.go | 4 ---- contrib/net/http/http.go | 9 +++++---- contrib/net/http/option.go | 10 ++++++++++ contrib/net/http/trace.go | 7 +++++++ contrib/net/http/trace_test.go | 24 ++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 6c92fb80ac..9e05d3ebe4 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -9,7 +9,6 @@ package httptrace import ( "context" - "fmt" "net/http" "strconv" "strings" @@ -71,9 +70,6 @@ func FinishRequestSpan(s tracer.Span, status int, opts ...tracer.FinishOption) { statusStr = strconv.Itoa(status) } s.SetTag(ext.HTTPCode, statusStr) - if status >= 500 && status < 600 { - s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status))) - } s.Finish(opts...) } 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 cb658b4551..bf421df180 100644 --- a/contrib/net/http/option.go +++ b/contrib/net/http/option.go @@ -27,6 +27,7 @@ type config struct { finishOpts []ddtrace.FinishOption ignoreRequest func(*http.Request) bool resourceNamer func(*http.Request) string + isStatusError func(int) bool headerTags *internal.LockMap } @@ -50,6 +51,7 @@ func defaults(cfg *config) { } cfg.ignoreRequest = func(_ *http.Request) bool { return false } cfg.resourceNamer = func(_ *http.Request) string { return "" } + cfg.isStatusError = func(status int) bool { return status >= 500 && status < 600 } } // WithIgnoreRequest holds the function to use for determining if the @@ -118,6 +120,14 @@ func WithResourceNamer(namer func(req *http.Request) 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 + } +} + // NoDebugStack prevents stack traces from being attached to spans finishing // with an error. This is useful in situations where errors are frequent and // performance is critical. diff --git a/contrib/net/http/trace.go b/contrib/net/http/trace.go index b04867c931..b27d484360 100644 --- a/contrib/net/http/trace.go +++ b/contrib/net/http/trace.go @@ -8,6 +8,7 @@ package http // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http" //go:generate sh -c "go run make_responsewriter.go | gofmt > trace_gen.go" import ( + "fmt" "net/http" "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace" @@ -42,6 +43,9 @@ type ServeConfig struct { // in as /user/123 we'll have {"id": "123"}). This field is optional and is used for monitoring // by AppSec. It is only taken into account when AppSec is enabled. RouteParams map[string]string + // IsStatusError returns whether or not the passed status code should be marked as + // an error + IsStatusError func(int) bool // FinishOpts specifies any options to be used when finishing the request span. FinishOpts []ddtrace.FinishOption // SpanOpts specifies any options to be applied to the request starting span. @@ -67,6 +71,9 @@ func TraceAndServe(h http.Handler, w http.ResponseWriter, r *http.Request, cfg * span, ctx := httptrace.StartRequestSpan(r, opts...) rw, ddrw := wrapResponseWriter(w) defer func() { + if cfg.IsStatusError != nil && cfg.IsStatusError(ddrw.status) { + span.SetTag(ext.Error, fmt.Errorf("%d: %s", ddrw.status, http.StatusText(ddrw.status))) + } httptrace.FinishRequestSpan(span, ddrw.status, cfg.FinishOpts...) }() diff --git a/contrib/net/http/trace_test.go b/contrib/net/http/trace_test.go index fe8e062973..3611b7b84d 100644 --- a/contrib/net/http/trace_test.go +++ b/contrib/net/http/trace_test.go @@ -260,6 +260,29 @@ func TestTraceAndServe(t *testing.T) { assert.Equal("200", spans[0].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("empty", func(t *testing.T) { mt := mocktracer.Start() assert := assert.New(t) @@ -319,6 +342,7 @@ func TestTraceAndServe(t *testing.T) { assert.Equal("/path?", span.Tag(ext.HTTPURL)) assert.Equal("200", span.Tag(ext.HTTPCode)) }) + } func TestTraceAndServeHost(t *testing.T) {