Skip to content

Commit

Permalink
Add WithStatus option for use with RecordError
Browse files Browse the repository at this point in the history
Fixes #1677 by adding a functional option that sets the Status of a
span when calling RecordError.
  • Loading branch information
treuherz committed May 5, 2022
1 parent c7cf945 commit 201ed00
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 36 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 13 additions & 3 deletions bridge/opentracing/internal/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
5 changes: 1 addition & 4 deletions internal/global/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,17 +171,14 @@ 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) {}

// End does nothing.
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) {}
Expand Down
25 changes: 16 additions & 9 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -757,17 +767,14 @@ 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) {}

// End does nothing.
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) {}
Expand Down
26 changes: 22 additions & 4 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
68 changes: 57 additions & 11 deletions trace/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}

Expand Down
5 changes: 1 addition & 4 deletions trace/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,14 @@ 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) {}

// End does nothing.
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) {}
Expand Down
2 changes: 1 addition & 1 deletion trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 201ed00

Please sign in to comment.