Unify API Span Start/End Options (#1108)

* Unify API Span Start/End Options

Replace both with `SpanOption`. Add a unified `SpanConfig` to match and
a `SpanConfigure` function to parse a `SpanConfig` from `SpanOption`s.

Update all the related options to use new `SpanOption`s.

* No non-zero SpanConfig defaults

The SDK uses an internal clock for the current time that cannot be use
if it does not know the time has not been set.

* Append attributes for WithAttributes

This preserves existing behavior.

* Add unit test for SpanConfigure

* Propagate changes

* Update append option documentation

* Update testing comments

* Move comments on guarantees to appropriate function

* Add documentation for SDK methods

Include SDK implementation specific information in the Tracer Start
method and Span End method.

* Add changes to Changelog

* Apply suggestions from code review

Co-authored-by: ET <[email protected]>

* Update the SpanKind comment in the  SpanConfig

Try for a less tautological comment.

Co-authored-by: ET <[email protected]>

CHANGELOG.md

@@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Added
 
+- A `SpanConfigure` function in `go.opentelemetry.io/otel/api/trace` to create a new `SpanConfig` from `SpanOption`s. (#1108)
 - In the `go.opentelemetry.io/otel/api/trace` package a new `TracerConfigure` function was added to configure a new `TracerConfig`.
    This addition was made to conform with our project option conventions. (#1109)
 - Instrumentation library information was added to the Zipkin exporter. (#1119)
@@ -19,6 +20,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Add reconnecting udp connection type to Jaeger exporter.
    This change adds a new optional implementation of the udp conn interface used to detect changes to an agent's host dns record.
    It then adopts the new destination address to ensure the exporter doesn't get stuck. This change was ported from jaegertracing/jaeger-client-go#520. (#1063)
+- Replace `StartOption` and `EndOption` in `go.opentelemetry.io/otel/api/trace` with `SpanOption`.
+   This change is matched by replacing the `StartConfig` and `EndConfig` with a unified `SpanConfig`. (#1108)
+- Replace the `LinkedTo` span option in `go.opentelemetry.io/otel/api/trace` with `WithLinks`.
+   This is be more consistent with our other option patterns, i.e. passing the item to be configured directly instead of its component parts, and provides a cleaner function signature. (#1108)
 - The `go.opentelemetry.io/otel/api/trace` `TracerOption` was changed to an interface to conform to project option conventions. (#1109)
 - Rename Jaeger tags used for instrumentation library information to reflect changes in OpenTelemetry specification. (#1119)
 

api/global/internal/trace.go

@@ -116,7 +116,7 @@ func (t *tracer) setDelegate(provider trace.Provider) {
 
 // Start implements trace.Tracer by forwarding the call to t.delegate if
 // set, otherwise it forwards the call to a NoopTracer.
-func (t *tracer) Start(ctx context.Context, name string, opts ...trace.StartOption) (context.Context, trace.Span) {
+func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) {
 	if t.delegate != nil {
 		return t.delegate.Start(ctx, name, opts...)
 	}

api/trace/api.go

@@ -68,23 +68,7 @@ func WithInstrumentationVersion(version string) TracerOption {
 
 type Tracer interface {
 	// Start a span.
-	Start(ctx context.Context, spanName string, opts ...StartOption) (context.Context, Span)
-}
-
-// EndConfig provides options to set properties of span at the time of ending
-// the span.
-type EndConfig struct {
-	EndTime time.Time
-}
-
-// EndOption applies changes to EndConfig that sets options when the span is ended.
-type EndOption func(*EndConfig)
-
-// WithEndTime sets the end time of the span to provided time t, when it is ended.
-func WithEndTime(t time.Time) EndOption {
-	return func(c *EndConfig) {
-		c.EndTime = t
-	}
+	Start(ctx context.Context, spanName string, opts ...SpanOption) (context.Context, Span)
 }
 
 // ErrorConfig provides options to set properties of an error event at the time it is recorded.
@@ -116,7 +100,7 @@ type Span interface {
 
 	// End completes the span. No updates are allowed to span after it
 	// ends. The only exception is setting status of the span.
-	End(options ...EndOption)
+	End(options ...SpanOption)
 
 	// AddEvent adds an event to the span.
 	AddEvent(ctx context.Context, name string, attrs ...label.KeyValue)
@@ -153,18 +137,106 @@ type Span interface {
 	SetAttribute(string, interface{})
 }
 
-// StartOption applies changes to StartConfig that sets options at span start time.
-type StartOption func(*StartConfig)
-
-// StartConfig provides options to set properties of span at the time of starting
-// a new span.
-type StartConfig struct {
+// SpanConfig is a group of options for a Span.
+type SpanConfig struct {
+	// Attributes describe the associated qualities of a Span.
 	Attributes []label.KeyValue
-	StartTime  time.Time
-	Links      []Link
-	Record     bool
-	NewRoot    bool
-	SpanKind   SpanKind
+	// Timestamp is a time in a Span life-cycle.
+	Timestamp time.Time
+	// Links are the associations a Span has with other Spans.
+	Links []Link
+	// Record is the recording state of a Span.
+	Record bool
+	// NewRoot identifies a Span as the root Span for a new trace. This is
+	// commonly used when an existing trace crosses trust boundaries and the
+	// remote parent span context should be ignored for security.
+	NewRoot bool
+	// SpanKind is the role a Span has in a trace.
+	SpanKind SpanKind
+}
+
+// SpanConfigure applies all the options to a returned SpanConfig.
+// The default value for all the fields of the returned SpanConfig are the
+// default zero value of the type. Also, this does not perform any validation
+// on the returned SpanConfig (e.g. no uniqueness checking or bounding of
+// data). Instead, it is left to the implementations of the SDK to perform this
+// action.
+func SpanConfigure(options []SpanOption) *SpanConfig {
+	config := new(SpanConfig)
+	for _, option := range options {
+		option.Apply(config)
+	}
+	return config
+}
+
+// SpanOption applies an option to a SpanConfig.
+type SpanOption interface {
+	Apply(*SpanConfig)
+}
+
+type attributeSpanOption []label.KeyValue
+
+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 {
+	return attributeSpanOption(attributes)
+}
+
+type timestampSpanOption time.Time
+
+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 {
+	return timestampSpanOption(t)
+}
+
+type linksSpanOption []Link
+
+func (o linksSpanOption) Apply(c *SpanConfig) { c.Links = append(c.Links, []Link(o)...) }
+
+// WithLinks adds links to a Span. The links are added to the existing Span
+// links, i.e. this does not overwrite.
+func WithLinks(links ...Link) SpanOption {
+	return linksSpanOption(links)
+}
+
+type recordSpanOption bool
+
+func (o recordSpanOption) Apply(c *SpanConfig) { c.Record = bool(o) }
+
+// WithRecord specifies that the span should be recorded. It is important to
+// note that implementations may override this option, i.e. if the span is a
+// child of an un-sampled trace.
+func WithRecord() SpanOption {
+	return recordSpanOption(true)
+}
+
+type newRootSpanOption bool
+
+func (o newRootSpanOption) Apply(c *SpanConfig) { c.NewRoot = bool(o) }
+
+// WithNewRoot specifies that the Span should be treated as a root Span. Any
+// existing parent span context will be ignored when defining the Span's trace
+// identifiers.
+func WithNewRoot() SpanOption {
+	return newRootSpanOption(true)
+}
+
+type spanKindSpanOption SpanKind
+
+func (o spanKindSpanOption) Apply(c *SpanConfig) { c.SpanKind = SpanKind(o) }
+
+// WithSpanKind sets the SpanKind of a Span.
+func WithSpanKind(kind SpanKind) SpanOption {
+	return spanKindSpanOption(kind)
 }
 
 // Link is used to establish relationship between two spans within the same Trace or
@@ -235,55 +307,3 @@ func (sk SpanKind) String() string {
 		return "unspecified"
 	}
 }
-
-// WithStartTime sets the start time of the span to provided time t, when it is started.
-// In absence of this option, wall clock time is used as start time.
-// This option is typically used when starting of the span is delayed.
-func WithStartTime(t time.Time) StartOption {
-	return func(c *StartConfig) {
-		c.StartTime = t
-	}
-}
-
-// WithAttributes sets attributes to span. These attributes provides additional
-// data about the span.
-// Multiple `WithAttributes` options appends the attributes preserving the order.
-func WithAttributes(attrs ...label.KeyValue) StartOption {
-	return func(c *StartConfig) {
-		c.Attributes = append(c.Attributes, attrs...)
-	}
-}
-
-// WithRecord specifies that the span should be recorded.
-// Note that the implementation may still override this preference,
-// e.g., if the span is a child in an unsampled trace.
-func WithRecord() StartOption {
-	return func(c *StartConfig) {
-		c.Record = true
-	}
-}
-
-// WithNewRoot specifies that the current span or remote span context
-// in context passed to `Start` should be ignored when deciding about
-// a parent, which effectively means creating a span with new trace
-// ID. The current span and the remote span context may be added as
-// links to the span by the implementation.
-func WithNewRoot() StartOption {
-	return func(c *StartConfig) {
-		c.NewRoot = true
-	}
-}
-
-// LinkedTo allows instantiating a Span with initial Links.
-func LinkedTo(sc SpanContext, attrs ...label.KeyValue) StartOption {
-	return func(c *StartConfig) {
-		c.Links = append(c.Links, Link{sc, attrs})
-	}
-}
-
-// WithSpanKind specifies the role a Span on a Trace.
-func WithSpanKind(sk SpanKind) StartOption {
-	return func(c *StartConfig) {
-		c.SpanKind = sk
-	}
-}

api/trace/context_test.go

@@ -101,7 +101,7 @@ func (mockSpan) SetAttribute(k string, v interface{}) {
 }
 
 // End does nothing.
-func (mockSpan) End(options ...trace.EndOption) {
+func (mockSpan) End(options ...trace.SpanOption) {
 }
 
 // RecordError does nothing.

api/trace/noop_span.go

@@ -54,7 +54,7 @@ func (NoopSpan) SetAttribute(k string, v interface{}) {
 }
 
 // End does nothing.
-func (NoopSpan) End(options ...EndOption) {
+func (NoopSpan) End(options ...SpanOption) {
 }
 
 // RecordError does nothing.

api/trace/noop_trace.go

@@ -23,7 +23,7 @@ type NoopTracer struct{}
 var _ Tracer = NoopTracer{}
 
 // Start starts a noop span.
-func (NoopTracer) Start(ctx context.Context, name string, opts ...StartOption) (context.Context, Span) {
+func (NoopTracer) Start(ctx context.Context, name string, opts ...SpanOption) (context.Context, Span) {
 	span := NoopSpan{}
 	return ContextWithSpan(ctx, span), span
 }