tracer: Check for nil interface types in span.Finish(WithError())

ddtrace/tracer/option.go

@@ -15,6 +15,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"runtime"
 	"runtime/debug"
@@ -1206,8 +1207,19 @@ func FinishTime(t time.Time) FinishOption {
 
 // WithError marks the span as having had an error. It uses the information from
 // err to set tags such as the error message, error type and stack trace. It has
-// no effect if the error is nil.
+// no effect if the error is nil or the underlying value is a nil pointer (https://go.dev/doc/faq#nil_error).
 func WithError(err error) FinishOption {
+	if err == nil {
+		return func(_ *ddtrace.FinishConfig) {}
+	}
+	v := reflect.ValueOf(err)
+	// It's easy to accidentally make a non-nil error with underlying nil: https://go.dev/doc/faq#nil_error
+	// This code checks to make sure we don't accidentally save an error with a nil pointer value.
+	// See also: https://github.com/DataDog/dd-trace-go/issues/2029
+	if !v.IsValid() || (v.Kind() == reflect.Ptr && v.IsNil()) {
+		log.Debug("Underlying error value is nil pointer, ignoring. See https://go.dev/doc/faq#nil_error for details.")
+		return func(_ *ddtrace.FinishConfig) {}
+	}
 	return func(cfg *ddtrace.FinishConfig) {
 		cfg.Error = err
 	}

ddtrace/tracer/span_test.go

@@ -232,6 +232,28 @@ func TestSpanFinishWithError(t *testing.T) {
 	assert.NotEmpty(span.Meta[ext.ErrorStack])
 }
 
+type MyError struct {
+	msg string
+}
+
+func (e *MyError) Error() string {
+	return e.msg
+}
+
+func TestSpanFinishWithErrorNilCustomError(t *testing.T) {
+	assert := assert.New(t)
+
+	var err error
+	var myError *MyError
+	myError = nil
+	err = myError
+
+	span := newBasicSpan("web.request")
+	span.Finish(WithError(err))
+
+	assert.Equal(int32(0), span.Error)
+}
+
 func TestSpanFinishWithErrorNoDebugStack(t *testing.T) {
 	assert := assert.New(t)
 

ddtrace/tracer/tracer_test.go

@@ -1999,9 +1999,10 @@ func BenchmarkTracerAddSpans(b *testing.B) {
 	tracer, _, _, stop := startTestTracer(b, WithLogger(log.DiscardLogger{}), WithSampler(NewRateSampler(0)))
 	defer stop()
 
+	e := errors.New("test error")
 	for n := 0; n < b.N; n++ {
 		span := tracer.StartSpan("pylons.request", ServiceName("pylons"), ResourceName("/"))
-		span.Finish()
+		span.Finish(WithError(e))
 	}
 }