Add Span#RecordError method to simplify adding error events to spans (#473)

* Add `Span#Error` method to simplify setting an error status and message.

* `Span#Error` should no-op on nil errors

* Record errors as a span event rather than status/attributes.

The implementation in the SDK package now relies on existing API methods.

* Add WithErrorStatus() ErrorOption to allow setting span status on error.

* Apply suggestions from code review

Co-Authored-By: Krzesimir Nowak <qdlacz@gmail.com>

* Address code review feedback

* Clean up RecordError tests

* Ensure complete and unique error type is recorded for defined types
* Avoid duplicating logic under test in tests

* Move TestError to internal/testing package, improve RecordError test scenarios

Co-authored-by: Krzesimir Nowak <qdlacz@gmail.com>

sdk/trace/span.go

@@ -16,6 +16,8 @@ package trace
 
 import (
 	"context"
+	"fmt"
+	"reflect"
 	"sync"
 	"time"
 
@@ -27,6 +29,12 @@ import (
 	"go.opentelemetry.io/otel/sdk/internal"
 )
 
+const (
+	errorTypeKey    = core.Key("error.type")
+	errorMessageKey = core.Key("error.message")
+	errorEventName  = "error"
+)
+
 // span implements apitrace.Span interface.
 type span struct {
 	// data contains information recorded about the span.
@@ -123,6 +131,42 @@ func (s *span) End(options ...apitrace.EndOption) {
 	})
 }
 
+func (s *span) RecordError(ctx context.Context, err error, opts ...apitrace.ErrorOption) {
+	if s == nil || err == nil {
+		return
+	}
+
+	if !s.IsRecording() {
+		return
+	}
+
+	cfg := apitrace.ErrorConfig{}
+
+	for _, o := range opts {
+		o(&cfg)
+	}
+
+	if cfg.Timestamp.IsZero() {
+		cfg.Timestamp = time.Now()
+	}
+
+	if cfg.Status != codes.OK {
+		s.SetStatus(cfg.Status)
+	}
+
+	errType := reflect.TypeOf(err)
+	errTypeString := fmt.Sprintf("%s.%s", errType.PkgPath(), errType.Name())
+	if errTypeString == "." {
+		// PkgPath() and Name() may be empty for builtin Types
+		errTypeString = errType.String()
+	}
+
+	s.AddEventWithTimestamp(ctx, cfg.Timestamp, errorEventName,
+		errorTypeKey.String(errTypeString),
+		errorMessageKey.String(err.Error()),
+	)
+}
+
 func (s *span) Tracer() apitrace.Tracer {
 	return s.tracer
 }

sdk/trace/trace_test.go

@@ -16,6 +16,7 @@ package trace
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math"
 	"strings"
@@ -31,6 +32,7 @@ import (
 	"go.opentelemetry.io/otel/api/testharness"
 	"go.opentelemetry.io/otel/api/trace"
 	apitrace "go.opentelemetry.io/otel/api/trace"
+	ottest "go.opentelemetry.io/otel/internal/testing"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
 )
 
@@ -870,6 +872,137 @@ func TestCustomStartEndTime(t *testing.T) {
 	}
 }
 
+func TestRecordError(t *testing.T) {
+	scenarios := []struct {
+		err error
+		typ string
+		msg string
+	}{
+		{
+			err: ottest.NewTestError("test error"),
+			typ: "go.opentelemetry.io/otel/internal/testing.TestError",
+			msg: "test error",
+		},
+		{
+			err: errors.New("test error 2"),
+			typ: "*errors.errorString",
+			msg: "test error 2",
+		},
+	}
+
+	for _, s := range scenarios {
+		te := &testExporter{}
+		tp, _ := NewProvider(WithSyncer(te))
+		span := startSpan(tp, "RecordError")
+
+		errTime := time.Now()
+		span.RecordError(context.Background(), s.err,
+			apitrace.WithErrorTime(errTime),
+		)
+
+		got, err := endSpan(te, span)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		want := &export.SpanData{
+			SpanContext: core.SpanContext{
+				TraceID:    tid,
+				TraceFlags: 0x1,
+			},
+			ParentSpanID:    sid,
+			Name:            "span0",
+			SpanKind:        apitrace.SpanKindInternal,
+			HasRemoteParent: true,
+			MessageEvents: []export.Event{
+				{
+					Name: errorEventName,
+					Time: errTime,
+					Attributes: []core.KeyValue{
+						errorTypeKey.String(s.typ),
+						errorMessageKey.String(s.msg),
+					},
+				},
+			},
+		}
+		if diff := cmpDiff(got, want); diff != "" {
+			t.Errorf("SpanErrorOptions: -got +want %s", diff)
+		}
+	}
+}
+
+func TestRecordErrorWithStatus(t *testing.T) {
+	te := &testExporter{}
+	tp, _ := NewProvider(WithSyncer(te))
+	span := startSpan(tp, "RecordErrorWithStatus")
+
+	testErr := ottest.NewTestError("test error")
+	errTime := time.Now()
+	testStatus := codes.Unknown
+	span.RecordError(context.Background(), testErr,
+		apitrace.WithErrorTime(errTime),
+		apitrace.WithErrorStatus(testStatus),
+	)
+
+	got, err := endSpan(te, span)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := &export.SpanData{
+		SpanContext: core.SpanContext{
+			TraceID:    tid,
+			TraceFlags: 0x1,
+		},
+		ParentSpanID:    sid,
+		Name:            "span0",
+		SpanKind:        apitrace.SpanKindInternal,
+		Status:          codes.Unknown,
+		HasRemoteParent: true,
+		MessageEvents: []export.Event{
+			{
+				Name: errorEventName,
+				Time: errTime,
+				Attributes: []core.KeyValue{
+					errorTypeKey.String("go.opentelemetry.io/otel/internal/testing.TestError"),
+					errorMessageKey.String("test error"),
+				},
+			},
+		},
+	}
+	if diff := cmpDiff(got, want); diff != "" {
+		t.Errorf("SpanErrorOptions: -got +want %s", diff)
+	}
+}
+
+func TestRecordErrorNil(t *testing.T) {
+	te := &testExporter{}
+	tp, _ := NewProvider(WithSyncer(te))
+	span := startSpan(tp, "RecordErrorNil")
+
+	span.RecordError(context.Background(), nil)
+
+	got, err := endSpan(te, span)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := &export.SpanData{
+		SpanContext: core.SpanContext{
+			TraceID:    tid,
+			TraceFlags: 0x1,
+		},
+		ParentSpanID:    sid,
+		Name:            "span0",
+		SpanKind:        apitrace.SpanKindInternal,
+		HasRemoteParent: true,
+		Status:          codes.OK,
+	}
+	if diff := cmpDiff(got, want); diff != "" {
+		t.Errorf("SpanErrorOptions: -got +want %s", diff)
+	}
+}
+
 func TestWithSpanKind(t *testing.T) {
 	var te testExporter
 	tp, _ := NewProvider(WithSyncer(&te), WithConfig(Config{DefaultSampler: AlwaysSample()}))