sdk/trace: removing ApplyConfig and Config

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, removes `WithSDK` functions from them, instead, introduces new
functions, for instance, `WithResource`, `WithDefaultSampler`, and
`WithSpanLimits`.

Resolves open-telemetry#1636.

CHANGELOG.md

@@ -13,6 +13,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Added `Marshaler` config option to `otlphttp` to enable otlp over json or protobufs. (#1586)
 - A `ForceFlush` method to the `"go.opentelemetry.io/otel/sdk/trace".TracerProvider` to flush all registered `SpanProcessor`s. (#1608)
 - Added `WithDefaultSampler` and `WithSpanLimits` to tracer provider. (#1633)
+- Added `WithDefaultSampler`, `WithSpanLimits` and `WithResource` to `go.opentelemetry.io/otel/exporters/trace/jaeger` package. (#1693)
+- Added `WithDefaultSampler`, `WithSpanLimits` and `WithResource` to `go.opentelemetry.io/otel/exporters/trace/zipkin` package. (#1693)
 
 ### Changed
 
@@ -32,6 +34,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
    These are now returned as a SpanProcessor interface from their respective constructors. (#1638)
 - Removed setting status to `Error` while recording an error as a span event in `RecordError`. (#1663)
 - Removed `WithConfig` from tracer provider to avoid overriding configuration. (#1633)
+- Removed `ApplyConfig` method and `Config` struct from tracer provider. (#1693)
+- Removed `WithSDK` from `go.opentelemetry.io/otel/exporters/trace/jaeger` package. (#1693)
+- Removed `WithSDK` from `go.opentelemetry.io/otel/exporters/trace/zipkin` package. (#1693)
 
 ### Fixed
 

example/jaeger/main.go

@@ -39,7 +39,7 @@ func initTracer() func() {
 				attribute.Float64("float", 312.23),
 			},
 		}),
-		jaeger.WithSDK(&sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		jaeger.WithDefaultSampler(sdktrace.AlwaysSample()),
 	)
 	if err != nil {
 		log.Fatal(err)

example/zipkin/main.go

@@ -42,7 +42,7 @@ func initTracer(url string) func() {
 		url,
 		"zipkin-test",
 		zipkin.WithLogger(logger),
-		zipkin.WithSDK(&sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
+		zipkin.WithDefaultSampler(sdktrace.AlwaysSample()),
 	)
 	if err != nil {
 		log.Fatal(err)

exporters/trace/jaeger/jaeger.go

@@ -28,6 +28,7 @@ import (
 	"go.opentelemetry.io/otel/codes"
 	gen "go.opentelemetry.io/otel/exporters/trace/jaeger/internal/gen-go/jaeger"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/trace"
 )
@@ -52,7 +53,14 @@ type options struct {
 	// BatchMaxCount defines the maximum number of spans sent in one batch
 	BatchMaxCount int
 
-	Config *sdktrace.Config
+	// DefaultSampler is the default sampler used when creating new spans.
+	DefaultSampler sdktrace.Sampler
+
+	// SpanLimits used to limit the number of attributes, events and links to a span.
+	SpanLimits sdktrace.SpanLimits
+
+	// Resource contains attributes representing an entity that produces telemetry.
+	Resource *resource.Resource
 
 	Disabled bool
 }
@@ -78,10 +86,25 @@ func WithBatchMaxCount(batchMaxCount int) Option {
 	}
 }
 
-// WithSDK sets the SDK config for the exporter pipeline.
-func WithSDK(config *sdktrace.Config) Option {
+// WithResource option attaches a resource to the provider.
+// The resource is added to the span when it is started.
+func WithResource(r *resource.Resource) Option {
+	return func(o *options) {
+		o.Resource = r
+	}
+}
+
+// WithDefaultSampler option registers a DefaultSampler with the the TracerProvider.
+func WithDefaultSampler(s sdktrace.Sampler) Option {
 	return func(o *options) {
-		o.Config = config
+		o.DefaultSampler = s
+	}
+}
+
+// WithSpanLimits option registers a SpanLimits with the the TracerProvider.
+func WithSpanLimits(sl sdktrace.SpanLimits) Option {
+	return func(o *options) {
+		o.SpanLimits = sl
 	}
 }
 
@@ -167,14 +190,11 @@ 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.WithDefaultSampler(exporter.o.Config.DefaultSampler),
-			sdktrace.WithIDGenerator(exporter.o.Config.IDGenerator),
-			sdktrace.WithSpanLimits(exporter.o.Config.SpanLimits),
-			sdktrace.WithResource(exporter.o.Config.Resource),
-		)
+	pOpts := []sdktrace.TracerProviderOption{
+		sdktrace.WithSyncer(exporter),
+		sdktrace.WithDefaultSampler(exporter.o.DefaultSampler),
+		sdktrace.WithSpanLimits(exporter.o.SpanLimits),
+		sdktrace.WithResource(exporter.o.Resource),
 	}
 	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"
 	"testing"
@@ -36,6 +37,7 @@ import (
 	"go.opentelemetry.io/otel/sdk/instrumentation"
 	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/semconv"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -112,9 +114,7 @@ func TestNewExportPipeline(t *testing.T) {
 			name:     "always on",
 			endpoint: WithCollectorEndpoint(collectorEndpoint),
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.AlwaysSample(),
-				}),
+				WithDefaultSampler(sdktrace.AlwaysSample()),
 			},
 			expectedProviderType: &sdktrace.TracerProvider{},
 			testSpanSampling:     true,
@@ -124,9 +124,7 @@ func TestNewExportPipeline(t *testing.T) {
 			name:     "never",
 			endpoint: WithCollectorEndpoint(collectorEndpoint),
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.NeverSample(),
-				}),
+				WithDefaultSampler(sdktrace.NeverSample()),
 			},
 			expectedProviderType: &sdktrace.TracerProvider{},
 			testSpanSampling:     true,
@@ -579,3 +577,79 @@ func TestErrorOnExportShutdownExporter(t *testing.T) {
 	assert.NoError(t, e.Shutdown(context.Background()))
 	assert.NoError(t, e.ExportSpans(context.Background(), nil))
 }
+
+type testCollectorEnpointWithSpans struct {
+	spansUploaded *[]*gen.Span
+}
+
+var _ batchUploader = (*testCollectorEnpointWithSpans)(nil)
+
+func (c *testCollectorEnpointWithSpans) upload(batch *gen.Batch) error {
+	*c.spansUploaded = append(*c.spansUploaded, batch.Spans...)
+	return nil
+}
+
+func withTestCollectorEnpointWithSpans(spansUploaded *[]*gen.Span) func() (batchUploader, error) {
+	return func() (batchUploader, error) {
+		return &testCollectorEnpointWithSpans{
+			spansUploaded: spansUploaded,
+		}, nil
+	}
+}
+
+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"
+		tagKey          = "key"
+		tagVal          = "val"
+		eventCountLimit = 10
+	)
+
+	var uploadedSpans []*gen.Span
+	tp, spanFlush, err := NewExportPipeline(
+		withTestCollectorEnpointWithSpans(&uploadedSpans),
+		WithResource(resource.NewWithAttributes(
+			semconv.ServiceNameKey.String(serviceName),
+			attribute.String(tagKey, tagVal),
+		)),
+		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())
+
+	assert.True(t, len(uploadedSpans) == 1)
+	uploadedSpan := uploadedSpans[0]
+	assert.Equal(t, len(uploadedSpan.GetLogs()), eventCountLimit)
+	assert.Condition(t, func() bool {
+		tagFound := false
+		serviceNameFound := false
+		for _, tag := range uploadedSpan.GetTags() {
+			if tag.GetKey() == tagKey && tag.GetVStr() == tagVal {
+				tagFound = true
+			}
+			if tag.GetKey() == string(semconv.ServiceNameKey) && tag.GetVStr() == serviceName {
+				serviceNameFound = true
+			}
+		}
+		return tagFound && serviceNameFound
+	})
+}

exporters/trace/zipkin/model.go

@@ -146,10 +146,13 @@ var extraZipkinTags = []string{
 }
 
 func toZipkinTags(data *export.SpanSnapshot) map[string]string {
-	m := make(map[string]string, len(data.Attributes)+len(extraZipkinTags))
+	m := make(map[string]string, len(data.Attributes)+len(data.Resource.Attributes())+len(extraZipkinTags))
 	for _, kv := range data.Attributes {
 		m[(string)(kv.Key)] = kv.Value.Emit()
 	}
+	for _, kv := range data.Resource.Attributes() {
+		m[(string)(kv.Key)] = kv.Value.Emit()
+	}
 	if v, ok := m["error"]; ok && v == "false" {
 		delete(m, "error")
 	}

exporters/trace/zipkin/zipkin.go

@@ -29,6 +29,7 @@ import (
 
 	"go.opentelemetry.io/otel"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 )
 
@@ -52,9 +53,11 @@ var (
 
 // Options contains configuration for the exporter.
 type options struct {
-	client *http.Client
-	logger *log.Logger
-	config *sdktrace.Config
+	client         *http.Client
+	logger         *log.Logger
+	defaultSampler sdktrace.Sampler
+	spanLimits     sdktrace.SpanLimits
+	resource       *resource.Resource
 }
 
 // Option defines a function that configures the exporter.
@@ -74,10 +77,25 @@ 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
+// WithResource option attaches a resource to the provider.
+// The resource is added to the span when it is started.
+func WithResource(r *resource.Resource) Option {
+	return func(opts *options) {
+		opts.resource = r
+	}
+}
+
+// WithDefaultSampler option registers a DefaultSampler with the the TracerProvider.
+func WithDefaultSampler(s sdktrace.Sampler) Option {
+	return func(opts *options) {
+		opts.defaultSampler = s
+	}
+}
+
+// WithSpanLimits option registers a SpanLimits with the the TracerProvider.
+func WithSpanLimits(sl sdktrace.SpanLimits) Option {
+	return func(opts *options) {
+		opts.spanLimits = sl
 	}
 }
 
@@ -118,10 +136,12 @@ func NewExportPipeline(collectorURL, serviceName string, opts ...Option) (*sdktr
 		return nil, err
 	}
 
-	tp := sdktrace.NewTracerProvider(sdktrace.WithBatcher(exporter))
-	if exporter.o.config != nil {
-		tp.ApplyConfig(*exporter.o.config)
-	}
+	tp := sdktrace.NewTracerProvider(
+		sdktrace.WithBatcher(exporter),
+		sdktrace.WithDefaultSampler(exporter.o.defaultSampler),
+		sdktrace.WithSpanLimits(exporter.o.spanLimits),
+		sdktrace.WithResource(exporter.o.resource),
+	)
 
 	return tp, err
 }

exporters/trace/zipkin/zipkin_test.go

@@ -31,9 +31,12 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/semconv"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -63,19 +66,15 @@ func TestNewExportPipeline(t *testing.T) {
 		{
 			name: "always on",
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.AlwaysSample(),
-				}),
+				WithDefaultSampler(sdktrace.AlwaysSample()),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: true,
 		},
 		{
 			name: "never",
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.NeverSample(),
-				}),
+				WithDefaultSampler(sdktrace.NeverSample()),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: false,
@@ -388,3 +387,45 @@ 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 (
+		tagKey          = "key"
+		tagVal          = "val"
+		eventCountLimit = 10
+	)
+
+	collector := startMockZipkinCollector(t)
+	defer collector.Close()
+
+	tp, err := NewExportPipeline(collector.url, serviceName,
+		WithResource(resource.NewWithAttributes(
+			semconv.ServiceNameKey.String(serviceName),
+			attribute.String(tagKey, tagVal),
+		)),
+		WithSpanLimits(sdktrace.SpanLimits{
+			EventCountLimit: eventCountLimit,
+		}),
+	)
+	require.NoError(t, err)
+
+	otel.SetTracerProvider(tp)
+	_, span := otel.Tracer("zipkin-tracer").Start(context.TODO(), "test-span")
+	for i := 0; i < eventCountLimit*2; i++ {
+		span.AddEvent(fmt.Sprintf("event-%d", i))
+	}
+	span.End()
+
+	err = tp.ForceFlush(context.TODO())
+	require.NoError(t, err)
+
+	checkFunc := func() bool {
+		return collector.ModelsLen() == 1
+	}
+	// It should wait for more than a default batch timeout of BatchSpanProcessor since Zipkin uses default options.
+	require.Eventually(t, checkFunc, 10*time.Second, 10*time.Millisecond)
+	model := collector.StealModels()[0]
+	require.Equal(t, len(model.Annotations), eventCountLimit)
+	require.Contains(t, model.Tags, tagKey)
+	require.Contains(t, model.Tags, string(semconv.ServiceNameKey))
+}