Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid overriding configuration of tracer provider #1633

Merged
merged 4 commits into from
Mar 8, 2021
Merged

Avoid overriding configuration of tracer provider #1633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
549393e
sdk/trace: add missing options to tracer provider
ijsong Mar 3, 2021
a2deff6
Update sdk/trace/provider.go
ijsong Mar 3, 2021
0e2f7a9
Update sdk/trace/provider.go
ijsong Mar 3, 2021
4117ec1
rebase and remove WithConfig
ijsong Mar 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it is the wrong place to put this changelog.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to not have checked carefully.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is fine. This should be handled by an automatic tool, which lowers everyone's mental burden. ;)


### Changed

Expand Down Expand Up @@ -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

Expand Down
6 changes: 1 addition & 5 deletions example/namedtracer/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion example/otel-collector/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/internal/otlptest/otlptest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/otlpgrpc/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlpgrpc/otlp_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion exporters/trace/jaeger/jaeger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion exporters/trace/jaeger/jaeger_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion sdk/trace/benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
21 changes: 14 additions & 7 deletions sdk/trace/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
49 changes: 26 additions & 23 deletions sdk/trace/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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("")
})
Expand Down Expand Up @@ -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++ {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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{
Expand Down
4 changes: 1 addition & 3 deletions sdk/trace/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}