Avoid overriding configuration of tracer provider (open-telemetry#1633)

* sdk/trace: add missing options to tracer provider

This change adds `WithDefaultSampler` and `WithSpanLimits` to the tracer
provider and removed `WithConfig` from it.

Before this change, `WithConfig` is the only way to set sampler or
limits of a span. However, it is prone to misuse, since `WithConfig` can
override tracing configurations that are configured by `WithResource` or
`WithIDGenerator`.  Thus to fix this, it adds new functional options -
`WithDefaultSampler` and `WithSpanLimits` and removes `WithConfig`.

Resolves open-telemetry#1631.

* Update sdk/trace/provider.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update sdk/trace/provider.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* rebase and remove WithConfig

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

CHANGELOG.md

@@ -56,6 +56,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
    | Windows | 1.14       | amd64        |
    | Windows | 1.15       | 386          |
    | Windows | 1.14       | 386          |
+- Added `WithDefaultSampler` and `WithSpanLimits` to tracer provider. (#1633)
 
 ### Changed
 
@@ -89,6 +90,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `test-benchmark` is no longer a dependency of the `precommit` make target. (#1567)
 - Removed the `test-386` make target.
    This was replaced with a full compatibility testing suite (i.e. multi OS/arch) in the CI system. (#1567)
+- Removed `WithConfig` from tracer provider to avoid overriding configuration. (#1633)
 
 ### Fixed
 

example/namedtracer/main.go

@@ -45,11 +45,7 @@ func initTracer() {
 	}
 	bsp := sdktrace.NewBatchSpanProcessor(exp)
 	tp = sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(
-			sdktrace.Config{
-				DefaultSampler: sdktrace.AlwaysSample(),
-			},
-		),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithSpanProcessor(bsp),
 	)
 	otel.SetTracerProvider(tp)

example/otel-collector/main.go

@@ -69,7 +69,7 @@ func initProvider() func() {
 
 	bsp := sdktrace.NewBatchSpanProcessor(exp)
 	tracerProvider := sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithResource(res),
 		sdktrace.WithSpanProcessor(bsp),
 	)

exporters/otlp/internal/otlptest/otlptest.go

@@ -40,7 +40,7 @@ import (
 // themselves.
 func RunEndToEndTest(ctx context.Context, t *testing.T, exp *otlp.Exporter, mcTraces, mcMetrics Collector) {
 	pOpts := []sdktrace.TracerProviderOption{
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithBatcher(
 			exp,
 			// add following two options to ensure flush

exporters/otlp/otlpgrpc/example_test.go

@@ -49,7 +49,7 @@ func Example_insecure() {
 	}()
 
 	tp := sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithBatcher(
 			exp,
 			// add following two options to ensure flush
@@ -102,7 +102,7 @@ func Example_withTLS() {
 	}()
 
 	tp := sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithBatcher(
 			exp,
 			// add following two options to ensure flush
@@ -163,7 +163,7 @@ func Example_withDifferentSignalCollectors() {
 	}()
 
 	tp := sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithBatcher(
 			exp,
 			// add following two options to ensure flush

exporters/otlp/otlpgrpc/otlp_integration_test.go

@@ -333,7 +333,7 @@ func TestNewExporter_withMultipleAttributeTypes(t *testing.T) {
 	}()
 
 	tp := sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithBatcher(
 			exp,
 			// add following two options to ensure flush

exporters/trace/jaeger/jaeger.go

@@ -169,7 +169,12 @@ func NewExportPipeline(endpointOption EndpointOption, opts ...Option) (trace.Tra
 
 	pOpts := []sdktrace.TracerProviderOption{sdktrace.WithSyncer(exporter)}
 	if exporter.o.Config != nil {
-		pOpts = append(pOpts, sdktrace.WithConfig(*exporter.o.Config))
+		pOpts = append(pOpts,
+			sdktrace.WithDefaultSampler(exporter.o.Config.DefaultSampler),
+			sdktrace.WithIDGenerator(exporter.o.Config.IDGenerator),
+			sdktrace.WithSpanLimits(exporter.o.Config.SpanLimits),
+			sdktrace.WithResource(exporter.o.Config.Resource),
+		)
 	}
 	tp := sdktrace.NewTracerProvider(pOpts...)
 	return tp, exporter.Flush, nil

exporters/trace/jaeger/jaeger_test.go

@@ -338,7 +338,7 @@ func TestExporter_ExportSpan(t *testing.T) {
 	assert.NoError(t, err)
 
 	tp := sdktrace.NewTracerProvider(
-		sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()),
 		sdktrace.WithSyncer(exp),
 	)
 	otel.SetTracerProvider(tp)

sdk/trace/benchmark_test.go

@@ -149,6 +149,6 @@ func traceBenchmark(b *testing.B, name string, fn func(*testing.B, trace.Tracer)
 }
 
 func tracer(b *testing.B, name string, sampler sdktrace.Sampler) trace.Tracer {
-	tp := sdktrace.NewTracerProvider(sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sampler}))
+	tp := sdktrace.NewTracerProvider(sdktrace.WithDefaultSampler(sampler))
 	return tp.Tracer(name)
 }

sdk/trace/provider.go

@@ -274,13 +274,6 @@ func WithSpanProcessor(sp SpanProcessor) TracerProviderOption {
 	}
 }
 
-// WithConfig option sets the configuration to provider.
-func WithConfig(config Config) TracerProviderOption {
-	return func(opts *TracerProviderConfig) {
-		opts.config = config
-	}
-}
-
 // WithResource option attaches a resource to the provider.
 // The resource is added to the span when it is started.
 func WithResource(r *resource.Resource) TracerProviderOption {
@@ -295,3 +288,17 @@ func WithIDGenerator(g IDGenerator) TracerProviderOption {
 		opts.config.IDGenerator = g
 	}
 }
+
+// WithDefaultSampler option registers a DefaultSampler with the the TracerProvider.
+func WithDefaultSampler(s Sampler) TracerProviderOption {
+	return func(opts *TracerProviderConfig) {
+		opts.config.DefaultSampler = s
+	}
+}
+
+// WithSpanLimits option registers a SpanLimits with the the TracerProvider.
+func WithSpanLimits(sl SpanLimits) TracerProviderOption {
+	return func(opts *TracerProviderConfig) {
+		opts.config.SpanLimits = sl
+	}
+}

sdk/trace/trace_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	ottest "go.opentelemetry.io/otel/internal/internaltest"
+
 	export "go.opentelemetry.io/otel/sdk/export/trace"
 	"go.opentelemetry.io/otel/sdk/instrumentation"
 	"go.opentelemetry.io/otel/sdk/resource"
@@ -81,10 +82,10 @@ func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) {
 	harness := oteltest.NewHarness(t)
 
 	harness.TestTracerProvider(func() trace.TracerProvider {
-		return NewTracerProvider(WithConfig(Config{DefaultSampler: TraceIDRatioBased(0)}))
+		return NewTracerProvider(WithDefaultSampler(TraceIDRatioBased(0)))
 	})
 
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: TraceIDRatioBased(0)}))
+	tp := NewTracerProvider(WithDefaultSampler(TraceIDRatioBased(0)))
 	harness.TestTracer(func() trace.Tracer {
 		return tp.Tracer("")
 	})
@@ -269,7 +270,7 @@ func TestSampling(t *testing.T) {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			p := NewTracerProvider(WithConfig(Config{DefaultSampler: tc.sampler}))
+			p := NewTracerProvider(WithDefaultSampler(tc.sampler))
 			tr := p.Tracer("test")
 			var sampled int
 			for i := 0; i < total; i++ {
@@ -421,7 +422,7 @@ func TestSetSpanAttributes(t *testing.T) {
 func TestSamplerAttributesLocalChildSpan(t *testing.T) {
 	sampler := &testSampler{prefix: "span", t: t}
 	te := NewTestExporter()
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: sampler}), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(WithDefaultSampler(sampler), WithSyncer(te), WithResource(resource.Empty()))
 
 	ctx := context.Background()
 	ctx, span := startLocalSpan(tp, ctx, "SpanOne", "span0")
@@ -484,8 +485,7 @@ func TestSamplerAttributesLocalChildSpan(t *testing.T) {
 
 func TestSetSpanAttributesOverLimit(t *testing.T) {
 	te := NewTestExporter()
-	cfg := Config{SpanLimits: SpanLimits{AttributeCountLimit: 2}}
-	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(WithSpanLimits(SpanLimits{AttributeCountLimit: 2}), WithSyncer(te), WithResource(resource.Empty()))
 
 	span := startSpan(tp, "SpanAttributesOverLimit")
 	span.SetAttributes(
@@ -522,8 +522,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) {
 
 func TestSetSpanAttributesWithInvalidKey(t *testing.T) {
 	te := NewTestExporter()
-	cfg := Config{SpanLimits: SpanLimits{}}
-	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(WithSpanLimits(SpanLimits{}), WithSyncer(te), WithResource(resource.Empty()))
 
 	span := startSpan(tp, "SpanToSetInvalidKeyOrValue")
 	span.SetAttributes(
@@ -602,8 +601,7 @@ func TestEvents(t *testing.T) {
 
 func TestEventsOverLimit(t *testing.T) {
 	te := NewTestExporter()
-	cfg := Config{SpanLimits: SpanLimits{EventCountLimit: 2}}
-	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(WithSpanLimits(SpanLimits{EventCountLimit: 2}), WithSyncer(te), WithResource(resource.Empty()))
 
 	span := startSpan(tp, "EventsOverLimit")
 	k1v1 := attribute.String("key1", "value1")
@@ -693,13 +691,12 @@ func TestLinks(t *testing.T) {
 
 func TestLinksOverLimit(t *testing.T) {
 	te := NewTestExporter()
-	cfg := Config{SpanLimits: SpanLimits{LinkCountLimit: 2}}
 
 	sc1 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}}
 	sc2 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}}
 	sc3 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}}
 
-	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(WithSpanLimits(SpanLimits{LinkCountLimit: 2}), WithSyncer(te), WithResource(resource.Empty()))
 
 	span := startSpan(tp, "LinksOverLimit",
 		trace.WithLinks(
@@ -958,7 +955,7 @@ func TestEndSpanTwice(t *testing.T) {
 
 func TestStartSpanAfterEnd(t *testing.T) {
 	te := NewTestExporter()
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: AlwaysSample()}), WithSyncer(te))
+	tp := NewTracerProvider(WithDefaultSampler(AlwaysSample()), WithSyncer(te))
 	ctx := context.Background()
 
 	tr := tp.Tracer("SpanAfterEnd")
@@ -1003,7 +1000,7 @@ func TestStartSpanAfterEnd(t *testing.T) {
 
 func TestChildSpanCount(t *testing.T) {
 	te := NewTestExporter()
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: AlwaysSample()}), WithSyncer(te))
+	tp := NewTracerProvider(WithDefaultSampler(AlwaysSample()), WithSyncer(te))
 
 	tr := tp.Tracer("ChidSpanCount")
 	ctx, span0 := tr.Start(context.Background(), "parent")
@@ -1057,7 +1054,7 @@ func TestNilSpanEnd(t *testing.T) {
 
 func TestExecutionTracerTaskEnd(t *testing.T) {
 	var n uint64
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: NeverSample()}))
+	tp := NewTracerProvider(WithDefaultSampler(NeverSample()))
 	tr := tp.Tracer("Execution Tracer Task End")
 
 	executionTracerTaskEnd := func() {
@@ -1106,7 +1103,7 @@ func TestExecutionTracerTaskEnd(t *testing.T) {
 
 func TestCustomStartEndTime(t *testing.T) {
 	te := NewTestExporter()
-	tp := NewTracerProvider(WithSyncer(te), WithConfig(Config{DefaultSampler: AlwaysSample()}))
+	tp := NewTracerProvider(WithSyncer(te), WithDefaultSampler(AlwaysSample()))
 
 	startTime := time.Date(2019, time.August, 27, 14, 42, 0, 0, time.UTC)
 	endTime := startTime.Add(time.Second * 20)
@@ -1220,7 +1217,7 @@ func TestRecordErrorNil(t *testing.T) {
 
 func TestWithSpanKind(t *testing.T) {
 	te := NewTestExporter()
-	tp := NewTracerProvider(WithSyncer(te), WithConfig(Config{DefaultSampler: AlwaysSample()}), WithResource(resource.Empty()))
+	tp := NewTracerProvider(WithSyncer(te), WithDefaultSampler(AlwaysSample()), WithResource(resource.Empty()))
 	tr := tp.Tracer("withSpanKind")
 
 	_, span := tr.Start(context.Background(), "WithoutSpanKind")
@@ -1290,7 +1287,7 @@ func TestWithResource(t *testing.T) {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			te := NewTestExporter()
-			defaultOptions := []TracerProviderOption{WithSyncer(te), WithConfig(Config{DefaultSampler: AlwaysSample()})}
+			defaultOptions := []TracerProviderOption{WithSyncer(te), WithDefaultSampler(AlwaysSample())}
 			tp := NewTracerProvider(append(defaultOptions, tc.options...)...)
 			span := startSpan(tp, "WithResource")
 			span.SetAttributes(attribute.String("key1", "value1"))
@@ -1497,8 +1494,11 @@ func TestReadWriteSpan(t *testing.T) {
 
 func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) {
 	te := NewTestExporter()
-	cfg := Config{SpanLimits: SpanLimits{AttributePerEventCountLimit: 2}}
-	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(
+		WithSpanLimits(SpanLimits{AttributePerEventCountLimit: 2}),
+		WithSyncer(te),
+		WithResource(resource.Empty()),
+	)
 
 	span := startSpan(tp, "AddSpanEventWithOverLimitedAttributes")
 	span.AddEvent("test1", trace.WithAttributes(
@@ -1559,8 +1559,11 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) {
 
 func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) {
 	te := NewTestExporter()
-	cfg := Config{SpanLimits: SpanLimits{AttributePerLinkCountLimit: 1}}
-	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+	tp := NewTracerProvider(
+		WithSpanLimits(SpanLimits{AttributePerLinkCountLimit: 1}),
+		WithSyncer(te),
+		WithResource(resource.Empty()),
+	)
 
 	k1v1 := attribute.String("key1", "value1")
 	k2v2 := attribute.String("key2", "value2")
@@ -1708,7 +1711,7 @@ func TestSamplerTraceState(t *testing.T) {
 		ts := ts
 		t.Run(ts.name, func(t *testing.T) {
 			te := NewTestExporter()
-			tp := NewTracerProvider(WithConfig(Config{DefaultSampler: ts.sampler}), WithSyncer(te), WithResource(resource.Empty()))
+			tp := NewTracerProvider(WithDefaultSampler(ts.sampler), WithSyncer(te), WithResource(resource.Empty()))
 			tr := tp.Tracer("TraceState")
 
 			sc1 := trace.SpanContext{

sdk/trace/util_test.go

@@ -20,9 +20,7 @@ import (
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 )
 
-var testConfig = sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}
-
 func basicTracerProvider(t *testing.T) *sdktrace.TracerProvider {
-	tp := sdktrace.NewTracerProvider(sdktrace.WithConfig(testConfig))
+	tp := sdktrace.NewTracerProvider(sdktrace.WithDefaultSampler(sdktrace.AlwaysSample()))
 	return tp
 }