Update Span API event methods (#1254)

* Update Span API event methods

Remove the context argument from the event methods. It is unused and can
be added back in as a passed option if needed in the future.

Update AddEvent to accept a required name and a set of options. These
options are the new EventOption type that can be used to configure a
SpanConfig Timestamp and Attributes.

Remove the AddEventWithTimestamp method as it is redundant to calling
AddEvent with a WithTimestamp option.

Update RecordError to also accept the EventOptions.

* Add changes to CHANGELOG

* Add LifeCycleOption

Use the LifeCycleOption to encapsulate the options passed to a span for
life cycle events.

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
 

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 {

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,
 		}),
 	})
 }

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)
 }
 

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

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
 }

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)

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

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")

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) {}