Update API configs.

CHANGELOG.md

@@ -45,6 +45,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Spans created by the global `Tracer` obtained from `go.opentelemetry.io/otel`, prior to a functioning `TracerProvider` being set, now propagate the span context from their parent if one exists. (#1901)
 - The `"go.opentelemetry.io/otel".Tracer` function now accepts tracer options. (#1902)
 - Move the `go.opentelemetry.io/otel/unit` package to `go.opentelemetry.io/otel/metric/unit`. (#1903)
+- Changed `go.opentelemetry.io/otel/trace.TracerConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config.) (#1921)
+- Changed `go.opentelemetry.io/otel/trace.SpanConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config). (#1921)
+- Changed `span.End()` now only accepts Options that are allowed at `end()`. (#1921)
+- Changed `go.opentelemetry.io/otel/metric.InstrumentConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config). (#1921)
+- Changed `go.opentelemetry.io/otel/metric.MeterConfig` to conform to the [Contributing guidelines](CONTRIBUTING.md#config). (#1921)
 - Refactor option types according to the contribution style guide. (#1882)
 
 ### Deprecated

CONTRIBUTING.md

@@ -190,8 +190,16 @@ will likely want to build custom options for the configuration, the `config`
 should be exported. Please, include in the documentation for the `config`
 how the user can extend the configuration.
 
-It is important that `config` are not shared across package boundaries.
-Meaning a `config` from one package should not be directly used by another.
+It is important that internal `config` are not shared across package boundaries.
+Meaning a `config` from one package should not be directly used by another. The
+one exception is the API packages.  The configs from the base API, eg.
+`go.opentelemetry.io/otel/trace.TracerConfig` and
+`go.opentelemetry.io/otel/metric.InstrumentConfig`, are intended to be consumed
+by the SDK therefor it is expected that these are exported.
+
+When a config is exported we want to maintain forward and backward
+compatibility, to achieve this no fields should be exported but should
+instead be accessed by methods.
 
 Optionally, it is common to include a `newConfig` function (with the same
 naming scheme). This function wraps any defaults setting and looping over

bridge/opentracing/bridge.go

@@ -98,7 +98,7 @@ func (s *bridgeSpan) Finish() {
 }
 
 func (s *bridgeSpan) FinishWithOptions(opts ot.FinishOptions) {
-	var otelOpts []trace.SpanOption
+	var otelOpts []trace.SpanEndOption
 
 	if !opts.FinishTime.IsZero() {
 		otelOpts = append(otelOpts, trace.WithTimestamp(opts.FinishTime))

bridge/opentracing/internal/mock.go

@@ -71,8 +71,8 @@ func NewMockTracer() *MockTracer {
 }
 
 func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanOption) (context.Context, trace.Span) {
-	config := trace.NewSpanConfig(opts...)
-	startTime := config.Timestamp
+	config := trace.NewSpanStartConfig(opts...)
+	startTime := config.Timestamp()
 	if startTime.IsZero() {
 		startTime = time.Now()
 	}
@@ -86,13 +86,13 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanO
 		officialTracer: t,
 		spanContext:    spanContext,
 		Attributes: baggage.NewMap(baggage.MapUpdate{
-			MultiKV: config.Attributes,
+			MultiKV: config.Attributes(),
 		}),
 		StartTime:    startTime,
 		EndTime:      time.Time{},
 		ParentSpanID: t.getParentSpanID(ctx, config),
 		Events:       nil,
-		SpanKind:     trace.ValidateSpanKind(config.SpanKind),
+		SpanKind:     trace.ValidateSpanKind(config.SpanKind()),
 	}
 	if !migration.SkipContextSetup(ctx) {
 		ctx = trace.ContextWithSpan(ctx, span)
@@ -137,7 +137,7 @@ func (t *MockTracer) getParentSpanID(ctx context.Context, config *trace.SpanConf
 }
 
 func (t *MockTracer) getParentSpanContext(ctx context.Context, config *trace.SpanConfig) trace.SpanContext {
-	if !config.NewRoot {
+	if !config.NewRoot() {
 		return trace.SpanContextFromContext(ctx)
 	}
 	return trace.SpanContext{}
@@ -232,12 +232,12 @@ func (s *MockSpan) applyUpdate(update baggage.MapUpdate) {
 	s.Attributes = s.Attributes.Apply(update)
 }
 
-func (s *MockSpan) End(options ...trace.SpanOption) {
+func (s *MockSpan) End(options ...trace.SpanEndOption) {
 	if !s.EndTime.IsZero() {
 		return // already finished
 	}
-	config := trace.NewSpanConfig(options...)
-	endTime := config.Timestamp
+	config := trace.NewSpanEndConfig(options...)
+	endTime := config.Timestamp()
 	if endTime.IsZero() {
 		endTime = time.Now()
 	}
@@ -269,10 +269,10 @@ func (s *MockSpan) Tracer() trace.Tracer {
 func (s *MockSpan) AddEvent(name string, o ...trace.EventOption) {
 	c := trace.NewEventConfig(o...)
 	s.Events = append(s.Events, MockEvent{
-		Timestamp: c.Timestamp,
+		Timestamp: c.Timestamp(),
 		Name:      name,
 		Attributes: baggage.NewMap(baggage.MapUpdate{
-			MultiKV: c.Attributes,
+			MultiKV: c.Attributes(),
 		}),
 	})
 }

internal/global/meter.go

@@ -153,7 +153,7 @@ func (p *meterProvider) Meter(instrumentationName string, opts ...metric.MeterOp
 
 	key := meterKey{
 		Name:    instrumentationName,
-		Version: metric.NewMeterConfig(opts...).InstrumentationVersion,
+		Version: metric.NewMeterConfig(opts...).InstrumentationVersion(),
 	}
 	entry, ok := p.meters[key]
 	if !ok {

internal/global/trace.go

@@ -92,7 +92,7 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
 
 	key := il{
 		name:    name,
-		version: trace.NewTracerConfig(opts...).InstrumentationVersion,
+		version: trace.NewTracerConfig(opts...).InstrumentationVersion(),
 	}
 
 	if p.tracers == nil {
@@ -175,7 +175,7 @@ func (nonRecordingSpan) SetError(bool) {}
 func (nonRecordingSpan) SetAttributes(...attribute.KeyValue) {}
 
 // End does nothing.
-func (nonRecordingSpan) End(...trace.SpanOption) {}
+func (nonRecordingSpan) End(...trace.SpanEndOption) {}
 
 // RecordError does nothing.
 func (nonRecordingSpan) RecordError(error, ...trace.EventOption) {}

internal/global/trace_test.go

@@ -93,7 +93,6 @@ func TestTraceProviderDelegates(t *testing.T) {
 		tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
 			called = true
 			assert.Equal(t, "abc", name)
-			assert.Equal(t, []trace.TracerOption{trace.WithInstrumentationVersion("xyz")}, opts)
 			return trace.NewNoopTracerProvider().Tracer("")
 		},
 	})
@@ -131,7 +130,6 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) {
 		tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
 			newVal := atomic.AddInt32(&called, 1)
 			assert.Equal(t, "abc", name)
-			assert.Equal(t, []trace.TracerOption{trace.WithInstrumentationVersion("xyz")}, opts)
 			if newVal == 10 {
 				// Signal the goroutine to finish.
 				close(quit)
@@ -175,7 +173,6 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) {
 	otel.SetTracerProvider(fnTracerProvider{
 		tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
 			assert.Equal(t, "abc", name)
-			assert.Equal(t, []trace.TracerOption{trace.WithInstrumentationVersion("xyz")}, opts)
 			return fnTracer{
 				start: func(ctx context.Context, spanName string, opts ...trace.SpanOption) (context.Context, trace.Span) {
 					newVal := atomic.AddInt32(&called, 1)

metric/config.go

@@ -20,109 +20,117 @@ import (
 
 // InstrumentConfig contains options for metric instrument descriptors.
 type InstrumentConfig struct {
-	// Description describes the instrument in human-readable terms.
-	Description string
-	// Unit describes the measurement unit for a instrument.
-	Unit unit.Unit
-	// InstrumentationName is the name of the library providing
-	// instrumentation.
-	InstrumentationName string
-	// InstrumentationVersion is the version of the library providing
-	// instrumentation.
-	InstrumentationVersion string
+	description            string
+	unit                   unit.Unit
+	instrumentationName    string
+	instrumentationVersion string
+}
+
+// Description describes the instrument in human-readable terms.
+func (cfg InstrumentConfig) Description() string {
+	return cfg.description
+}
+
+// Unit describes the measurement unit for a instrument.
+func (cfg InstrumentConfig) Unit() unit.Unit {
+	return cfg.unit
+}
+
+// InstrumentationName is the name of the library providing
+// instrumentation.
+func (cfg InstrumentConfig) InstrumentationName() string {
+	return cfg.instrumentationName
+}
+
+// InstrumentationVersion is the version of the library providing
+// instrumentation.
+func (cfg InstrumentConfig) InstrumentationVersion() string {
+	return cfg.instrumentationVersion
 }
 
 // InstrumentOption is an interface for applying metric instrument options.
 type InstrumentOption interface {
 	// ApplyMeter is used to set a InstrumentOption value of a
 	// InstrumentConfig.
-	ApplyInstrument(*InstrumentConfig)
+	applyInstrument(*InstrumentConfig)
 }
 
 // NewInstrumentConfig creates a new InstrumentConfig
 // and applies all the given options.
 func NewInstrumentConfig(opts ...InstrumentOption) InstrumentConfig {
 	var config InstrumentConfig
 	for _, o := range opts {
-		o.ApplyInstrument(&config)
+		o.applyInstrument(&config)
 	}
 	return config
 }
 
-// WithDescription applies provided description.
-func WithDescription(desc string) InstrumentOption {
-	return descriptionOption(desc)
-}
+type instrumentOptionFunc func(*InstrumentConfig)
 
-type descriptionOption string
+func (fn instrumentOptionFunc) applyInstrument(cfg *InstrumentConfig) {
+	fn(cfg)
+}
 
-func (d descriptionOption) ApplyInstrument(config *InstrumentConfig) {
-	config.Description = string(d)
+// WithDescription applies provided description.
+func WithDescription(desc string) InstrumentOption {
+	return instrumentOptionFunc(func(cfg *InstrumentConfig) {
+		cfg.description = desc
+	})
 }
 
 // WithUnit applies provided unit.
 func WithUnit(unit unit.Unit) InstrumentOption {
-	return unitOption(unit)
-}
-
-type unitOption unit.Unit
-
-func (u unitOption) ApplyInstrument(config *InstrumentConfig) {
-	config.Unit = unit.Unit(u)
+	return instrumentOptionFunc(func(cfg *InstrumentConfig) {
+		cfg.unit = unit
+	})
 }
 
 // WithInstrumentationName sets the instrumentation name.
 func WithInstrumentationName(name string) InstrumentOption {
-	return instrumentationNameOption(name)
-}
-
-type instrumentationNameOption string
-
-func (i instrumentationNameOption) ApplyInstrument(config *InstrumentConfig) {
-	config.InstrumentationName = string(i)
+	return instrumentOptionFunc(func(cfg *InstrumentConfig) {
+		cfg.instrumentationName = name
+	})
 }
 
 // MeterConfig contains options for Meters.
 type MeterConfig struct {
 	// InstrumentationVersion is the version of the library providing
 	// instrumentation.
-	InstrumentationVersion string
+	instrumentationVersion string
+}
+
+func (cfg MeterConfig) InstrumentationVersion() string {
+	return cfg.instrumentationVersion
 }
 
 // MeterOption is an interface for applying Meter options.
 type MeterOption interface {
 	// ApplyMeter is used to set a MeterOption value of a MeterConfig.
-	ApplyMeter(*MeterConfig)
+	applyMeter(*MeterConfig)
 }
 
 // NewMeterConfig creates a new MeterConfig and applies
 // all the given options.
 func NewMeterConfig(opts ...MeterOption) MeterConfig {
 	var config MeterConfig
 	for _, o := range opts {
-		o.ApplyMeter(&config)
+		o.applyMeter(&config)
 	}
 	return config
 }
 
-// InstrumentationOption is an interface for applying instrumentation specific
-// options.
-type InstrumentationOption interface {
-	InstrumentOption
-	MeterOption
-}
-
 // WithInstrumentationVersion sets the instrumentation version.
-func WithInstrumentationVersion(version string) InstrumentationOption {
+//nolint:golint
+func WithInstrumentationVersion(version string) instrumentationVersionOption {
 	return instrumentationVersionOption(version)
 }
 
 type instrumentationVersionOption string
 
-func (i instrumentationVersionOption) ApplyMeter(config *MeterConfig) {
-	config.InstrumentationVersion = string(i)
+func (i instrumentationVersionOption) applyMeter(config *MeterConfig) {
+	config.instrumentationVersion = string(i)
 }
 
-func (i instrumentationVersionOption) ApplyInstrument(config *InstrumentConfig) {
-	config.InstrumentationVersion = string(i)
+func (i instrumentationVersionOption) applyInstrument(config *InstrumentConfig) {
+	config.instrumentationVersion = string(i)
 }

metric/metric.go

@@ -285,8 +285,8 @@ func (m Meter) newAsync(
 		return NoopAsync{}, nil
 	}
 	desc := NewDescriptor(name, mkind, nkind, opts...)
-	desc.config.InstrumentationName = m.name
-	desc.config.InstrumentationVersion = m.version
+	desc.config.instrumentationName = m.name
+	desc.config.instrumentationVersion = m.version
 	return m.impl.NewAsyncInstrument(desc, runner)
 }
 
@@ -304,8 +304,8 @@ func (m Meter) newSync(
 		return NoopSync{}, nil
 	}
 	desc := NewDescriptor(name, metricKind, numberKind, opts...)
-	desc.config.InstrumentationName = m.name
-	desc.config.InstrumentationVersion = m.version
+	desc.config.instrumentationName = m.name
+	desc.config.instrumentationVersion = m.version
 	return m.impl.NewSyncInstrument(desc)
 }
 
@@ -549,13 +549,13 @@ func (d Descriptor) InstrumentKind() InstrumentKind {
 // Description provides a human-readable description of the metric
 // instrument.
 func (d Descriptor) Description() string {
-	return d.config.Description
+	return d.config.Description()
 }
 
 // Unit describes the units of the metric instrument.  Unitless
 // metrics return the empty string.
 func (d Descriptor) Unit() unit.Unit {
-	return d.config.Unit
+	return d.config.Unit()
 }
 
 // NumberKind returns whether this instrument is declared over int64,
@@ -567,11 +567,11 @@ func (d Descriptor) NumberKind() number.Kind {
 // InstrumentationName returns the name of the library that provided
 // instrumentation for this instrument.
 func (d Descriptor) InstrumentationName() string {
-	return d.config.InstrumentationName
+	return d.config.InstrumentationName()
 }
 
 // InstrumentationVersion returns the version of the library that provided
 // instrumentation for this instrument.
 func (d Descriptor) InstrumentationVersion() string {
-	return d.config.InstrumentationVersion
+	return d.config.InstrumentationVersion()
 }

metric/metric_sdkapi.go

@@ -90,6 +90,6 @@ func WrapMeterImpl(impl MeterImpl, instrumentationName string, opts ...MeterOpti
 	return Meter{
 		impl:    impl,
 		name:    instrumentationName,
-		version: NewMeterConfig(opts...).InstrumentationVersion,
+		version: NewMeterConfig(opts...).InstrumentationVersion(),
 	}
 }