Skip to content
2 changes: 1 addition & 1 deletion contrib/gofiber/fiber.v2/fiber.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func Middleware(opts ...Option) func(c *fiber.Ctx) error {
span.SetTag(ext.Error, err)
} else if cfg.isStatusError(status) {
// mark 5xx server error
span.SetTag(ext.Error, fmt.Errorf("%d: %s", status, http.StatusText(status)))
span.SetTag(ext.ErrorNoStackTrace, fmt.Errorf("%d: %s", status, http.StatusText(status)))
}
return err
}
Expand Down
2 changes: 1 addition & 1 deletion contrib/net/http/internal/wrap/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func ObserveRoundTrip(cfg *config.RoundTripperConfig, req *http.Request) (*http.
span.SetTag(ext.HTTPCode, strconv.Itoa(resp.StatusCode))
if cfg.IsStatusError(resp.StatusCode) {
span.SetTag("http.errors", resp.Status)
span.SetTag(ext.Error, fmt.Errorf("%d: %s", resp.StatusCode, http.StatusText(resp.StatusCode)))
span.SetTag(ext.ErrorNoStackTrace, fmt.Errorf("%d: %s", resp.StatusCode, http.StatusText(resp.StatusCode)))
}
}

Expand Down
2 changes: 1 addition & 1 deletion contrib/valyala/fasthttp/fasthttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func WrapHandler(h fasthttp.RequestHandler, opts ...Option) fasthttp.RequestHand
span.SetTag(ext.ResourceName, cfg.resourceNamer(fctx))
status := fctx.Response.StatusCode()
if cfg.isStatusError(status) {
span.SetTag(ext.Error, fmt.Errorf("%d: %s", status, string(fctx.Response.Body())))
span.SetTag(ext.ErrorNoStackTrace, fmt.Errorf("%d: %s", status, string(fctx.Response.Body())))
}
span.SetTag(ext.HTTPCode, strconv.Itoa(status))
}
Expand Down
9 changes: 8 additions & 1 deletion ddtrace/ext/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,19 @@ const (
// ErrorType specifies the error type.
ErrorType = "error.type"

// ErrorStack specifies the stack dump.
// ErrorStack specifies the stack dump when the error is thrown.
ErrorStack = "error.stack"

// ErrorHandlingStack specifies the stack dump when the error is captured.
ErrorHandlingStack = "error.handling_stack"
Copy link
Member

Choose a reason for hiding this comment

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

We are adding these two new tags, what are the implications on the ingestion side? Especially for error tracking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new tags will change the behavior of error tags on spans; handling_stack should already be handled from the error tracking side, but we just haven't been taking advantage of it earlier. The fingerprints of spans will change after we release this PR, so we should give them a heads-up before so that they can adjust in the backend.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation ☺️


// ErrorDetails holds details about an error which implements a formatter.
// Deprecated: Use ErrorStack instead. This tag is not supported by Error Tracking.
ErrorDetails = "error.details"

// ErrorNoStackTrace is a tag that specifies that the error stack trace should not be captured.
ErrorNoStackTrace = "error.no_stack_trace"

// Environment specifies the environment to use with a trace.
Environment = "env"

Expand Down
23 changes: 14 additions & 9 deletions ddtrace/tracer/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@
noDebugStack: s.noDebugStack,
})
return
case ext.ErrorNoStackTrace:
s.setTagError(value, errorConfig{
noDebugStack: true,
})
return
case ext.Component:
integration, ok := value.(string)
if ok {
Expand Down Expand Up @@ -233,7 +238,7 @@
// and replace the string value with "<nil>", just as Sprintf does.
// Other panics should not be handled.
s.setMeta(key, "<nil>")
return

Check failure on line 241 in ddtrace/tracer/span.go

View workflow job for this annotation

GitHub Actions / checklocks

return with unexpected locks held (locks: &({param:s}.mu) exclusively)
}
panic(e)
}
Expand Down Expand Up @@ -430,23 +435,23 @@
setError(true)
s.setMeta(ext.ErrorMsg, v.Error())
s.setMeta(ext.ErrorType, reflect.TypeOf(v).String())
if cfg.noDebugStack {
return
}
switch err := v.(type) {
case xerrors.Formatter:
s.setMeta(ext.ErrorDetails, fmt.Sprintf("%+v", v))
s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v))
case fmt.Formatter:
// pkg/errors approach
s.setMeta(ext.ErrorDetails, fmt.Sprintf("%+v", v))
s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v))
case *errortrace.TracerError:
// instrumentation/errortrace approach
s.setMeta(ext.ErrorDetails, fmt.Sprintf("%+v", v))
if !cfg.noDebugStack {
s.setMeta(ext.ErrorStack, err.Format())
}
s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v))
s.setMeta(ext.ErrorHandlingStack, err.Format())
return
}
if !cfg.noDebugStack {
s.setMeta(ext.ErrorStack, takeStacktrace(cfg.stackFrames, cfg.stackSkip))
}
stack := takeStacktrace(cfg.stackFrames, cfg.stackSkip)
s.setMeta(ext.ErrorHandlingStack, stack)
case nil:
// no error
setError(false)
Expand Down
43 changes: 31 additions & 12 deletions ddtrace/tracer/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func TestSpanFinishWithError(t *testing.T) {
assert.Equal(int32(1), span.error)
assert.Equal("test error", span.meta[ext.ErrorMsg])
assert.Equal("*errors.errorString", span.meta[ext.ErrorType])
assert.NotEmpty(span.meta[ext.ErrorStack])
assert.NotEmpty(span.meta[ext.ErrorHandlingStack])
}

