diff --git a/CHANGELOG.md b/CHANGELOG.md index b64a142286df..dddcd5af71b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add a `WithStatus` option to `go.opentelemetry.io/otel/trace` to allow setting the span status when calling `span.RecordError`. (#1677) + ## [1.7.0/0.30.0] - 2022-04-28 ### Added diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index 159e94d4048b..8fa1067ce679 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -256,7 +256,7 @@ func (s *MockSpan) End(options ...trace.SpanEndOption) { s.mockTracer.FinishedSpans = append(s.mockTracer.FinishedSpans, s) } -func (s *MockSpan) RecordError(err error, opts ...trace.EventOption) { +func (s *MockSpan) RecordError(err error, opts ...trace.ErrorOption) { if err == nil { return // no-op on nil error } @@ -265,12 +265,22 @@ func (s *MockSpan) RecordError(err error, opts ...trace.EventOption) { return // already finished } - s.SetStatus(codes.Error, "") opts = append(opts, trace.WithAttributes( semconv.ExceptionTypeKey.String(reflect.TypeOf(err).String()), semconv.ExceptionMessageKey.String(err.Error()), )) - s.AddEvent(semconv.ExceptionEventName, opts...) + + c := trace.NewErrorConfig(opts...) + if c.SetErrorStatus() { + s.SetStatus(codes.Error, err.Error()) + } + + var eventOpts []trace.EventOption + for _, opt := range opts { + eventOpts = append(eventOpts, opt) + } + + s.AddEvent(semconv.ExceptionEventName, eventOpts...) } func (s *MockSpan) Tracer() trace.Tracer { diff --git a/internal/global/trace.go b/internal/global/trace.go index 5f008d0982be..257bb8592e6b 100644 --- a/internal/global/trace.go +++ b/internal/global/trace.go @@ -171,9 +171,6 @@ func (nonRecordingSpan) IsRecording() bool { return false } // SetStatus does nothing. func (nonRecordingSpan) SetStatus(codes.Code, string) {} -// SetError does nothing. -func (nonRecordingSpan) SetError(bool) {} - // SetAttributes does nothing. func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {} @@ -181,7 +178,7 @@ func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {} func (nonRecordingSpan) End(...trace.SpanEndOption) {} // RecordError does nothing. -func (nonRecordingSpan) RecordError(error, ...trace.EventOption) {} +func (nonRecordingSpan) RecordError(error, ...trace.ErrorOption) {} // AddEvent does nothing. func (nonRecordingSpan) AddEvent(string, ...trace.EventOption) {} diff --git a/sdk/trace/span.go b/sdk/trace/span.go index edf456d93a7a..fc26bb2b9867 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -401,7 +401,7 @@ func (s *recordingSpan) End(options ...trace.SpanEndOption) { // SetStatus is required if the Status of the Span should be set to Error, this method // does not change the Span status. If this span is not being recorded or err is nil // than this method does nothing. -func (s *recordingSpan) RecordError(err error, opts ...trace.EventOption) { +func (s *recordingSpan) RecordError(err error, opts ...trace.ErrorOption) { if s == nil || err == nil || !s.IsRecording() { return } @@ -411,14 +411,24 @@ func (s *recordingSpan) RecordError(err error, opts ...trace.EventOption) { semconv.ExceptionMessageKey.String(err.Error()), )) - c := trace.NewEventConfig(opts...) - if c.StackTrace() { - opts = append(opts, trace.WithAttributes( + c := trace.NewErrorConfig(opts...) + if c.SetErrorStatus() { + s.SetStatus(codes.Error, err.Error()) + } + + var eventOpts []trace.EventOption + for _, opt := range opts { + eventOpts = append(eventOpts, opt) + } + + eventConfig := trace.NewEventConfig(eventOpts...) + if eventConfig.StackTrace() { + eventOpts = append(eventOpts, trace.WithAttributes( semconv.ExceptionStacktraceKey.String(recordStackTrace()), )) } - s.addEvent(semconv.ExceptionEventName, opts...) + s.addEvent(semconv.ExceptionEventName, eventOpts...) } func typeStr(i interface{}) string { @@ -757,9 +767,6 @@ func (nonRecordingSpan) IsRecording() bool { return false } // SetStatus does nothing. func (nonRecordingSpan) SetStatus(codes.Code, string) {} -// SetError does nothing. -func (nonRecordingSpan) SetError(bool) {} - // SetAttributes does nothing. func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {} @@ -767,7 +774,7 @@ func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {} func (nonRecordingSpan) End(...trace.SpanEndOption) {} // RecordError does nothing. -func (nonRecordingSpan) RecordError(error, ...trace.EventOption) {} +func (nonRecordingSpan) RecordError(error, ...trace.ErrorOption) {} // AddEvent does nothing. func (nonRecordingSpan) AddEvent(string, ...trace.EventOption) {} diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index f359eef99d6f..b1e38bd564d4 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1182,9 +1182,10 @@ func TestCustomStartEndTime(t *testing.T) { func TestRecordError(t *testing.T) { scenarios := []struct { - err error - typ string - msg string + err error + typ string + msg string + withStatus bool }{ { err: ottest.NewTestError("test error"), @@ -1196,6 +1197,12 @@ func TestRecordError(t *testing.T) { typ: "*errors.errorString", msg: "test error 2", }, + { + err: errors.New("test error 3"), + typ: "*errors.errorString", + msg: "test error 3", + withStatus: true, + }, } for _, s := range scenarios { @@ -1204,7 +1211,12 @@ func TestRecordError(t *testing.T) { span := startSpan(tp, "RecordError") errTime := time.Now() - span.RecordError(s.err, trace.WithTimestamp(errTime)) + opts := []trace.ErrorOption{trace.WithTimestamp(errTime)} + if s.withStatus { + opts = append(opts, trace.WithStatus()) + } + + span.RecordError(s.err, opts...) got, err := endSpan(te, span) if err != nil { @@ -1232,6 +1244,12 @@ func TestRecordError(t *testing.T) { }, instrumentationLibrary: instrumentation.Library{Name: "RecordError"}, } + + if s.withStatus { + want.status.Code = codes.Error + want.status.Description = s.msg + } + if diff := cmpDiff(got, want); diff != "" { t.Errorf("SpanErrorOptions: -got +want %s", diff) } diff --git a/trace/config.go b/trace/config.go index f058cc781e00..999544546ad4 100644 --- a/trace/config.go +++ b/trace/config.go @@ -189,16 +189,42 @@ type SpanOption interface { SpanEndOption } -// SpanStartEventOption are options that can be used at the start of a span, or with an event. -type SpanStartEventOption interface { +// SpanStartEventErrorOption are options that can be used at the start of a span, or with an event, or when recording +// an error. +type SpanStartEventErrorOption interface { SpanStartOption EventOption + ErrorOption } -// SpanEndEventOption are options that can be used at the end of a span, or with an event. -type SpanEndEventOption interface { +// SpanEndEventErrorOption are options that can be used at the end of a span, or with an event, or when recording an +// error. +type SpanEndEventErrorOption interface { SpanEndOption EventOption + ErrorOption +} + +type ErrorConfig struct { + setErrorStatus bool +} + +func (cfg *ErrorConfig) SetErrorStatus() bool { + return cfg.setErrorStatus +} + +func NewErrorConfig(options ...ErrorOption) ErrorConfig { + var c ErrorConfig + for _, option := range options { + c = option.applyError(c) + } + return c +} + +// ErrorOption are options that can be used when recording an error to a span. +type ErrorOption interface { + applyError(ErrorConfig) ErrorConfig + EventOption } type attributeOption []attribute.KeyValue @@ -208,12 +234,13 @@ func (o attributeOption) applySpan(c SpanConfig) SpanConfig { return c } func (o attributeOption) applySpanStart(c SpanConfig) SpanConfig { return o.applySpan(c) } +func (o attributeOption) applyError(c ErrorConfig) ErrorConfig { return c } func (o attributeOption) applyEvent(c EventConfig) EventConfig { c.attributes = append(c.attributes, []attribute.KeyValue(o)...) return c } -var _ SpanStartEventOption = attributeOption{} +var _ SpanStartEventErrorOption = attributeOption{} // WithAttributes adds the attributes related to a span life-cycle event. // These attributes are used to describe the work a Span represents when this @@ -224,14 +251,15 @@ var _ SpanStartEventOption = attributeOption{} // If multiple of these options are passed the attributes of each successive // option will extend the attributes instead of overwriting. There is no // guarantee of uniqueness in the resulting attributes. -func WithAttributes(attributes ...attribute.KeyValue) SpanStartEventOption { +func WithAttributes(attributes ...attribute.KeyValue) SpanStartEventErrorOption { return attributeOption(attributes) } -// SpanEventOption are options that can be used with an event or a span. -type SpanEventOption interface { +// SpanEventErrorOption are options that can be used with an event or a span, or when recording an error. +type SpanEventErrorOption interface { SpanOption EventOption + ErrorOption } type timestampOption time.Time @@ -242,21 +270,25 @@ func (o timestampOption) applySpan(c SpanConfig) SpanConfig { } func (o timestampOption) applySpanStart(c SpanConfig) SpanConfig { return o.applySpan(c) } func (o timestampOption) applySpanEnd(c SpanConfig) SpanConfig { return o.applySpan(c) } +func (o timestampOption) applyError(c ErrorConfig) ErrorConfig { + return c +} func (o timestampOption) applyEvent(c EventConfig) EventConfig { c.timestamp = time.Time(o) return c } -var _ SpanEventOption = timestampOption{} +var _ SpanEventErrorOption = timestampOption{} // WithTimestamp sets the time of a Span or Event life-cycle moment (e.g. // started, stopped, errored). -func WithTimestamp(t time.Time) SpanEventOption { +func WithTimestamp(t time.Time) SpanEventErrorOption { return timestampOption(t) } type stackTraceOption bool +func (o stackTraceOption) applyError(c ErrorConfig) ErrorConfig { return c } func (o stackTraceOption) applyEvent(c EventConfig) EventConfig { c.stackTrace = bool(o) return c @@ -267,8 +299,22 @@ func (o stackTraceOption) applySpan(c SpanConfig) SpanConfig { } func (o stackTraceOption) applySpanEnd(c SpanConfig) SpanConfig { return o.applySpan(c) } +// WithStatus is used when recording an error, and sets the span status to "Error" and +// adds the error's Error string as the description. +func WithStatus() ErrorOption { + return spanErrorStatus{} +} + +type spanErrorStatus struct{} + +func (o spanErrorStatus) applyError(c ErrorConfig) ErrorConfig { + c.setErrorStatus = true + return c +} +func (o spanErrorStatus) applyEvent(c EventConfig) EventConfig { return c } + // WithStackTrace sets the flag to capture the error with stack trace (e.g. true, false). -func WithStackTrace(b bool) SpanEndEventOption { +func WithStackTrace(b bool) SpanEndEventErrorOption { return stackTraceOption(b) } diff --git a/trace/noop.go b/trace/noop.go index 73950f207789..45ea96b56f5e 100644 --- a/trace/noop.go +++ b/trace/noop.go @@ -67,9 +67,6 @@ func (noopSpan) IsRecording() bool { return false } // SetStatus does nothing. func (noopSpan) SetStatus(codes.Code, string) {} -// SetError does nothing. -func (noopSpan) SetError(bool) {} - // SetAttributes does nothing. func (noopSpan) SetAttributes(...attribute.KeyValue) {} @@ -77,7 +74,7 @@ func (noopSpan) SetAttributes(...attribute.KeyValue) {} func (noopSpan) End(...SpanEndOption) {} // RecordError does nothing. -func (noopSpan) RecordError(error, ...EventOption) {} +func (noopSpan) RecordError(error, ...ErrorOption) {} // AddEvent does nothing. func (noopSpan) AddEvent(string, ...EventOption) {} diff --git a/trace/trace.go b/trace/trace.go index 1bc040c27640..054c538745ac 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -357,7 +357,7 @@ type Span interface { // additional call to SetStatus is required if the Status of the Span should // be set to Error, as this method does not change the Span status. If this // span is not being recorded or err is nil then this method does nothing. - RecordError(err error, options ...EventOption) + RecordError(err error, options ...ErrorOption) // SpanContext returns the SpanContext of the Span. The returned SpanContext // is usable even after the End method has been called for the Span.