Skip to content

Commit 364ad1e

Browse files
lastchiliarchlastchiliarch
committed
Changed signature of the Span RecordError, replace EventOption by ErrorOption (open-telemetry#1677)
* Change signature of the Span `RecordError, replace EventOption by ErrorOption * Add WithEventOpts, WithErrorStatus * Set status when WithErrorStatus is passed to RecordError Signed-off-by: lastchiliarch <lastchiliarch@163.com>
2 parents c1573c2 + d3b58b6 commit 364ad1e

File tree

10 files changed

+191
-54
lines changed

10 files changed

+191
-54
lines changed

‎CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
141141

142142
### Added
143143

144-
- Added `WithStatus` event option to set Span status at the same time when call RecordError. (#1677)
145144
- Added `resource.Default()` for use with meter and tracer providers. (#1507)
146145
- `AttributePerEventCountLimit` and `AttributePerLinkCountLimit` for `SpanLimits`. (#1535)
147146
- Added `Keys()` method to `propagation.TextMapCarrier` and `propagation.HeaderCarrier` to adapt `http.Header` to this interface. (#1544)
@@ -162,6 +161,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
162161

163162
### Changed
164163

164+
- Changed signature of the Span `RecordError` , replace EventOption by ErrorOption
165165
- Replaced interface `oteltest.SpanRecorder` with its existing implementation
166166
`StandardSpanRecorder`. (#1542)
167167
- Default span limit values to 128. (#1535)

‎bridge/opentracing/internal/mock.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ func (s *MockSpan) End(options ...trace.SpanOption) {
245245
s.mockTracer.FinishedSpans = append(s.mockTracer.FinishedSpans, s)
246246
}
247247

248-
func (s *MockSpan) RecordError(err error, opts ...trace.EventOption) {
248+
func (s *MockSpan) RecordError(err error, options ...trace.ErrorOption) {
249249
if err == nil {
250250
return // no-op on nil error
251251
}
@@ -255,11 +255,13 @@ func (s *MockSpan) RecordError(err error, opts ...trace.EventOption) {
255255
}
256256

257257
s.SetStatus(codes.Error, "")
258-
opts = append(opts, trace.WithAttributes(
258+
c := trace.NewErrorConfig(options...)
259+
eventOpts := c.EventOpts
260+
eventOpts = append(eventOpts, trace.WithAttributes(
259261
semconv.ExceptionTypeKey.String(reflect.TypeOf(err).String()),
260262
semconv.ExceptionMessageKey.String(err.Error()),
261263
))
262-
s.AddEvent(semconv.ExceptionEventName, opts...)
264+
s.AddEvent(semconv.ExceptionEventName, eventOpts...)
263265
}
264266

265267
func (s *MockSpan) Tracer() trace.Tracer {

‎oteltest/span.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (s *Span) End(opts ...trace.SpanOption) {
7575
}
7676

7777
// RecordError records an error as an exception Span event.
78-
func (s *Span) RecordError(err error, opts ...trace.EventOption) {
78+
func (s *Span) RecordError(err error, options ...trace.ErrorOption) {
7979
if err == nil || s.ended {
8080
return
8181
}
@@ -86,12 +86,14 @@ func (s *Span) RecordError(err error, opts ...trace.EventOption) {
8686
errTypeString = errType.String()
8787
}
8888

89-
opts = append(opts, trace.WithAttributes(
89+
c := trace.NewErrorConfig(options...)
90+
eventOpts := c.EventOpts
91+
eventOpts = append(eventOpts, trace.WithAttributes(
9092
semconv.ExceptionTypeKey.String(errTypeString),
9193
semconv.ExceptionMessageKey.String(err.Error()),
9294
))
9395

94-
s.AddEvent(semconv.ExceptionEventName, opts...)
96+
s.AddEvent(semconv.ExceptionEventName, eventOpts...)
9597
}
9698

9799
// AddEvent adds an event to s.

‎oteltest/span_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func TestSpan(t *testing.T) {
157157
e.Expect(ok).ToBeTrue()
158158

159159
testTime := time.Now()
160-
subject.RecordError(s.err, trace.WithTimestamp(testTime))
160+
subject.RecordError(s.err, trace.WithEventOpts(trace.WithTimestamp(testTime)))
161161

162162
expectedEvents := []oteltest.Event{{
163163
Timestamp: testTime,

‎sdk/trace/span.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -248,20 +248,22 @@ func (s *span) End(options ...trace.SpanOption) {
248248
// This method does not change the Span status in default
249249
// To change Span status, pass `WithStatus` as options.
250250
// This method does nothing If this span is not being recorded or err is nil.
251-
func (s *span) RecordError(err error, opts ...trace.EventOption) {
251+
func (s *span) RecordError(err error, options ...trace.ErrorOption) {
252252
if s == nil || err == nil || !s.IsRecording() {
253253
return
254254
}
255-
c := trace.NewEventConfig(opts...)
256-
if c.WithStatus {
257-
s.SetStatus(codes.Error, "")
255+
256+
c := trace.NewErrorConfig(options...)
257+
if c.Code != codes.Unset {
258+
s.SetStatus(c.Code, c.Message)
258259
}
259260

260-
opts = append(opts, trace.WithAttributes(
261+
eventOpts := c.EventOpts
262+
eventOpts = append(eventOpts, trace.WithAttributes(
261263
semconv.ExceptionTypeKey.String(typeStr(err)),
262264
semconv.ExceptionMessageKey.String(err.Error()),
263265
))
264-
s.addEvent(semconv.ExceptionEventName, opts...)
266+
s.addEvent(semconv.ExceptionEventName, eventOpts...)
265267
}
266268

267269
func typeStr(i interface{}) string {

‎sdk/trace/trace_test.go

+52-10
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,7 @@ func TestRecordError(t *testing.T) {
11011101
span := startSpan(tp, "RecordError")
11021102

11031103
errTime := time.Now()
1104-
span.RecordError(s.err, trace.WithTimestamp(errTime))
1104+
span.RecordError(s.err, trace.WithEventOpts(trace.WithTimestamp(errTime)))
11051105

11061106
got, err := endSpan(te, span)
11071107
if err != nil {
@@ -1164,14 +1164,34 @@ func TestRecordErrorNil(t *testing.T) {
11641164
}
11651165
}
11661166

1167-
func TestRecordErrorWithStatus(t *testing.T) {
1167+
func TestRecordErrorOptions(t *testing.T) {
11681168
errTime := time.Now()
11691169
scenarios := []struct {
1170-
err error
1171-
want *SpanSnapshot
1170+
err error
1171+
options []trace.ErrorOption
1172+
want *SpanSnapshot
11721173
}{
1174+
{
1175+
err: nil,
1176+
options: []trace.ErrorOption{},
1177+
want: &SpanSnapshot{
1178+
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
1179+
TraceID: tid,
1180+
TraceFlags: 0x1,
1181+
}),
1182+
Parent: sc.WithRemote(true),
1183+
Name: "span0",
1184+
StatusCode: codes.Unset,
1185+
SpanKind: trace.SpanKindInternal,
1186+
InstrumentationLibrary: instrumentation.Library{Name: "RecordError"},
1187+
},
1188+
},
11731189
{
11741190
err: ottest.NewTestError("test error"),
1191+
options: []trace.ErrorOption{
1192+
trace.WithErrorStatus(codes.Error, "test error"),
1193+
trace.WithEventOpts(trace.WithTimestamp(errTime)),
1194+
},
11751195
want: &SpanSnapshot{
11761196
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
11771197
TraceID: tid,
@@ -1191,20 +1211,42 @@ func TestRecordErrorWithStatus(t *testing.T) {
11911211
},
11921212
},
11931213
},
1214+
StatusMessage: "test error",
11941215
InstrumentationLibrary: instrumentation.Library{Name: "RecordError"},
11951216
},
11961217
},
11971218
{
1198-
err: nil,
1219+
err: ottest.NewTestError("test error"),
1220+
options: []trace.ErrorOption{
1221+
trace.WithErrorStatus(codes.Error, "test error"),
1222+
trace.WithEventOpts(
1223+
trace.WithTimestamp(errTime),
1224+
trace.WithAttributes(attribute.String("key1", "value1")),
1225+
trace.WithAttributes(attribute.String("key2", "value2")),
1226+
),
1227+
},
11991228
want: &SpanSnapshot{
12001229
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
12011230
TraceID: tid,
12021231
TraceFlags: 0x1,
12031232
}),
1204-
Parent: sc.WithRemote(true),
1205-
Name: "span0",
1206-
StatusCode: codes.Unset,
1207-
SpanKind: trace.SpanKindInternal,
1233+
Parent: sc.WithRemote(true),
1234+
Name: "span0",
1235+
StatusCode: codes.Error,
1236+
SpanKind: trace.SpanKindInternal,
1237+
MessageEvents: []trace.Event{
1238+
{
1239+
Name: semconv.ExceptionEventName,
1240+
Time: errTime,
1241+
Attributes: []attribute.KeyValue{
1242+
attribute.String("key1", "value1"),
1243+
attribute.String("key2", "value2"),
1244+
semconv.ExceptionTypeKey.String("go.opentelemetry.io/otel/internal/internaltest.TestError"),
1245+
semconv.ExceptionMessageKey.String("test error"),
1246+
},
1247+
},
1248+
},
1249+
StatusMessage: "test error",
12081250
InstrumentationLibrary: instrumentation.Library{Name: "RecordError"},
12091251
},
12101252
},
@@ -1215,7 +1257,7 @@ func TestRecordErrorWithStatus(t *testing.T) {
12151257
tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty()))
12161258

12171259
span := startSpan(tp, "RecordError")
1218-
span.RecordError(s.err, trace.WithTimestamp(errTime), trace.WithStatus(true))
1260+
span.RecordError(s.err, s.options...)
12191261
got, err := endSpan(te, span)
12201262
if err != nil {
12211263
t.Fatal(err)

‎trace/config.go

+51-10
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ package trace
1717
import (
1818
"time"
1919

20+
"go.opentelemetry.io/otel/codes"
21+
2022
"go.opentelemetry.io/otel/attribute"
2123
)
2224

@@ -60,8 +62,6 @@ type SpanConfig struct {
6062
NewRoot bool
6163
// SpanKind is the role a Span has in a trace.
6264
SpanKind SpanKind
63-
// WithStatus is used to control whether or not set Span status when RecordError
64-
WithStatus bool
6565
}
6666

6767
// NewSpanConfig applies all the options to a returned SpanConfig.
@@ -206,14 +206,55 @@ func (i instrumentationVersionOption) ApplyTracer(config *TracerConfig) {
206206

207207
func (instrumentationVersionOption) private() {}
208208

209-
type withStatusSpanOption bool
209+
// ErrorConfig is a group of options for error.
210+
type ErrorConfig struct {
211+
Code codes.Code
212+
Message string
213+
// EventOpts will be passed to addEvent
214+
EventOpts []EventOption
215+
}
216+
217+
// ErrorOption applies an option to a ErrorConfig.
218+
type ErrorOption interface {
219+
ApplyError(config *ErrorConfig)
220+
221+
// A private method to prevent users implementing the
222+
// interface and so future additions to it will not
223+
// violate compatibility.
224+
private()
225+
}
226+
227+
// NewErrorConfig applies all the options to a returned ErrorConfig.
228+
func NewErrorConfig(options ...ErrorOption) *ErrorConfig {
229+
c := new(ErrorConfig)
230+
for _, option := range options {
231+
option.ApplyError(c)
232+
}
233+
return c
234+
}
235+
236+
type statusErrorOption struct {
237+
Code codes.Code
238+
Message string
239+
}
240+
241+
func (o statusErrorOption) ApplyError(c *ErrorConfig) {
242+
c.Code = o.Code
243+
c.Message = o.Message
244+
}
245+
func (statusErrorOption) private() {}
246+
247+
// WithErrorStatus set the error code and message when code is not codes.Unset
248+
func WithErrorStatus(code codes.Code, message string) ErrorOption {
249+
return statusErrorOption{code, message}
250+
}
251+
252+
type eventOptsErrorOption []EventOption
210253

211-
func (o withStatusSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) }
212-
func (o withStatusSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) }
213-
func (withStatusSpanOption) private() {}
214-
func (o withStatusSpanOption) apply(c *SpanConfig) { c.WithStatus = bool(o) }
254+
func (o eventOptsErrorOption) ApplyError(c *ErrorConfig) { c.EventOpts = o }
255+
func (eventOptsErrorOption) private() {}
215256

216-
// WithStatus set the status at the same time when RecordError
217-
func WithStatus(o bool) LifeCycleOption {
218-
return withStatusSpanOption(o)
257+
// WithEventOpts set event options, will be passed to addEvent
258+
func WithEventOpts(option ...EventOption) ErrorOption {
259+
return eventOptsErrorOption(option)
219260
}

0 commit comments

Comments
 (0)