func TestSpanFinishWithErrorNoDebugStack(t *testing.T) {
Expand All @@ -365,9 +365,9 @@ func TestSpanFinishWithErrorStackFrames(t *testing.T) {
assert.Equal(int32(1), span.error)
assert.Equal("test error", span.meta[ext.ErrorMsg])
assert.Equal("*errors.errorString", span.meta[ext.ErrorType])
assert.Contains(span.meta[ext.ErrorStack], "tracer.TestSpanFinishWithErrorStackFrames")
assert.Contains(span.meta[ext.ErrorStack], "tracer.(*Span).Finish")
assert.Equal(strings.Count(span.meta[ext.ErrorStack], "\n\t"), 2)
assert.Contains(span.meta[ext.ErrorHandlingStack], "tracer.TestSpanFinishWithErrorStackFrames")
assert.Contains(span.meta[ext.ErrorHandlingStack], "tracer.(*Span).Finish")
assert.Equal(strings.Count(span.meta[ext.ErrorHandlingStack], "\n\t"), 2)
}

// nilStringer is used to test nil detection when setting tags.
Expand Down Expand Up @@ -413,7 +413,7 @@ func TestSpanSetTag(t *testing.T) {
assert.Equal(int32(1), span.error)
assert.Equal("abc", span.meta[ext.ErrorMsg])
assert.Equal("*errors.errorString", span.meta[ext.ErrorType])
assert.NotEmpty(span.meta[ext.ErrorStack])
assert.NotEmpty(span.meta[ext.ErrorHandlingStack])

span.SetTag(ext.Error, "something else")
assert.Equal(int32(1), span.error)
Expand Down Expand Up @@ -523,7 +523,7 @@ func TestSpanSetTagError(t *testing.T) {
t.Run("on", func(t *testing.T) {
span := newBasicSpan("web.request")
span.setTagError(errors.New("error value with trace"), errorConfig{noDebugStack: false})
assert.NotEmpty(t, span.meta[ext.ErrorStack])
assert.NotEmpty(t, span.meta[ext.ErrorHandlingStack])
})
}

Expand Down Expand Up @@ -793,10 +793,11 @@ func TestErrorStack(t *testing.T) {
assert.Equal("Something wrong", span.meta[ext.ErrorMsg])
assert.Equal("*errortrace.TracerError", span.meta[ext.ErrorType])

stack := span.meta[ext.ErrorStack]
stack := span.meta[ext.ErrorHandlingStack]
assert.NotEqual("", stack)
assert.Contains(stack, "tracer.TestErrorStack")
assert.Contains(stack, "tracer.createErrorTrace")

span.Finish()
})

Expand All @@ -813,10 +814,11 @@ func TestErrorStack(t *testing.T) {
assert.Equal("Something wrong", span.meta[ext.ErrorMsg])
assert.Equal("*errors.errorString", span.meta[ext.ErrorType])

stack := span.meta[ext.ErrorStack]
stack := span.meta[ext.ErrorHandlingStack]
assert.NotEqual("", stack)
assert.Contains(stack, "tracer.TestErrorStack")
assert.NotContains(stack, "tracer.createTestError") // this checks our old behavior
assert.NotContains(stack, "tracer.createTestError")

span.Finish()
})
}
Expand All @@ -835,7 +837,7 @@ func TestSpanError(t *testing.T) {
assert.Equal(int32(1), span.error)
assert.Equal("Something wrong", span.meta[ext.ErrorMsg])
assert.Equal("*errors.errorString", span.meta[ext.ErrorType])
assert.NotEqual("", span.meta[ext.ErrorStack])
assert.NotEqual("", span.meta[ext.ErrorHandlingStack])
span.Finish()

// operating on a finished span is a no-op
Expand Down Expand Up @@ -866,7 +868,7 @@ func TestSpanError_Typed(t *testing.T) {
assert.Equal(int32(1), span.error)
assert.Equal("boom", span.meta[ext.ErrorMsg])
assert.Equal("*tracer.boomError", span.meta[ext.ErrorType])
assert.NotEqual("", span.meta[ext.ErrorStack])
assert.NotEqual("", span.meta[ext.ErrorHandlingStack])
}

func TestSpanErrorNil(t *testing.T) {
Expand Down Expand Up @@ -978,7 +980,6 @@ func TestSpanErrorStackMetrics(t *testing.T) {
}

assert.Equal(0.0, telemetryClient.Count(telemetry.NamespaceTracers, "errorstack.source", []string{"source:takeStacktrace"}).Get())
assert.Equal(0.0, telemetryClient.Distribution(telemetry.NamespaceTracers, "errorstack.duration", []string{"source:takeStacktrace"}).Get())

assert.Equal(5.0, telemetryClient.Count(telemetry.NamespaceTracers, "errorstack.source", []string{"source:TracerError"}).Get())
if !windows {
Expand All @@ -987,6 +988,24 @@ func TestSpanErrorStackMetrics(t *testing.T) {
})
}

func TestSpanErrorNoStackTrace(t *testing.T) {
t.Run("default", func(t *testing.T) {
assert := assert.New(t)
tracer, _, _, stop, err := startTestTracer(t, WithDebugStack(true))
assert.Nil(err)
defer stop()

span := tracer.StartSpan("operation")
span.SetTag(ext.ErrorNoStackTrace, errors.New("test"))
span.Finish()

assert.Equal(int32(1), span.error)
assert.Equal("", span.meta[ext.ErrorStack])
assert.Equal("test", span.meta[ext.ErrorMsg])
assert.Equal("*errors.errorString", span.meta[ext.ErrorType])
})
}

func TestUniqueTagKeys(t *testing.T) {
assert := assert.New(t)
span := newBasicSpan("web.request")
Expand Down
4 changes: 2 additions & 2 deletions instrumentation/httptrace/httptrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,14 @@ func FinishRequestSpan(s *tracer.Span, status int, errorFn func(int) bool, opts
if status == 0 {
if fn(status) {
statusStr = "0"
s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status)))
s.SetTag(ext.ErrorNoStackTrace, fmt.Errorf("%s: %s", statusStr, http.StatusText(status)))
} else {
statusStr = "200"
}
} else {
statusStr = strconv.Itoa(status)
if fn(status) {
s.SetTag(ext.Error, fmt.Errorf("%s: %s", statusStr, http.StatusText(status)))
s.SetTag(ext.ErrorNoStackTrace, fmt.Errorf("%s: %s", statusStr, http.StatusText(status)))
}
}
fc := &tracer.FinishConfig{}
Expand Down
Loading