Skip to content

Commit

Permalink
contrib/go-chi/chi: Added option to set custom error status check (#773)
Browse files Browse the repository at this point in the history
This commit introduces a new option for the chi tracing package called
WithIsErrorCheck which can be used to provide custom status code checking
logic.
  • Loading branch information
Hunrik authored Dec 11, 2020
1 parent e22c264 commit 45c7d74
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 28 deletions.
2 changes: 1 addition & 1 deletion contrib/go-chi/chi/chi.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler {
}
span.SetTag(ext.HTTPCode, strconv.Itoa(status))

if status >= 500 && status < 600 {
if cfg.isStatusError(status) {
// mark 5xx server error
span.SetTag(ext.Error, fmt.Errorf("%d: %s", status, http.StatusText(status)))
}
Expand Down
92 changes: 65 additions & 27 deletions contrib/go-chi/chi/chi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strconv"
"testing"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
Expand Down Expand Up @@ -99,37 +100,74 @@ func TestTrace200(t *testing.T) {
}

func TestError(t *testing.T) {
assert := assert.New(t)
mt := mocktracer.Start()
defer mt.Stop()
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]
assert.Equal("http.request", span.OperationName())
assert.Equal("foobar", span.Tag(ext.ServiceName))

// setup
router := chi.NewRouter()
router.Use(Middleware(WithServiceName("foobar")))
code := 500
wantErr := fmt.Sprintf("%d: %s", code, http.StatusText(code))
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())
}

// a handler with an error and make the requests
router.Get("/err", func(w http.ResponseWriter, r *http.Request) {
http.Error(w, fmt.Sprintf("%d!", code), code)
t.Run("default", func(t *testing.T) {
assert := assert.New(t)
mt := mocktracer.Start()
defer mt.Stop()

// setup
router := chi.NewRouter()
router.Use(Middleware(WithServiceName("foobar")))
code := 500

// a handler with an error and make the requests
router.Get("/err", func(w http.ResponseWriter, r *http.Request) {
http.Error(w, fmt.Sprintf("%d!", code), code)
})
r := httptest.NewRequest("GET", "/err", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
response := w.Result()
assert.Equal(response.StatusCode, code)

// verify the errors and status are correct
spans := mt.FinishedSpans()
assertSpan(assert, spans, code)
})
r := httptest.NewRequest("GET", "/err", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
response := w.Result()
assert.Equal(response.StatusCode, 500)

// verify the errors and status are correct
spans := mt.FinishedSpans()
assert.Len(spans, 1)
if len(spans) < 1 {
t.Fatalf("no spans")
}
span := spans[0]
assert.Equal("http.request", span.OperationName())
assert.Equal("foobar", span.Tag(ext.ServiceName))
assert.Equal("500", span.Tag(ext.HTTPCode))
assert.Equal(wantErr, span.Tag(ext.Error).(error).Error())
t.Run("custom", func(t *testing.T) {
assert := assert.New(t)
mt := mocktracer.Start()
defer mt.Stop()

// setup
router := chi.NewRouter()
router.Use(Middleware(
WithServiceName("foobar"),
WithStatusCheck(func(statusCode int) bool {
return statusCode >= 400
}),
))
code := 404
// a handler with an error and make the requests
router.Get("/err", func(w http.ResponseWriter, r *http.Request) {
http.Error(w, fmt.Sprintf("%d!", code), code)
})
r := httptest.NewRequest("GET", "/err", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
response := w.Result()
assert.Equal(response.StatusCode, code)

// verify the errors and status are correct
spans := mt.FinishedSpans()
assertSpan(assert, spans, code)
})
}

func TestGetSpanNotInstrumented(t *testing.T) {
Expand Down
14 changes: 14 additions & 0 deletions contrib/go-chi/chi/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type config struct {
serviceName string
spanOpts []ddtrace.StartSpanOption // additional span options to be applied
analyticsRate float64
isStatusError func(statusCode int) bool
}

// Option represents an option that can be passed to NewRouter.
Expand All @@ -32,6 +33,7 @@ func defaults(cfg *config) {
} else {
cfg.analyticsRate = globalconfig.AnalyticsRate()
}
cfg.isStatusError = isServerError
}

// WithServiceName sets the given service name for the router.
Expand Down Expand Up @@ -71,3 +73,15 @@ func WithAnalyticsRate(rate float64) Option {
}
}
}

// WithStatusCheck specifies a function fn which reports whether the passed
// statusCode should be considered an error.
func WithStatusCheck(fn func(statusCode int) bool) Option {
return func(cfg *config) {
cfg.isStatusError = fn
}
}

func isServerError(statusCode int) bool {
return statusCode >= 500 && statusCode < 600
}

0 comments on commit 45c7d74

Please sign in to comment.