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.
ijsong · Mar 11, 2021 · 61031e5 · 61031e5
1 parent 77aa218
commit 61031e5
Show file tree

Hide file tree

Showing 11 changed files with 150 additions and 127 deletions.
diff --git a/CHANGELOG.md b/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
 

diff --git a/example/jaeger/main.go b/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)

diff --git a/example/zipkin/main.go b/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)

diff --git a/exporters/trace/jaeger/jaeger.go b/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

diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go
@@ -112,9 +112,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 +122,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,

diff --git a/exporters/trace/zipkin/zipkin.go b/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
 }

diff --git a/exporters/trace/zipkin/zipkin_test.go b/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(),
-				}),
+				WithDefaultSampler(sdktrace.AlwaysSample()),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: true,
 		},
 		{
 			name: "never",
 			options: []Option{
-				WithSDK(&sdktrace.Config{
-					DefaultSampler: sdktrace.NeverSample(),
-				}),
+				WithDefaultSampler(sdktrace.NeverSample()),
 			},
 			testSpanSampling:    true,
 			spanShouldBeSampled: false,

diff --git a/sdk/trace/config.go b/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.