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

@@ -27,6 +27,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `trace.NewSpanContext()` can be used in conjunction with the `trace.SpanContextConfig` struct to initialize a new `SpanContext` where all values are known.
 - Renamed the `LabelSet` method of `"go.opentelemetry.io/otel/sdk/resource".Resource` to `Set`. (#1692)
 - Jaeger exporter populates Jaeger's Span Process from Resource. (#1673)
+- 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)
+- Update zipkin export pipeline in `go.opentelemetry.io/otel/exporters/trace/zipkin` package to add resources specified by `WithResource` its tags. (#1693)
 
 ### Removed
 
@@ -36,6 +39,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - 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 `jaeger.WithProcess`. (#1673)
+- Removed `ApplyConfig` method and `Config` struct from tracer provider. (#1693)
 
 ### Fixed
 

example/jaeger/main.go

@@ -42,6 +42,7 @@ func initTracer() func() {
 				attribute.Float64("float", 312.23),
 			),
 		}),
+		jaeger.WithSDKOptions(sdktrace.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.WithSDKOptions(sdktrace.WithDefaultSampler(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.WithDefaultSampler(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.WithDefaultSampler(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.WithDefaultSampler(sdktrace.NeverSample())),
 			},
 			expectedProviderType: &sdktrace.TracerProvider{},
 			testSpanSampling:     true,
@@ -886,3 +883,81 @@ func TestProcess(t *testing.T) {
 		})
 	}
 }
+
+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),
+		WithSDKOptions(
+			sdktrace.WithResource(resource.NewWithAttributes(
+				semconv.ServiceNameKey.String(serviceName),
+				attribute.String(tagKey, tagVal),
+			)),
+			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())
+
+	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,7 +146,10 @@ 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.Resource.Attributes() {
+		m[(string)(kv.Key)] = kv.Value.Emit()
+	}
 	for _, kv := range data.Attributes {
 		m[(string)(kv.Key)] = kv.Value.Emit()
 	}

exporters/trace/zipkin/zipkin.go

@@ -54,7 +54,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.
@@ -74,10 +74,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 for tracer provider of sdk.
+func WithSDKOptions(tpOpts ...sdktrace.TracerProviderOption) Option {
+	return func(opts *options) {
+		opts.tpOpts = tpOpts
 	}
 }
 
@@ -118,10 +118,8 @@ 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)
-	}
+	tpOpts := append(exporter.o.tpOpts, sdktrace.WithBatcher(exporter))
+	tp := sdktrace.NewTracerProvider(tpOpts...)
 
 	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(),
-				}),
+				WithSDKOptions(sdktrace.WithDefaultSampler(sdktrace.AlwaysSample())),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: true,
 		},
 		{
 			name: "never",
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.NeverSample(),
-				}),
+				WithSDKOptions(sdktrace.WithDefaultSampler(sdktrace.NeverSample())),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: false,
@@ -388,3 +387,47 @@ 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,
+		WithSDKOptions(
+			sdktrace.WithResource(resource.NewWithAttributes(
+				semconv.ServiceNameKey.String(serviceName),
+				attribute.String(tagKey, tagVal),
+			)),
+			sdktrace.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))
+}

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.