Convert XConfigure into constructors (#1155)

* Convert XConfigure into constructors

Previously, we discussed the possibility of converting
the config types into internal ones. But due to the
cyclic dependencies it introduces, we are only
converting XConfigure into constructors and document that
XConfig types are most likely are not going to be directly
used by developers.

In package documents, constructors will be nicely listed
under the config types and they won't be yet another
standalone symbol developers need to learn about.

Fixes #1130.

* Add the changes to the CHANGELOG

CHANGELOG.md

@@ -11,8 +11,8 @@ 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)
+- In the `go.opentelemetry.io/otel/api/trace` package, `NewTracerConfig` was added to construct new `TracerConfig`s.
+   This addition was made to conform with our project option conventions. (#1155)
 - Instrumentation library information was added to the Zipkin exporter. (#1119)
 
 ### Changed
@@ -32,6 +32,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Move `tools` package under `internal`. (#1141)
 - Move `go.opentelemetry.io/otel/api/correlation` package to `go.opentelemetry.io/otel/api/baggage`. (#1142)
    The `correlation.CorrelationContext` propagator has been renamed `baggage.Baggage`.  Other exported functions and types are unchanged.
+- In the `go.opentelemetry.io/otel/api/trace` package, `SpanConfigure` was renamed to `NewSpanConfig`. (#1155)
 
 ## [0.11.0] - 2020-08-24
 

api/trace/api.go

@@ -33,20 +33,22 @@ type Provider interface {
 }
 
 // TracerConfig is a group of options for a Tracer.
+//
+// Most users will use the tracer options instead.
 type TracerConfig struct {
 	// InstrumentationVersion is the version of the instrumentation library.
 	InstrumentationVersion string
 }
 
-// TracerConfigure applies all the options to a returned TracerConfig.
+// NewTracerConfig applies all the options to a returned TracerConfig.
 // The default value for all the fields of the returned TracerConfig are the
 // default zero value of the type. Also, this does not perform any validation
 // on the returned TracerConfig (e.g. no uniqueness checking or bounding of
 // data), instead it is left to the implementations of the SDK to perform this
 // action.
-func TracerConfigure(options []TracerOption) *TracerConfig {
+func NewTracerConfig(opts ...TracerOption) *TracerConfig {
 	config := new(TracerConfig)
-	for _, option := range options {
+	for _, option := range opts {
 		option.Apply(config)
 	}
 	return config
@@ -71,7 +73,10 @@ type Tracer interface {
 	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.
+// ErrorConfig provides options to set properties of an error
+// event at the time it is recorded.
+//
+// Most users will use the error options instead.
 type ErrorConfig struct {
 	Timestamp  time.Time
 	StatusCode codes.Code
@@ -138,6 +143,8 @@ type Span interface {
 }
 
 // SpanConfig is a group of options for a Span.
+//
+// Most users will use span options instead.
 type SpanConfig struct {
 	// Attributes describe the associated qualities of a Span.
 	Attributes []label.KeyValue
@@ -155,18 +162,18 @@ type SpanConfig struct {
 	SpanKind SpanKind
 }
 
-// SpanConfigure applies all the options to a returned SpanConfig.
+// NewSpanConfig 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)
+func NewSpanConfig(opts ...SpanOption) *SpanConfig {
+	c := new(SpanConfig)
+	for _, option := range opts {
+		option.Apply(c)
 	}
-	return config
+	return c
 }
 
 // SpanOption applies an option to a SpanConfig.

api/trace/span_test.go

@@ -23,7 +23,7 @@ import (
 	"go.opentelemetry.io/otel/label"
 )
 
-func TestSpanConfigure(t *testing.T) {
+func TestNewSpanConfig(t *testing.T) {
 	k1v1 := label.String("key1", "value1")
 	k1v2 := label.String("key1", "value2")
 	k2v2 := label.String("key2", "value2")
@@ -190,6 +190,6 @@ func TestSpanConfigure(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		assert.Equal(t, test.expected, SpanConfigure(test.options))
+		assert.Equal(t, test.expected, NewSpanConfig(test.options...))
 	}
 }

api/trace/traceprovider_test.go

@@ -20,7 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestTracerConfigure(t *testing.T) {
+func TestTracerConfig(t *testing.T) {
 	v1 := "semver:0.0.1"
 	v2 := "semver:1.0.0"
 	tests := []struct {
@@ -52,6 +52,7 @@ func TestTracerConfigure(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		assert.Equal(t, test.expected, TracerConfigure(test.options))
+		config := NewTracerConfig(test.options...)
+		assert.Equal(t, test.expected, config)
 	}
 }

api/trace/tracetest/mock_tracer.go

@@ -20,6 +20,7 @@ import (
 	"encoding/binary"
 	"sync/atomic"
 
+	"go.opentelemetry.io/otel/api/trace"
 	apitrace "go.opentelemetry.io/otel/api/trace"
 	otelparent "go.opentelemetry.io/otel/internal/trace/parent"
 )
@@ -48,7 +49,8 @@ var _ apitrace.Tracer = (*MockTracer)(nil)
 // If Parent SpanContext option is not specified then random TraceID is used.
 // No other options are supported.
 func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.SpanOption) (context.Context, apitrace.Span) {
-	config := apitrace.SpanConfigure(o)
+	config := trace.NewSpanConfig(o...)
+
 	var span *MockSpan
 	var sc apitrace.SpanContext
 

api/trace/tracetest/provider.go

@@ -41,7 +41,8 @@ type instrumentation struct {
 }
 
 func (p *Provider) Tracer(instName string, opts ...trace.TracerOption) trace.Tracer {
-	conf := trace.TracerConfigure(opts)
+	conf := trace.NewTracerConfig(opts...)
+
 	inst := instrumentation{
 		Name:    instName,
 		Version: conf.InstrumentationVersion,

api/trace/tracetest/span.go

@@ -63,7 +63,7 @@ func (s *Span) End(opts ...trace.SpanOption) {
 		return
 	}
 
-	c := trace.SpanConfigure(opts)
+	c := trace.NewSpanConfig(opts...)
 	s.endTime = time.Now()
 	if endTime := c.Timestamp; !endTime.IsZero() {
 		s.endTime = endTime

api/trace/tracetest/tracer.go

@@ -36,7 +36,7 @@ type Tracer struct {
 }
 
 func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) {
-	c := trace.SpanConfigure(opts)
+	c := trace.NewSpanConfig(opts...)
 	startTime := time.Now()
 	if st := c.Timestamp; !st.IsZero() {
 		startTime = st

bridge/opentracing/internal/mock.go

@@ -71,7 +71,7 @@ func NewMockTracer() *MockTracer {
 }
 
 func (t *MockTracer) Start(ctx context.Context, name string, opts ...oteltrace.SpanOption) (context.Context, oteltrace.Span) {
-	config := oteltrace.SpanConfigure(opts)
+	config := oteltrace.NewSpanConfig(opts...)
 	startTime := config.Timestamp
 	if startTime.IsZero() {
 		startTime = time.Now()
@@ -240,7 +240,7 @@ func (s *MockSpan) End(options ...oteltrace.SpanOption) {
 	if !s.EndTime.IsZero() {
 		return // already finished
 	}
-	config := oteltrace.SpanConfigure(options)
+	config := oteltrace.NewSpanConfig(options...)
 	endTime := config.Timestamp
 	if endTime.IsZero() {
 		endTime = time.Now()
@@ -259,7 +259,6 @@ func (s *MockSpan) RecordError(ctx context.Context, err error, opts ...oteltrace
 	}
 
 	cfg := oteltrace.ErrorConfig{}
-
 	for _, o := range opts {
 		o(&cfg)
 	}

sdk/trace/provider.go

@@ -22,6 +22,7 @@ import (
 	"go.opentelemetry.io/otel/sdk/instrumentation"
 	"go.opentelemetry.io/otel/sdk/resource"
 
+	"go.opentelemetry.io/otel/api/trace"
 	apitrace "go.opentelemetry.io/otel/api/trace"
 )
 
@@ -82,7 +83,8 @@ func NewProvider(opts ...ProviderOption) *Provider {
 // Tracer with the given name. If a tracer for the given name does not exist,
 // it is created first. If the name is empty, DefaultTracerName is used.
 func (p *Provider) Tracer(name string, opts ...apitrace.TracerOption) apitrace.Tracer {
-	c := apitrace.TracerConfigure(opts)
+	c := trace.NewTracerConfig(opts...)
+
 	p.mu.Lock()
 	defer p.mu.Unlock()
 	if name == "" {

sdk/trace/span.go

@@ -137,7 +137,7 @@ func (s *span) End(options ...apitrace.SpanOption) {
 	if !s.IsRecording() {
 		return
 	}
-	config := apitrace.SpanConfigure(options)
+	config := apitrace.NewSpanConfig(options...)
 	s.endOnce.Do(func() {
 		sps, _ := s.tracer.provider.spanProcessors.Load().(spanProcessorMap)
 		mustExportOrProcess := len(sps) > 0

sdk/trace/tracer.go

@@ -36,7 +36,7 @@ var _ apitrace.Tracer = &tracer{}
 // configured appropriately by any SpanOption passed. Any Timestamp option
 // passed will be used as the start time of the Span's life-cycle.
 func (tr *tracer) Start(ctx context.Context, name string, options ...apitrace.SpanOption) (context.Context, apitrace.Span) {
-	config := apitrace.SpanConfigure(options)
+	config := apitrace.NewSpanConfig(options...)
 
 	parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, config.NewRoot)