From 86836bf1e08f34150f8491223fa4d033b0528692 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Fri, 1 Aug 2025 16:25:58 -0400 Subject: [PATCH 1/7] do not record error stack for status code errors --- contrib/gofiber/fiber.v2/fiber.go | 2 +- contrib/net/http/internal/wrap/roundtrip.go | 2 +- contrib/valyala/fasthttp/fasthttp.go | 2 +- ddtrace/ext/tags.go | 3 +++ instrumentation/httptrace/httptrace.go | 4 ++-- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/gofiber/fiber.v2/fiber.go b/contrib/gofiber/fiber.v2/fiber.go index 701ebe8292..e910181528 100644 --- a/contrib/gofiber/fiber.v2/fiber.go +++ b/contrib/gofiber/fiber.v2/fiber.go @@ -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 } diff --git a/contrib/net/http/internal/wrap/roundtrip.go b/contrib/net/http/internal/wrap/roundtrip.go index 3d171be322..91d080c254 100644 --- a/contrib/net/http/internal/wrap/roundtrip.go +++ b/contrib/net/http/internal/wrap/roundtrip.go @@ -104,7 +104,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))) } } diff --git a/contrib/valyala/fasthttp/fasthttp.go b/contrib/valyala/fasthttp/fasthttp.go index f740b2e895..1cea3ac3c2 100644 --- a/contrib/valyala/fasthttp/fasthttp.go +++ b/contrib/valyala/fasthttp/fasthttp.go @@ -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)) } diff --git a/ddtrace/ext/tags.go b/ddtrace/ext/tags.go index b4eb5292e4..9638c40ea8 100644 --- a/ddtrace/ext/tags.go +++ b/ddtrace/ext/tags.go @@ -93,6 +93,9 @@ const ( // ErrorDetails holds details about an error which implements a formatter. 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" diff --git a/instrumentation/httptrace/httptrace.go b/instrumentation/httptrace/httptrace.go index c9cfe8d560..ce0bd67b94 100644 --- a/instrumentation/httptrace/httptrace.go +++ b/instrumentation/httptrace/httptrace.go @@ -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{} From 830b0f67033dcd0f604d06b7469c2db7dfb4726e Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Fri, 1 Aug 2025 16:26:23 -0400 Subject: [PATCH 2/7] support new error tag type --- ddtrace/tracer/span.go | 5 +++++ ddtrace/tracer/span_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index 080a9ab4d8..c9c8a231f9 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -197,6 +197,11 @@ func (s *Span) SetTag(key string, value interface{}) { noDebugStack: s.noDebugStack, }) return + case ext.ErrorNoStackTrace: + s.setTagError(value, errorConfig{ + noDebugStack: true, + }) + return case ext.Component: integration, ok := value.(string) if ok { diff --git a/ddtrace/tracer/span_test.go b/ddtrace/tracer/span_test.go index 6b42b80305..0f3720b328 100644 --- a/ddtrace/tracer/span_test.go +++ b/ddtrace/tracer/span_test.go @@ -987,6 +987,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") From e7cbad2d02f24d10194f7e88cd0374dd905cdc43 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 5 Aug 2025 13:22:37 -0400 Subject: [PATCH 3/7] add handler stack --- ddtrace/ext/tags.go | 5 ++++- ddtrace/tracer/span.go | 7 +++++-- ddtrace/tracer/span_test.go | 9 ++++++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/ddtrace/ext/tags.go b/ddtrace/ext/tags.go index 9638c40ea8..a4ecd8f07e 100644 --- a/ddtrace/ext/tags.go +++ b/ddtrace/ext/tags.go @@ -87,9 +87,12 @@ 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" + // ErrorDetails holds details about an error which implements a formatter. ErrorDetails = "error.details" diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index c9c8a231f9..ef7d62da54 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -446,11 +446,15 @@ func (s *Span) setTagError(value interface{}, cfg errorConfig) { s.setMeta(ext.ErrorDetails, fmt.Sprintf("%+v", v)) if !cfg.noDebugStack { s.setMeta(ext.ErrorStack, err.Format()) + s.setMeta(ext.ErrorHandlingStack, takeStacktrace(cfg.stackFrames, cfg.stackSkip)) } return } if !cfg.noDebugStack { - s.setMeta(ext.ErrorStack, takeStacktrace(cfg.stackFrames, cfg.stackSkip)) + telemetry.Count(telemetry.NamespaceTracers, "errorstack.source", []string{"source:takeStacktrace"}).Submit(1) + stack := takeStacktrace(cfg.stackFrames, cfg.stackSkip) + s.setMeta(ext.ErrorStack, stack) + s.setMeta(ext.ErrorHandlingStack, stack) } case nil: // no error @@ -468,7 +472,6 @@ const defaultStackLength = 32 // takeStacktrace takes a stack trace of maximum n entries, skipping the first skip entries. // If n is 0, up to 20 entries are retrieved. func takeStacktrace(n, skip uint) string { - telemetry.Count(telemetry.NamespaceTracers, "errorstack.source", []string{"source:takeStacktrace"}).Submit(1) now := time.Now() defer func() { dur := float64(time.Since(now)) diff --git a/ddtrace/tracer/span_test.go b/ddtrace/tracer/span_test.go index 0f3720b328..0b94fd9f97 100644 --- a/ddtrace/tracer/span_test.go +++ b/ddtrace/tracer/span_test.go @@ -797,6 +797,10 @@ func TestErrorStack(t *testing.T) { assert.NotEqual("", stack) assert.Contains(stack, "tracer.TestErrorStack") assert.Contains(stack, "tracer.createErrorTrace") + + handlingStack := span.meta[ext.ErrorHandlingStack] + assert.NotEqual("", handlingStack) + assert.NotEqual(stack, handlingStack) span.Finish() }) @@ -817,6 +821,10 @@ func TestErrorStack(t *testing.T) { assert.NotEqual("", stack) assert.Contains(stack, "tracer.TestErrorStack") assert.NotContains(stack, "tracer.createTestError") // this checks our old behavior + + handlingStack := span.meta[ext.ErrorHandlingStack] + assert.NotEqual("", handlingStack) + assert.Equal(stack, handlingStack) span.Finish() }) } @@ -978,7 +986,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 { From 5e13bf391f23557bd20cf26d23a3c19879710a39 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 5 Aug 2025 13:29:33 -0400 Subject: [PATCH 4/7] change tag name to match convention --- ddtrace/ext/tags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/ext/tags.go b/ddtrace/ext/tags.go index a4ecd8f07e..a2251011cb 100644 --- a/ddtrace/ext/tags.go +++ b/ddtrace/ext/tags.go @@ -91,7 +91,7 @@ const ( ErrorStack = "error.stack" // ErrorHandlingStack specifies the stack dump when the error is captured. - ErrorHandlingStack = "error.handling.stack" + ErrorHandlingStack = "error.handling_stack" // ErrorDetails holds details about an error which implements a formatter. ErrorDetails = "error.details" From 65d5441527abe1e63c25e8e04c2aa39746495ac4 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 23 Sep 2025 10:06:58 -0400 Subject: [PATCH 5/7] completely move error.stack to error.handling_stack --- ddtrace/tracer/span.go | 16 +++++++++------- ddtrace/tracer/span_test.go | 28 +++++++++++----------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index ef7d62da54..9378d4aadb 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -437,23 +437,24 @@ func (s *Span) setTagError(value interface{}, cfg errorConfig) { s.setMeta(ext.ErrorType, reflect.TypeOf(v).String()) switch err := v.(type) { case xerrors.Formatter: - s.setMeta(ext.ErrorDetails, fmt.Sprintf("%+v", v)) + if !cfg.noDebugStack { + s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) + } case fmt.Formatter: // pkg/errors approach - s.setMeta(ext.ErrorDetails, fmt.Sprintf("%+v", v)) + if !cfg.noDebugStack { + 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.ErrorHandlingStack, takeStacktrace(cfg.stackFrames, cfg.stackSkip)) + s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) + s.setMeta(ext.ErrorHandlingStack, err.Format()) } return } if !cfg.noDebugStack { - telemetry.Count(telemetry.NamespaceTracers, "errorstack.source", []string{"source:takeStacktrace"}).Submit(1) stack := takeStacktrace(cfg.stackFrames, cfg.stackSkip) - s.setMeta(ext.ErrorStack, stack) s.setMeta(ext.ErrorHandlingStack, stack) } case nil: @@ -472,6 +473,7 @@ const defaultStackLength = 32 // takeStacktrace takes a stack trace of maximum n entries, skipping the first skip entries. // If n is 0, up to 20 entries are retrieved. func takeStacktrace(n, skip uint) string { + telemetry.Count(telemetry.NamespaceTracers, "errorstack.source", []string{"source:takeStacktrace"}).Submit(1) now := time.Now() defer func() { dur := float64(time.Since(now)) diff --git a/ddtrace/tracer/span_test.go b/ddtrace/tracer/span_test.go index 0b94fd9f97..dadc423be6 100644 --- a/ddtrace/tracer/span_test.go +++ b/ddtrace/tracer/span_test.go @@ -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) { @@ -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. @@ -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) @@ -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]) }) } @@ -793,14 +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") - handlingStack := span.meta[ext.ErrorHandlingStack] - assert.NotEqual("", handlingStack) - assert.NotEqual(stack, handlingStack) span.Finish() }) @@ -817,14 +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") - handlingStack := span.meta[ext.ErrorHandlingStack] - assert.NotEqual("", handlingStack) - assert.Equal(stack, handlingStack) span.Finish() }) } @@ -843,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 @@ -874,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) { From 27b1c533186137e5e70d2c8bfd8a8a7f1d34fec1 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 23 Sep 2025 10:08:28 -0400 Subject: [PATCH 6/7] deprecate error.details --- ddtrace/ext/tags.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ddtrace/ext/tags.go b/ddtrace/ext/tags.go index a2251011cb..c9783de624 100644 --- a/ddtrace/ext/tags.go +++ b/ddtrace/ext/tags.go @@ -94,6 +94,7 @@ const ( ErrorHandlingStack = "error.handling_stack" // 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. From e80c232669d0e4ff50a810d52470b616e5fc984f Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 23 Sep 2025 10:33:49 -0400 Subject: [PATCH 7/7] code clean up --- ddtrace/tracer/span.go | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index 9378d4aadb..b2495695ec 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -435,28 +435,23 @@ func (s *Span) setTagError(value interface{}, cfg errorConfig) { 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: - if !cfg.noDebugStack { - s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) - } + s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) case fmt.Formatter: // pkg/errors approach - if !cfg.noDebugStack { - s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) - } + s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) case *errortrace.TracerError: // instrumentation/errortrace approach - if !cfg.noDebugStack { - s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) - s.setMeta(ext.ErrorHandlingStack, err.Format()) - } + s.setMeta(ext.ErrorStack, fmt.Sprintf("%+v", v)) + s.setMeta(ext.ErrorHandlingStack, err.Format()) return } - if !cfg.noDebugStack { - stack := takeStacktrace(cfg.stackFrames, cfg.stackSkip) - s.setMeta(ext.ErrorHandlingStack, stack) - } + stack := takeStacktrace(cfg.stackFrames, cfg.stackSkip) + s.setMeta(ext.ErrorHandlingStack, stack) case nil: // no error setError(false)