diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b1f76163..fe5638917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added +- `EventOption` and the related `NewEventConfig` function are added to the `go.opentelemetry.io/otel` package to configure Span events. (#1254) - A `TextMapPropagator` and associated `TextMapCarrier` are added to the `go.opentelemetry.io/otel/oteltest` package to test TextMap type propagators and their use. (#1259) ### Changed @@ -27,10 +28,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm This matches the returned type and fixes misuse of the term metric. (#1240) - Move test harness from the `go.opentelemetry.io/otel/api/apitest` package into `go.opentelemetry.io/otel/oteltest`. (#1241) - Rename `MergeItererator` to `MergeIterator` in the `go.opentelemetry.io/otel/label` package. (#1244) +- The function signature of the Span `AddEvent` method in `go.opentelemetry.io/otel` is updated to no longer take an unused context and instead take a required name and a variable number of `EventOption`s. (#1254) +- The function signature of the Span `RecordError` method in `go.opentelemetry.io/otel` is updated to no longer take an unused context and instead take a required error value and a variable number of `EventOption`s. (#1254) ### Removed - The `ErrInvalidHexID`, `ErrInvalidTraceIDLength`, `ErrInvalidSpanIDLength`, `ErrInvalidSpanIDLength`, or `ErrNilSpanID` from the `go.opentelemetry.io/otel` package are unexported now. (#1243) +- The `AddEventWithTimestamp` method on the `Span` interface in `go.opentelemetry.io/otel` is removed due to its redundancy. + It is replaced by using the `AddEvent` method with a `WithTimestamp` option. (#1254) ### Fixed diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index d11826d95..b8190f412 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -114,7 +114,11 @@ func (s *bridgeSpan) FinishWithOptions(opts ot.FinishOptions) { } func (s *bridgeSpan) logRecord(record ot.LogRecord) { - s.otelSpan.AddEventWithTimestamp(context.Background(), record.Timestamp, "", otLogFieldsToOTelLabels(record.Fields)...) + s.otelSpan.AddEvent( + "", + otel.WithTimestamp(record.Timestamp), + otel.WithAttributes(otLogFieldsToOTelLabels(record.Fields)...), + ) } func (s *bridgeSpan) Context() ot.SpanContext { @@ -141,7 +145,10 @@ func (s *bridgeSpan) SetTag(key string, value interface{}) ot.Span { } func (s *bridgeSpan) LogFields(fields ...otlog.Field) { - s.otelSpan.AddEvent(context.Background(), "", otLogFieldsToOTelLabels(fields)...) + s.otelSpan.AddEvent( + "", + otel.WithAttributes(otLogFieldsToOTelLabels(fields)...), + ) } type bridgeFieldEncoder struct { diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index d7fb6ea21..48d08eec8 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -179,10 +179,9 @@ func (t *MockTracer) DeferredContextSetupHook(ctx context.Context, span otel.Spa } type MockEvent struct { - CtxAttributes baggage.Map - Timestamp time.Time - Name string - Attributes baggage.Map + Timestamp time.Time + Name string + Attributes baggage.Map } type MockSpan struct { @@ -245,7 +244,7 @@ func (s *MockSpan) End(options ...otel.SpanOption) { s.mockTracer.FinishedSpans = append(s.mockTracer.FinishedSpans, s) } -func (s *MockSpan) RecordError(ctx context.Context, err error, opts ...otel.ErrorOption) { +func (s *MockSpan) RecordError(err error, opts ...otel.EventOption) { if err == nil { return // no-op on nil error } @@ -254,37 +253,25 @@ func (s *MockSpan) RecordError(ctx context.Context, err error, opts ...otel.Erro return // already finished } - cfg := otel.NewErrorConfig(opts...) - - if cfg.Timestamp.IsZero() { - cfg.Timestamp = time.Now() - } - - if cfg.StatusCode != codes.Ok { - s.SetStatus(cfg.StatusCode, "") - } - - s.AddEventWithTimestamp(ctx, cfg.Timestamp, "error", + s.SetStatus(codes.Error, "") + opts = append(opts, otel.WithAttributes( label.String("error.type", reflect.TypeOf(err).String()), label.String("error.message", err.Error()), - ) + )) + s.AddEvent("error", opts...) } func (s *MockSpan) Tracer() otel.Tracer { return s.officialTracer } -func (s *MockSpan) AddEvent(ctx context.Context, name string, attrs ...label.KeyValue) { - s.AddEventWithTimestamp(ctx, time.Now(), name, attrs...) -} - -func (s *MockSpan) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, name string, attrs ...label.KeyValue) { +func (s *MockSpan) AddEvent(name string, o ...otel.EventOption) { + c := otel.NewEventConfig(o...) s.Events = append(s.Events, MockEvent{ - CtxAttributes: baggage.MapFromContext(ctx), - Timestamp: timestamp, - Name: name, + Timestamp: c.Timestamp, + Name: name, Attributes: baggage.NewMap(baggage.MapUpdate{ - MultiKV: attrs, + MultiKV: c.Attributes, }), }) } diff --git a/config.go b/config.go index 70de99306..9113cb569 100644 --- a/config.go +++ b/config.go @@ -17,7 +17,6 @@ package otel import ( "time" - "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/label" ) @@ -51,44 +50,6 @@ func WithInstrumentationVersion(version string) TracerOption { return instVersionTracerOption(version) } -// ErrorConfig is a group of options for an error event. -type ErrorConfig struct { - Timestamp time.Time - StatusCode codes.Code -} - -// NewErrorConfig applies all the options to a returned ErrorConfig. -func NewErrorConfig(options ...ErrorOption) *ErrorConfig { - c := new(ErrorConfig) - for _, option := range options { - option.ApplyError(c) - } - return c -} - -// ErrorOption applies an option to a ErrorConfig. -type ErrorOption interface { - ApplyError(*ErrorConfig) -} - -type errorTimeOption time.Time - -func (o errorTimeOption) ApplyError(c *ErrorConfig) { c.Timestamp = time.Time(o) } - -// WithErrorTime sets the time at which the error event should be recorded. -func WithErrorTime(t time.Time) ErrorOption { - return errorTimeOption(t) -} - -type errorStatusOption struct{ c codes.Code } - -func (o errorStatusOption) ApplyError(c *ErrorConfig) { c.StatusCode = o.c } - -// WithErrorStatus indicates the span status that should be set when recording an error event. -func WithErrorStatus(c codes.Code) ErrorOption { - return errorStatusOption{c} -} - // SpanConfig is a group of options for a Span. type SpanConfig struct { // Attributes describe the associated qualities of a Span. @@ -124,27 +85,64 @@ type SpanOption interface { ApplySpan(*SpanConfig) } +// NewEventConfig applies all the EventOptions to a returned SpanConfig. If no +// timestamp option is passed, the returned SpanConfig will have a Timestamp +// set to the call time, otherwise no validation is performed on the returned +// SpanConfig. +func NewEventConfig(options ...EventOption) *SpanConfig { + c := new(SpanConfig) + for _, option := range options { + option.ApplyEvent(c) + } + if c.Timestamp.IsZero() { + c.Timestamp = time.Now() + } + return c +} + +// EventOption applies span event options to a SpanConfig. +type EventOption interface { + ApplyEvent(*SpanConfig) +} + +// LifeCycleOption applies span life-cycle options to a SpanConfig. These +// options set values releated to events in a spans life-cycle like starting, +// ending, experiencing an error and other user defined notable events. +type LifeCycleOption interface { + SpanOption + EventOption +} + type attributeSpanOption []label.KeyValue -func (o attributeSpanOption) ApplySpan(c *SpanConfig) { +func (o attributeSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) } +func (o attributeSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) } +func (o attributeSpanOption) apply(c *SpanConfig) { c.Attributes = append(c.Attributes, []label.KeyValue(o)...) } -// WithAttributes adds the attributes to a span. These attributes are meant to -// provide additional information about the work the Span represents. The -// attributes are added to the existing Span attributes, i.e. this does not -// overwrite. -func WithAttributes(attributes ...label.KeyValue) SpanOption { +// WithAttributes adds the attributes related to a span life-cycle event. +// These attributes are used to describe the work a Span represents when this +// option is provided to a Span's start or end events. Otherwise, these +// attributes provide additional information about the event being recorded +// (e.g. error, state change, processing progress, system event). +// +// 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 ...label.KeyValue) LifeCycleOption { return attributeSpanOption(attributes) } type timestampSpanOption time.Time -func (o timestampSpanOption) ApplySpan(c *SpanConfig) { c.Timestamp = time.Time(o) } +func (o timestampSpanOption) ApplySpan(c *SpanConfig) { o.apply(c) } +func (o timestampSpanOption) ApplyEvent(c *SpanConfig) { o.apply(c) } +func (o timestampSpanOption) apply(c *SpanConfig) { c.Timestamp = time.Time(o) } -// WithTimestamp sets the time of a Span life-cycle moment (e.g. started or -// stopped). -func WithTimestamp(t time.Time) SpanOption { +// WithTimestamp sets the time of a Span life-cycle moment (e.g. started, +// stopped, errored). +func WithTimestamp(t time.Time) LifeCycleOption { return timestampSpanOption(t) } diff --git a/example/basic/main.go b/example/basic/main.go index e7d86d5c1..394576559 100644 --- a/example/basic/main.go +++ b/example/basic/main.go @@ -88,7 +88,7 @@ func main() { ctx, span = tracer.Start(ctx, "operation") defer span.End() - span.AddEvent(ctx, "Nice operation!", label.Int("bogons", 100)) + span.AddEvent("Nice operation!", otel.WithAttributes(label.Int("bogons", 100))) span.SetAttributes(anotherKey.String("yes")) meter.RecordBatch( @@ -105,7 +105,7 @@ func main() { defer span.End() span.SetAttributes(lemonsKey.String("five")) - span.AddEvent(ctx, "Sub span event") + span.AddEvent("Sub span event") valuerecorder.Record(ctx, 1.3) return nil diff --git a/example/namedtracer/foo/foo.go b/example/namedtracer/foo/foo.go index d5ddbaaea..c4e50c8f8 100644 --- a/example/namedtracer/foo/foo.go +++ b/example/namedtracer/foo/foo.go @@ -34,10 +34,10 @@ func SubOperation(ctx context.Context) error { tr := global.Tracer("example/namedtracer/foo") var span otel.Span - ctx, span = tr.Start(ctx, "Sub operation...") + _, span = tr.Start(ctx, "Sub operation...") defer span.End() span.SetAttributes(lemonsKey.String("five")) - span.AddEvent(ctx, "Sub span event") + span.AddEvent("Sub span event") return nil } diff --git a/example/namedtracer/main.go b/example/namedtracer/main.go index f5e648f65..67cba82d0 100644 --- a/example/namedtracer/main.go +++ b/example/namedtracer/main.go @@ -68,7 +68,7 @@ func main() { var span otel.Span ctx, span = tracer.Start(ctx, "operation") defer span.End() - span.AddEvent(ctx, "Nice operation!", label.Int("bogons", 100)) + span.AddEvent("Nice operation!", otel.WithAttributes(label.Int("bogons", 100))) span.SetAttributes(anotherKey.String("yes")) if err := foo.SubOperation(ctx); err != nil { panic(err) diff --git a/oteltest/event.go b/oteltest/event.go index 175ccb510..f8a483c30 100644 --- a/oteltest/event.go +++ b/oteltest/event.go @@ -20,7 +20,7 @@ import ( "go.opentelemetry.io/otel/label" ) -// Event encapsulates the properties of calls to AddEvent or AddEventWithTimestamp. +// Event encapsulates the properties of calls to AddEvent. type Event struct { Timestamp time.Time Name string diff --git a/oteltest/harness.go b/oteltest/harness.go index c657eddb9..9739ae783 100644 --- a/oteltest/harness.go +++ b/oteltest/harness.go @@ -188,10 +188,10 @@ func (h *Harness) testSpan(tracerFactory func() otel.Tracer) { span.End() }, "#AddEvent": func(span otel.Span) { - span.AddEvent(context.Background(), "test event") + span.AddEvent("test event") }, "#AddEventWithTimestamp": func(span otel.Span) { - span.AddEventWithTimestamp(context.Background(), time.Now(), "test event") + span.AddEvent("test event", otel.WithTimestamp(time.Now().Add(1*time.Second))) }, "#SetStatus": func(span otel.Span) { span.SetStatus(codes.Error, "internal") diff --git a/oteltest/mock_span.go b/oteltest/mock_span.go index d7c13ebf2..86093ac5e 100644 --- a/oteltest/mock_span.go +++ b/oteltest/mock_span.go @@ -15,9 +15,6 @@ package oteltest import ( - "context" - "time" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/label" @@ -44,9 +41,7 @@ func (ms *MockSpan) SpanContext() otel.SpanContext { } // IsRecording always returns false for MockSpan. -func (ms *MockSpan) IsRecording() bool { - return false -} +func (ms *MockSpan) IsRecording() bool { return false } // SetStatus does nothing. func (ms *MockSpan) SetStatus(status codes.Code, msg string) { @@ -55,35 +50,22 @@ func (ms *MockSpan) SetStatus(status codes.Code, msg string) { } // SetError does nothing. -func (ms *MockSpan) SetError(v bool) { -} +func (ms *MockSpan) SetError(v bool) {} // SetAttributes does nothing. -func (ms *MockSpan) SetAttributes(attributes ...label.KeyValue) { -} +func (ms *MockSpan) SetAttributes(attributes ...label.KeyValue) {} // End does nothing. -func (ms *MockSpan) End(options ...otel.SpanOption) { -} +func (ms *MockSpan) End(options ...otel.SpanOption) {} // RecordError does nothing. -func (ms *MockSpan) RecordError(ctx context.Context, err error, opts ...otel.ErrorOption) { -} +func (ms *MockSpan) RecordError(err error, opts ...otel.EventOption) {} // SetName sets the span name. -func (ms *MockSpan) SetName(name string) { - ms.Name = name -} +func (ms *MockSpan) SetName(name string) { ms.Name = name } // Tracer returns MockTracer implementation of Tracer. -func (ms *MockSpan) Tracer() otel.Tracer { - return ms.tracer -} +func (ms *MockSpan) Tracer() otel.Tracer { return ms.tracer } // AddEvent does nothing. -func (ms *MockSpan) AddEvent(ctx context.Context, name string, attrs ...label.KeyValue) { -} - -// AddEventWithTimestamp does nothing. -func (ms *MockSpan) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, name string, attrs ...label.KeyValue) { -} +func (ms *MockSpan) AddEvent(string, ...otel.EventOption) {} diff --git a/oteltest/span.go b/oteltest/span.go index 4154d5261..a94ea5667 100644 --- a/oteltest/span.go +++ b/oteltest/span.go @@ -15,7 +15,6 @@ package oteltest import ( - "context" "fmt" "reflect" "sync" @@ -81,40 +80,28 @@ func (s *Span) End(opts ...otel.SpanOption) { } // RecordError records an error as a Span event. -func (s *Span) RecordError(ctx context.Context, err error, opts ...otel.ErrorOption) { +func (s *Span) RecordError(err error, opts ...otel.EventOption) { if err == nil || s.ended { return } - cfg := otel.NewErrorConfig(opts...) - - if cfg.Timestamp.IsZero() { - cfg.Timestamp = time.Now() - } - - if cfg.StatusCode != codes.Unset { - s.SetStatus(cfg.StatusCode, "") - } - errType := reflect.TypeOf(err) errTypeString := fmt.Sprintf("%s.%s", errType.PkgPath(), errType.Name()) if errTypeString == "." { errTypeString = errType.String() } - s.AddEventWithTimestamp(ctx, cfg.Timestamp, errorEventName, + s.SetStatus(codes.Error, "") + opts = append(opts, otel.WithAttributes( errorTypeKey.String(errTypeString), errorMessageKey.String(err.Error()), - ) + )) + + s.AddEvent(errorEventName, opts...) } // AddEvent adds an event to s. -func (s *Span) AddEvent(ctx context.Context, name string, attrs ...label.KeyValue) { - s.AddEventWithTimestamp(ctx, time.Now(), name, attrs...) -} - -// AddEventWithTimestamp adds an event that occurred at timestamp to s. -func (s *Span) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, name string, attrs ...label.KeyValue) { +func (s *Span) AddEvent(name string, o ...otel.EventOption) { s.lock.Lock() defer s.lock.Unlock() @@ -122,14 +109,18 @@ func (s *Span) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, n return } - attributes := make(map[label.Key]label.Value) + c := otel.NewEventConfig(o...) - for _, attr := range attrs { - attributes[attr.Key] = attr.Value + var attributes map[label.Key]label.Value + if l := len(c.Attributes); l > 0 { + attributes = make(map[label.Key]label.Value, l) + for _, attr := range c.Attributes { + attributes[attr.Key] = attr.Value + } } s.events = append(s.events, Event{ - Timestamp: timestamp, + Timestamp: c.Timestamp, Name: name, Attributes: attributes, }) @@ -186,16 +177,12 @@ func (s *Span) SetAttributes(attrs ...label.KeyValue) { // Name returns the name most recently set on s, either at or after creation // time. It cannot be change after End has been called on s. -func (s *Span) Name() string { - return s.name -} +func (s *Span) Name() string { return s.name } // ParentSpanID returns the SpanID of the parent Span. If s is a root Span, // and therefore does not have a parent, the returned SpanID will be invalid // (i.e., it will contain all zeroes). -func (s *Span) ParentSpanID() otel.SpanID { - return s.parentSpanID -} +func (s *Span) ParentSpanID() otel.SpanID { return s.parentSpanID } // Attributes returns the attributes set on s, either at or after creation // time. If the same attribute key was set multiple times, the last call will @@ -215,9 +202,7 @@ func (s *Span) Attributes() map[label.Key]label.Value { // Events returns the events set on s. Events cannot be changed after End has // been called on s. -func (s *Span) Events() []Event { - return s.events -} +func (s *Span) Events() []Event { return s.events } // Links returns the links set on s at creation time. If multiple links for // the same SpanContext were set, the last link will be used. @@ -233,37 +218,25 @@ func (s *Span) Links() map[otel.SpanContext][]label.KeyValue { // StartTime returns the time at which s was started. This will be the // wall-clock time unless a specific start time was provided. -func (s *Span) StartTime() time.Time { - return s.startTime -} +func (s *Span) StartTime() time.Time { return s.startTime } // EndTime returns the time at which s was ended if at has been ended, or // false otherwise. If the span has been ended, the returned time will be the // wall-clock time unless a specific end time was provided. -func (s *Span) EndTime() (time.Time, bool) { - return s.endTime, s.ended -} +func (s *Span) EndTime() (time.Time, bool) { return s.endTime, s.ended } // Ended returns whether s has been ended, i.e. whether End has been called at // least once on s. -func (s *Span) Ended() bool { - return s.ended -} +func (s *Span) Ended() bool { return s.ended } // StatusCode returns the code of the status most recently set on s, or // codes.OK if no status has been explicitly set. It cannot be changed after // End has been called on s. -func (s *Span) StatusCode() codes.Code { - return s.statusCode -} +func (s *Span) StatusCode() codes.Code { return s.statusCode } // StatusMessage returns the status message most recently set on s or the // empty string if no status message was set. -func (s *Span) StatusMessage() string { - return s.statusMessage -} +func (s *Span) StatusMessage() string { return s.statusMessage } // SpanKind returns the span kind of s. -func (s *Span) SpanKind() otel.SpanKind { - return s.spanKind -} +func (s *Span) SpanKind() otel.SpanKind { return s.spanKind } diff --git a/oteltest/span_test.go b/oteltest/span_test.go index 2b71dabbf..de0171da7 100644 --- a/oteltest/span_test.go +++ b/oteltest/span_test.go @@ -150,13 +150,13 @@ func TestSpan(t *testing.T) { e := matchers.NewExpecter(t) tracer := tp.Tracer(t.Name()) - ctx, span := tracer.Start(context.Background(), "test") + _, span := tracer.Start(context.Background(), "test") subject, ok := span.(*oteltest.Span) e.Expect(ok).ToBeTrue() testTime := time.Now() - subject.RecordError(ctx, s.err, otel.WithErrorTime(testTime)) + subject.RecordError(s.err, otel.WithTimestamp(testTime)) expectedEvents := []oteltest.Event{{ Timestamp: testTime, @@ -167,7 +167,7 @@ func TestSpan(t *testing.T) { }, }} e.Expect(subject.Events()).ToEqual(expectedEvents) - e.Expect(subject.StatusCode()).ToEqual(codes.Unset) + e.Expect(subject.StatusCode()).ToEqual(codes.Error) e.Expect(subject.StatusMessage()).ToEqual("") } }) @@ -178,7 +178,7 @@ func TestSpan(t *testing.T) { e := matchers.NewExpecter(t) tracer := tp.Tracer(t.Name()) - ctx, span := tracer.Start(context.Background(), "test") + _, span := tracer.Start(context.Background(), "test") subject, ok := span.(*oteltest.Span) e.Expect(ok).ToBeTrue() @@ -186,8 +186,7 @@ func TestSpan(t *testing.T) { errMsg := "test error message" testErr := ottest.NewTestError(errMsg) testTime := time.Now() - expStatusCode := codes.Error - subject.RecordError(ctx, testErr, otel.WithErrorTime(testTime), otel.WithErrorStatus(expStatusCode)) + subject.RecordError(testErr, otel.WithTimestamp(testTime)) expectedEvents := []oteltest.Event{{ Timestamp: testTime, @@ -198,7 +197,7 @@ func TestSpan(t *testing.T) { }, }} e.Expect(subject.Events()).ToEqual(expectedEvents) - e.Expect(subject.StatusCode()).ToEqual(expStatusCode) + e.Expect(subject.StatusCode()).ToEqual(codes.Error) }) t.Run("cannot be set after the span has ended", func(t *testing.T) { @@ -207,13 +206,13 @@ func TestSpan(t *testing.T) { e := matchers.NewExpecter(t) tracer := tp.Tracer(t.Name()) - ctx, span := tracer.Start(context.Background(), "test") + _, span := tracer.Start(context.Background(), "test") subject, ok := span.(*oteltest.Span) e.Expect(ok).ToBeTrue() subject.End() - subject.RecordError(ctx, errors.New("ignored error")) + subject.RecordError(errors.New("ignored error")) e.Expect(len(subject.Events())).ToEqual(0) }) @@ -224,12 +223,12 @@ func TestSpan(t *testing.T) { e := matchers.NewExpecter(t) tracer := tp.Tracer(t.Name()) - ctx, span := tracer.Start(context.Background(), "test") + _, span := tracer.Start(context.Background(), "test") subject, ok := span.(*oteltest.Span) e.Expect(ok).ToBeTrue() - subject.RecordError(ctx, nil) + subject.RecordError(nil) e.Expect(len(subject.Events())).ToEqual(0) }) @@ -465,7 +464,7 @@ func TestSpan(t *testing.T) { } event1Start := time.Now() - subject.AddEvent(context.Background(), event1Name, event1Attributes...) + subject.AddEvent(event1Name, otel.WithAttributes(event1Attributes...)) event1End := time.Now() event2Timestamp := time.Now().AddDate(5, 0, 0) @@ -474,7 +473,7 @@ func TestSpan(t *testing.T) { label.String("event2Attr", "abc"), } - subject.AddEventWithTimestamp(context.Background(), event2Timestamp, event2Name, event2Attributes...) + subject.AddEvent(event2Name, otel.WithTimestamp(event2Timestamp), otel.WithAttributes(event2Attributes...)) events := subject.Events() @@ -511,14 +510,14 @@ func TestSpan(t *testing.T) { subject, ok := span.(*oteltest.Span) e.Expect(ok).ToBeTrue() - subject.AddEvent(context.Background(), "test") + subject.AddEvent("test") e.Expect(len(subject.Events())).ToEqual(1) expectedEvent := subject.Events()[0] subject.End() - subject.AddEvent(context.Background(), "should not occur") + subject.AddEvent("should not occur") e.Expect(len(subject.Events())).ToEqual(1) e.Expect(subject.Events()[0]).ToEqual(expectedEvent) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index a4748ebe0..e6557ca71 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -15,7 +15,6 @@ package trace import ( - "context" "errors" "fmt" "reflect" @@ -119,11 +118,12 @@ func (s *span) End(options ...otel.SpanOption) { if recovered := recover(); recovered != nil { // Record but don't stop the panic. defer panic(recovered) - s.addEventWithTimestamp( - time.Now(), + s.addEvent( errorEventName, - errorTypeKey.String(typeStr(recovered)), - errorMessageKey.String(fmt.Sprint(recovered)), + otel.WithAttributes( + errorTypeKey.String(typeStr(recovered)), + errorMessageKey.String(fmt.Sprint(recovered)), + ), ) } @@ -151,29 +151,17 @@ func (s *span) End(options ...otel.SpanOption) { }) } -func (s *span) RecordError(ctx context.Context, err error, opts ...otel.ErrorOption) { - if s == nil || err == nil { +func (s *span) RecordError(err error, opts ...otel.EventOption) { + if s == nil || err == nil || !s.IsRecording() { return } - if !s.IsRecording() { - return - } - - cfg := otel.NewErrorConfig(opts...) - - if cfg.Timestamp.IsZero() { - cfg.Timestamp = time.Now() - } - - if cfg.StatusCode != codes.Unset { - s.SetStatus(cfg.StatusCode, "") - } - - s.AddEventWithTimestamp(ctx, cfg.Timestamp, errorEventName, + s.SetStatus(codes.Error, "") + opts = append(opts, otel.WithAttributes( errorTypeKey.String(typeStr(err)), errorMessageKey.String(err.Error()), - ) + )) + s.addEvent(errorEventName, opts...) } func typeStr(i interface{}) string { @@ -189,27 +177,22 @@ func (s *span) Tracer() otel.Tracer { return s.tracer } -func (s *span) AddEvent(ctx context.Context, name string, attrs ...label.KeyValue) { +func (s *span) AddEvent(name string, o ...otel.EventOption) { if !s.IsRecording() { return } - s.addEventWithTimestamp(time.Now(), name, attrs...) + s.addEvent(name, o...) } -func (s *span) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, name string, attrs ...label.KeyValue) { - if !s.IsRecording() { - return - } - s.addEventWithTimestamp(timestamp, name, attrs...) -} +func (s *span) addEvent(name string, o ...otel.EventOption) { + c := otel.NewEventConfig(o...) -func (s *span) addEventWithTimestamp(timestamp time.Time, name string, attrs ...label.KeyValue) { s.mu.Lock() defer s.mu.Unlock() s.messageEvents.add(export.Event{ Name: name, - Attributes: attrs, - Time: timestamp, + Attributes: c.Attributes, + Time: c.Timestamp, }) } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index f3b9697e6..d76f26f10 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -512,11 +512,11 @@ func TestEvents(t *testing.T) { k2v2 := label.Bool("key2", true) k3v3 := label.Int64("key3", 3) - span.AddEvent(context.Background(), "foo", label.String("key1", "value1")) - span.AddEvent(context.Background(), "bar", + span.AddEvent("foo", otel.WithAttributes(label.String("key1", "value1"))) + span.AddEvent("bar", otel.WithAttributes( label.Bool("key2", true), label.Int64("key3", 3), - ) + )) got, err := endSpan(te, span) if err != nil { t.Fatal(err) @@ -558,16 +558,16 @@ func TestEventsOverLimit(t *testing.T) { k2v2 := label.Bool("key2", false) k3v3 := label.String("key3", "value3") - span.AddEvent(context.Background(), "fooDrop", label.String("key1", "value1")) - span.AddEvent(context.Background(), "barDrop", + span.AddEvent("fooDrop", otel.WithAttributes(label.String("key1", "value1"))) + span.AddEvent("barDrop", otel.WithAttributes( label.Bool("key2", true), label.String("key3", "value3"), - ) - span.AddEvent(context.Background(), "foo", label.String("key1", "value1")) - span.AddEvent(context.Background(), "bar", + )) + span.AddEvent("foo", otel.WithAttributes(label.String("key1", "value1"))) + span.AddEvent("bar", otel.WithAttributes( label.Bool("key2", false), label.String("key3", "value3"), - ) + )) got, err := endSpan(te, span) if err != nil { t.Fatal(err) @@ -1062,9 +1062,7 @@ func TestRecordError(t *testing.T) { span := startSpan(tp, "RecordError") errTime := time.Now() - span.RecordError(context.Background(), s.err, - otel.WithErrorTime(errTime), - ) + span.RecordError(s.err, otel.WithTimestamp(errTime)) got, err := endSpan(te, span) if err != nil { @@ -1078,6 +1076,7 @@ func TestRecordError(t *testing.T) { }, ParentSpanID: sid, Name: "span0", + StatusCode: codes.Error, SpanKind: otel.SpanKindInternal, HasRemoteParent: true, MessageEvents: []export.Event{ @@ -1098,58 +1097,12 @@ func TestRecordError(t *testing.T) { } } -func TestRecordErrorWithStatus(t *testing.T) { - te := NewTestExporter() - tp := NewTracerProvider(WithSyncer(te)) - span := startSpan(tp, "RecordErrorWithStatus") - - testErr := ottest.NewTestError("test error") - errTime := time.Now() - testStatus := codes.Error - span.RecordError(context.Background(), testErr, - otel.WithErrorTime(errTime), - otel.WithErrorStatus(testStatus), - ) - - got, err := endSpan(te, span) - if err != nil { - t.Fatal(err) - } - - want := &export.SpanData{ - SpanContext: otel.SpanContext{ - TraceID: tid, - TraceFlags: 0x1, - }, - ParentSpanID: sid, - Name: "span0", - SpanKind: otel.SpanKindInternal, - StatusCode: codes.Error, - StatusMessage: "", - HasRemoteParent: true, - MessageEvents: []export.Event{ - { - Name: errorEventName, - Time: errTime, - Attributes: []label.KeyValue{ - errorTypeKey.String("go.opentelemetry.io/otel/internal/testing.TestError"), - errorMessageKey.String("test error"), - }, - }, - }, - InstrumentationLibrary: instrumentation.Library{Name: "RecordErrorWithStatus"}, - } - if diff := cmpDiff(got, want); diff != "" { - t.Errorf("SpanErrorOptions: -got +want %s", diff) - } -} - func TestRecordErrorNil(t *testing.T) { te := NewTestExporter() tp := NewTracerProvider(WithSyncer(te)) span := startSpan(tp, "RecordErrorNil") - span.RecordError(context.Background(), nil) + span.RecordError(nil) got, err := endSpan(te, span) if err != nil { diff --git a/trace.go b/trace.go index be798bb2a..edb95efa7 100644 --- a/trace.go +++ b/trace.go @@ -19,7 +19,6 @@ import ( "context" "encoding/hex" "encoding/json" - "time" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/label" @@ -241,18 +240,15 @@ type Span interface { // called other than setting the status. End(options ...SpanOption) - // AddEvent adds an event to the span. - AddEvent(ctx context.Context, name string, attrs ...label.KeyValue) - // AddEventWithTimestamp adds an event that occurred at timestamp to the - // Span. - AddEventWithTimestamp(ctx context.Context, timestamp time.Time, name string, attrs ...label.KeyValue) + // AddEvent adds an event with the provided name and options. + AddEvent(name string, options ...EventOption) // IsRecording returns the recording state of the Span. It will return // true if the Span is active and events can be recorded. IsRecording() bool // RecordError records an error as a Span event. - RecordError(ctx context.Context, err error, opts ...ErrorOption) + RecordError(err error, options ...EventOption) // SpanContext returns the SpanContext of the Span. The returned // SpanContext is usable even after the End has been called for the Span. diff --git a/trace_noop.go b/trace_noop.go index b36973bcd..8e1483edd 100644 --- a/trace_noop.go +++ b/trace_noop.go @@ -16,7 +16,6 @@ package otel import ( "context" - "time" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/label" @@ -73,16 +72,13 @@ func (noopSpan) SetAttributes(...label.KeyValue) {} func (noopSpan) End(...SpanOption) {} // RecordError does nothing. -func (noopSpan) RecordError(context.Context, error, ...ErrorOption) {} +func (noopSpan) RecordError(error, ...EventOption) {} // Tracer returns the Tracer that created this Span. func (noopSpan) Tracer() Tracer { return noopTracer{} } // AddEvent does nothing. -func (noopSpan) AddEvent(context.Context, string, ...label.KeyValue) {} - -// AddEventWithTimestamp does nothing. -func (noopSpan) AddEventWithTimestamp(context.Context, time.Time, string, ...label.KeyValue) {} +func (noopSpan) AddEvent(string, ...EventOption) {} // SetName does nothing. func (noopSpan) SetName(string) {}