sdk/trace: removing ApplyConfig and Config (#1693)

This patch removes `ApplyConfig` method and `Config` struct from
`go.opentelemetry.io/otel/sdk/trace` package.  To ensure valid config
for TracerProvider, it adds `ensureValidTracerProviderConfig` private
function.
Jaeger and Zipkin have been used the `Config` directly across package
boundaries. Since `Config` is removed, they can't use it. This change,
thus, replaces `WithSDK` with `WithSDKOptions`.

Resolves #1636, #1705.

CHANGELOG.md

@@ -31,6 +31,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Jaeger exporter populates Jaeger's Span Process from Resource. (#1673)
 - `"go.opentelemetry.io/otel/sdk/resource".NewWithAttributes` will now drop any invalid attributes passed. (#1703)
 - `"go.opentelemetry.io/otel/sdk/resource".StringDetector` will now error if the produced attribute is invalid. (#1703)
+- Changed `WithSDK` to `WithSDKOptions` to accept variadic arguments of `TracerProviderOption` type in `go.opentelemetry.io/otel/exporters/trace/jaeger` package. (#1693)
+- Changed `WithSDK` to `WithSDKOptions` to accept variadic arguments of `TracerProviderOption` type in `go.opentelemetry.io/otel/exporters/trace/zipkin` package. (#1693)
 
 ### Removed
 
@@ -41,6 +43,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Removed `WithConfig` from tracer provider to avoid overriding configuration. (#1633)
 - Removed `serviceName` parameter from Zipkin exporter and uses resource instead. (#1549)
 - Removed `jaeger.WithProcess`. (#1673)
+- Removed `ApplyConfig` method and `Config` struct from tracer provider. (#1693)
 
 ### Fixed
 

example/jaeger/main.go

@@ -34,14 +34,14 @@ func initTracer() func() {
 	// Create and install Jaeger export pipeline.
 	flush, err := jaeger.InstallNewPipeline(
 		jaeger.WithCollectorEndpoint("http://localhost:14268/api/traces"),
-		jaeger.WithSDK(&sdktrace.Config{
-			DefaultSampler: sdktrace.AlwaysSample(),
-			Resource: resource.NewWithAttributes(
+		jaeger.WithSDKOptions(
+			sdktrace.WithSampler(sdktrace.AlwaysSample()),
+			sdktrace.WithResource(resource.NewWithAttributes(
 				semconv.ServiceNameKey.String("trace-demo"),
 				attribute.String("exporter", "jaeger"),
 				attribute.Float64("float", 312.23),
-			),
-		}),
+			)),
+		),
 	)
 	if err != nil {
 		log.Fatal(err)

example/zipkin/main.go

@@ -43,7 +43,7 @@ func initTracer(url string) func() {
 	exporter, err := zipkin.NewRawExporter(
 		url,
 		zipkin.WithLogger(logger),
-		zipkin.WithSDK(&sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		zipkin.WithSDKOptions(sdktrace.WithSampler(sdktrace.AlwaysSample())),
 	)
 	if err != nil {
 		log.Fatal(err)

exporters/trace/jaeger/jaeger.go

@@ -52,7 +52,8 @@ type options struct {
 	// BatchMaxCount defines the maximum number of spans sent in one batch
 	BatchMaxCount int
 
-	Config *sdktrace.Config
+	// TracerProviderOptions defines the options for tracer provider of sdk.
+	TracerProviderOptions []sdktrace.TracerProviderOption
 
 	Disabled bool
 }
@@ -71,10 +72,10 @@ func WithBatchMaxCount(batchMaxCount int) Option {
 	}
 }
 
-// WithSDK sets the SDK config for the exporter pipeline.
-func WithSDK(config *sdktrace.Config) Option {
+// WithSDKOptions configures options for tracer provider of sdk.
+func WithSDKOptions(opts ...sdktrace.TracerProviderOption) Option {
 	return func(o *options) {
-		o.Config = config
+		o.TracerProviderOptions = opts
 	}
 }
 
@@ -157,15 +158,7 @@ func NewExportPipeline(endpointOption EndpointOption, opts ...Option) (trace.Tra
 		return nil, nil, err
 	}
 
-	pOpts := []sdktrace.TracerProviderOption{sdktrace.WithSyncer(exporter)}
-	if exporter.o.Config != nil {
-		pOpts = append(pOpts,
-			sdktrace.WithSampler(exporter.o.Config.DefaultSampler),
-			sdktrace.WithIDGenerator(exporter.o.Config.IDGenerator),
-			sdktrace.WithSpanLimits(exporter.o.Config.SpanLimits),
-			sdktrace.WithResource(exporter.o.Config.Resource),
-		)
-	}
+	pOpts := append(exporter.o.TracerProviderOptions, sdktrace.WithSyncer(exporter))
 	tp := sdktrace.NewTracerProvider(pOpts...)
 	return tp, exporter.Flush, nil
 }

exporters/trace/jaeger/jaeger_test.go

@@ -17,6 +17,7 @@ package jaeger
 import (
 	"context"
 	"encoding/binary"
+	"fmt"
 	"os"
 	"sort"
 	"strings"
@@ -114,9 +115,7 @@ func TestNewExportPipeline(t *testing.T) {
 			name:     "always on",
 			endpoint: WithCollectorEndpoint(collectorEndpoint),
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.AlwaysSample(),
-				}),
+				WithSDKOptions(sdktrace.WithSampler(sdktrace.AlwaysSample())),
 			},
 			expectedProviderType: &sdktrace.TracerProvider{},
 			testSpanSampling:     true,
@@ -126,9 +125,7 @@ func TestNewExportPipeline(t *testing.T) {
 			name:     "never",
 			endpoint: WithCollectorEndpoint(collectorEndpoint),
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.NeverSample(),
-				}),
+				WithSDKOptions(sdktrace.WithSampler(sdktrace.NeverSample())),
 			},
 			expectedProviderType: &sdktrace.TracerProvider{},
 			testSpanSampling:     true,
@@ -281,11 +278,11 @@ func TestNewRawExporterShouldFailIfCollectorUnset(t *testing.T) {
 }
 
 type testCollectorEnpoint struct {
-	spansUploaded []*gen.Span
+	batchesUploaded []*gen.Batch
 }
 
 func (c *testCollectorEnpoint) upload(batch *gen.Batch) error {
-	c.spansUploaded = append(c.spansUploaded, batch.Spans...)
+	c.batchesUploaded = append(c.batchesUploaded, batch)
 	return nil
 }
 
@@ -297,6 +294,12 @@ func withTestCollectorEndpoint() func() (batchUploader, error) {
 	}
 }
 
+func withTestCollectorEndpointInjected(ce *testCollectorEnpoint) func() (batchUploader, error) {
+	return func() (batchUploader, error) {
+		return ce, nil
+	}
+}
+
 func TestExporter_ExportSpan(t *testing.T) {
 	const (
 		serviceName = "test-service"
@@ -306,12 +309,12 @@ func TestExporter_ExportSpan(t *testing.T) {
 	// Create Jaeger Exporter
 	exp, err := NewRawExporter(
 		withTestCollectorEndpoint(),
-		WithSDK(&sdktrace.Config{
-			Resource: resource.NewWithAttributes(
+		WithSDKOptions(
+			sdktrace.WithResource(resource.NewWithAttributes(
 				semconv.ServiceNameKey.String(serviceName),
 				attribute.String(tagKey, tagVal),
-			),
-		}),
+			)),
+		),
 	)
 
 	assert.NoError(t, err)
@@ -328,7 +331,8 @@ func TestExporter_ExportSpan(t *testing.T) {
 
 	exp.Flush()
 	tc := exp.uploader.(*testCollectorEnpoint)
-	assert.True(t, len(tc.spansUploaded) == 1)
+	assert.True(t, len(tc.batchesUploaded) == 1)
+	assert.True(t, len(tc.batchesUploaded[0].GetSpans()) == 1)
 }
 
 func Test_spanSnapshotToThrift(t *testing.T) {
@@ -886,3 +890,50 @@ func TestProcess(t *testing.T) {
 		})
 	}
 }
+
+func TestNewExporterPipelineWithOptions(t *testing.T) {
+	envStore, err := ottest.SetEnvVariables(map[string]string{
+		envDisabled: "false",
+	})
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, envStore.Restore())
+	}()
+
+	const (
+		serviceName     = "test-service"
+		eventCountLimit = 10
+	)
+
+	testCollector := &testCollectorEnpoint{}
+	tp, spanFlush, err := NewExportPipeline(
+		withTestCollectorEndpointInjected(testCollector),
+		WithSDKOptions(
+			sdktrace.WithResource(resource.NewWithAttributes(
+				semconv.ServiceNameKey.String(serviceName),
+			)),
+			sdktrace.WithSpanLimits(sdktrace.SpanLimits{
+				EventCountLimit: eventCountLimit,
+			}),
+		),
+	)
+	assert.NoError(t, err)
+
+	otel.SetTracerProvider(tp)
+	_, span := otel.Tracer("test-tracer").Start(context.Background(), "test-span")
+	for i := 0; i < eventCountLimit*2; i++ {
+		span.AddEvent(fmt.Sprintf("event-%d", i))
+	}
+	span.End()
+	spanFlush()
+
+	assert.True(t, span.SpanContext().IsValid())
+
+	batchesUploaded := testCollector.batchesUploaded
+	assert.True(t, len(batchesUploaded) == 1)
+	uploadedBatch := batchesUploaded[0]
+	assert.Equal(t, serviceName, uploadedBatch.GetProcess().GetServiceName())
+	assert.True(t, len(uploadedBatch.GetSpans()) == 1)
+	uploadedSpan := uploadedBatch.GetSpans()[0]
+	assert.Equal(t, eventCountLimit, len(uploadedSpan.GetLogs()))
+}

exporters/trace/zipkin/zipkin.go

@@ -53,7 +53,7 @@ var (
 type options struct {
 	client *http.Client
 	logger *log.Logger
-	config *sdktrace.Config
+	tpOpts []sdktrace.TracerProviderOption
 }
 
 // Option defines a function that configures the exporter.
@@ -73,10 +73,10 @@ func WithClient(client *http.Client) Option {
 	}
 }
 
-// WithSDK sets the SDK config for the exporter pipeline.
-func WithSDK(config *sdktrace.Config) Option {
-	return func(o *options) {
-		o.config = config
+// WithSDKOptions configures options passed to the created TracerProvider.
+func WithSDKOptions(tpOpts ...sdktrace.TracerProviderOption) Option {
+	return func(opts *options) {
+		opts.tpOpts = tpOpts
 	}
 }
 
@@ -116,10 +116,8 @@ func NewExportPipeline(collectorURL string, opts ...Option) (*sdktrace.TracerPro
 		return nil, err
 	}
 
-	tp := sdktrace.NewTracerProvider(sdktrace.WithBatcher(exporter))
-	if exporter.o.config != nil {
-		tp.ApplyConfig(*exporter.o.config)
-	}
+	tpOpts := append(exporter.o.tpOpts, sdktrace.WithBatcher(exporter))
+	tp := sdktrace.NewTracerProvider(tpOpts...)
 
 	return tp, err
 }

exporters/trace/zipkin/zipkin_test.go

@@ -63,19 +63,15 @@ func TestNewExportPipeline(t *testing.T) {
 		{
 			name: "always on",
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.AlwaysSample(),
-				}),
+				WithSDKOptions(sdktrace.WithSampler(sdktrace.AlwaysSample())),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: true,
 		},
 		{
 			name: "never",
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.NeverSample(),
-				}),
+				WithSDKOptions(sdktrace.WithSampler(sdktrace.NeverSample())),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: false,
@@ -389,3 +385,34 @@ func TestErrorOnExportShutdownExporter(t *testing.T) {
 	assert.NoError(t, exp.Shutdown(context.Background()))
 	assert.NoError(t, exp.ExportSpans(context.Background(), nil))
 }
+
+func TestNewExportPipelineWithOptions(t *testing.T) {
+	const eventCountLimit = 10
+
+	collector := startMockZipkinCollector(t)
+	defer collector.Close()
+
+	tp, err := NewExportPipeline(collector.url,
+		WithSDKOptions(
+			sdktrace.WithResource(resource.NewWithAttributes(
+				semconv.ServiceNameKey.String("zipkin-test"),
+			)),
+			sdktrace.WithSpanLimits(sdktrace.SpanLimits{
+				EventCountLimit: eventCountLimit,
+			}),
+		),
+	)
+	require.NoError(t, err)
+
+	otel.SetTracerProvider(tp)
+	_, span := otel.Tracer("zipkin-tracer").Start(context.Background(), "test-span")
+	for i := 0; i < eventCountLimit*2; i++ {
+		span.AddEvent(fmt.Sprintf("event-%d", i))
+	}
+	span.End()
+
+	require.NoError(t, tp.Shutdown(context.Background()))
+	require.Equal(t, 1, collector.ModelsLen())
+	model := collector.StealModels()[0]
+	require.Equal(t, len(model.Annotations), eventCountLimit)
+}

sdk/trace/config.go

@@ -14,25 +14,6 @@
 
 package trace // import "go.opentelemetry.io/otel/sdk/trace"
 
-import (
-	"go.opentelemetry.io/otel/sdk/resource"
-)
-
-// Config represents the global tracing configuration.
-type Config struct {
-	// DefaultSampler is the default sampler used when creating new spans.
-	DefaultSampler Sampler
-
-	// IDGenerator is for internal use only.
-	IDGenerator IDGenerator
-
-	// SpanLimits used to limit the number of attributes, events and links to a span.
-	SpanLimits SpanLimits
-
-	// Resource contains attributes representing an entity that produces telemetry.
-	Resource *resource.Resource
-}
-
 // SpanLimits represents the limits of a span.
 type SpanLimits struct {
 	// AttributeCountLimit is the maximum allowed span attribute count.
@@ -51,6 +32,24 @@ type SpanLimits struct {
 	AttributePerLinkCountLimit int
 }
 
+func (sl *SpanLimits) ensureDefault() {
+	if sl.EventCountLimit <= 0 {
+		sl.EventCountLimit = DefaultEventCountLimit
+	}
+	if sl.AttributeCountLimit <= 0 {
+		sl.AttributeCountLimit = DefaultAttributeCountLimit
+	}
+	if sl.LinkCountLimit <= 0 {
+		sl.LinkCountLimit = DefaultLinkCountLimit
+	}
+	if sl.AttributePerEventCountLimit <= 0 {
+		sl.AttributePerEventCountLimit = DefaultAttributePerEventCountLimit
+	}
+	if sl.AttributePerLinkCountLimit <= 0 {
+		sl.AttributePerLinkCountLimit = DefaultAttributePerLinkCountLimit
+	}
+}
+
 const (
 	// DefaultAttributeCountLimit is the default maximum allowed span attribute count.
 	DefaultAttributeCountLimit = 128