Skip to content
7 changes: 6 additions & 1 deletion contrib/gin-gonic/gin/gintrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package gin // import "github.com/DataDog/dd-trace-go/contrib/gin-gonic/gin/v2"

import (
"errors"
"fmt"
"math"

Expand Down Expand Up @@ -53,7 +54,11 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
opts = append(opts, httptrace.HeaderTagsFromRequest(c.Request, cfg.headerTags))
span, ctx, finishSpans := httptrace.StartRequestSpan(c.Request, opts...)
defer func() {
finishSpans(c.Writer.Status(), nil)
status := c.Writer.Status()
if cfg.useGinErrors && cfg.isStatusError(status) && len(c.Errors) > 0 {
finishSpans(status, cfg.isStatusError, tracer.WithError(errors.New(c.Errors.String())))
}
finishSpans(status, cfg.isStatusError)
}()

// pass the span through the request context
Expand Down
81 changes: 77 additions & 4 deletions contrib/gin-gonic/gin/gintrace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,84 @@ func TestError(t *testing.T) {
mt := mocktracer.Start()
defer mt.Stop()

// setup
router := gin.New()
router.Use(Middleware("foobar"))
responseErr := errors.New("oh no")

t.Run("server error", func(*testing.T) {
t.Run("server error - with error propagation", func(*testing.T) {
defer mt.Reset()

router := gin.New()
router.Use(Middleware("foobar", WithUseGinErrors()))

// configure a handler that returns an error and 5xx status code
router.GET("/server_err", func(c *gin.Context) {
c.AbortWithError(500, responseErr)
})
r := httptest.NewRequest("GET", "/server_err", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
response := w.Result()
defer response.Body.Close()
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(fmt.Sprintf("Error #01: %s\n", responseErr), span.Tag("gin.errors"))
// server errors set the ext.ErrorMsg tag
assert.Equal(fmt.Sprintf("Error #01: %s\n", responseErr), span.Tag(ext.ErrorMsg))
assert.Equal(ext.SpanKindServer, span.Tag(ext.SpanKind))
assert.Equal("gin-gonic/gin", span.Tag(ext.Component))
assert.Equal(componentName, span.Integration())
})

t.Run("server error - with error propagation - nil Errors in gin context", func(*testing.T) {
defer mt.Reset()

router := gin.New()
router.Use(Middleware("foobar", WithUseGinErrors()))

// configure a handler that returns an error and 5xx status code
router.GET("/server_err", func(c *gin.Context) {
c.AbortWithStatus(500)
})
r := httptest.NewRequest("GET", "/server_err", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
response := w.Result()
defer response.Body.Close()
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.Empty(span.Tag("gin.errors"))
// server errors set the ext.ErrorMsg tag
assert.Equal("500: Internal Server Error", span.Tag(ext.ErrorMsg))
assert.Equal(ext.SpanKindServer, span.Tag(ext.SpanKind))
assert.Equal("gin-gonic/gin", span.Tag(ext.Component))
assert.Equal(componentName, span.Integration())
})

t.Run("server error - without error propagation", func(*testing.T) {
defer mt.Reset()

router := gin.New()
router.Use(Middleware("foobar"))

// configure a handler that returns an error and 5xx status code
router.GET("/server_err", func(c *gin.Context) {
c.AbortWithError(500, responseErr)
Expand Down Expand Up @@ -219,6 +289,9 @@ func TestError(t *testing.T) {
t.Run("client error", func(*testing.T) {
defer mt.Reset()

router := gin.New()
router.Use(Middleware("foobar"))

// configure a handler that returns an error and 4xx status code
router.GET("/client_err", func(c *gin.Context) {
c.AbortWithError(418, responseErr)
Expand Down
24 changes: 24 additions & 0 deletions contrib/gin-gonic/gin/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ type config struct {
resourceNamer func(c *gin.Context) string
serviceName string
ignoreRequest func(c *gin.Context) bool
isStatusError func(statusCode int) bool
useGinErrors bool
headerTags instrumentation.HeaderTags
}

Expand All @@ -32,6 +34,8 @@ func newConfig(serviceName string) *config {
resourceNamer: defaultResourceNamer,
serviceName: serviceName,
ignoreRequest: func(_ *gin.Context) bool { return false },
isStatusError: isServerError,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value here should be nil so it fallbacks to use the DD_TRACE_HTTP_SERVER_ERROR_STATUSES env var (this behavior is inside the common httptrace lib we are using).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the same implementation as in the echo contrib: https://github.com/DataDog/dd-trace-go/blob/main/contrib/labstack/echo.v4/option.go#L52
Because I can't default it to nil, otherwise the code in gintrace would crash because the function is used directly.
I'm not sure this will fix the issue, and I can't find the test that's failing in the repo.
Maybe it's better to really default it to nil and check in the code if it's nil ? This way I would be sure that the behavior is the same as before

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok! this should work as well, thanks!

the test is in our shared cross-tracer testing suite here: https://github.com/DataDog/system-tests/blob/main/tests/test_config_consistency.py#L67-L98

useGinErrors: false,
headerTags: instr.HTTPHeadersAsTags(),
}
}
Expand Down Expand Up @@ -79,6 +83,26 @@ func WithResourceNamer(namer func(c *gin.Context) string) OptionFn {
}
}

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

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

// WithUseGinErrors enables the usage of gin's errors for the span instead of crafting generic errors from the status code.
// If there are multiple errors in the gin context, they will be all added to the span.
func WithUseGinErrors() OptionFn {
return func(cfg *config) {
cfg.useGinErrors = true
}
}

// WithHeaderTags enables the integration to attach HTTP request headers as span tags.
// Warning:
// Using this feature can risk exposing sensitive data such as authorization tokens to Datadog.
Expand Down
Loading