Avoid global configuration overriding all (#655)

It will be more common for the users to tweak a few settings rather than all. Rename SetConfig to ApplyConfig and only override the non-zero values.
census-instrumentation · Mar 29, 2018 · 7630269 · 7630269
1 parent 7cc1962
commit 7630269
Show file tree

Hide file tree

Showing 15 changed files with 56 additions and 26 deletions.
diff --git a/examples/http/helloworld_client/main.go b/examples/http/helloworld_client/main.go
@@ -35,7 +35,7 @@ func main() {
 	trace.RegisterExporter(exporter)
 
 	// Always trace for this demo.
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	// Report stats at every second.
 	view.SetReportingPeriod(1 * time.Second)

diff --git a/examples/http/helloworld_server/main.go b/examples/http/helloworld_server/main.go
@@ -37,7 +37,7 @@ func main() {
 	trace.RegisterExporter(exporter)
 
 	// Always trace for this demo.
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	// Report stats at every second.
 	view.SetReportingPeriod(1 * time.Second)

diff --git a/exporter/jaeger/example/main.go b/exporter/jaeger/example/main.go
@@ -39,7 +39,7 @@ func main() {
 	trace.RegisterExporter(exporter)
 
 	// For demoing purposes, always sample.
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	ctx, span := trace.StartSpan(ctx, "/foo")
 	bar(ctx)

diff --git a/exporter/stackdriver/stackdriver_test.go b/exporter/stackdriver/stackdriver_test.go
@@ -47,7 +47,7 @@ func TestExport(t *testing.T) {
 	view.RegisterExporter(exporter)
 	defer view.UnregisterExporter(exporter)
 
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	span := trace.NewSpan("custom-span", nil, trace.StartOptions{})
 	time.Sleep(10 * time.Millisecond)
@@ -100,7 +100,7 @@ func TestGRPC(t *testing.T) {
 	view.RegisterExporter(exporter)
 	defer view.UnregisterExporter(exporter)
 
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	client, done := testpb.NewTestClient(t)
 	defer done()

diff --git a/exporter/stackdriver/trace_test.go b/exporter/stackdriver/trace_test.go
@@ -35,7 +35,7 @@ func TestBundling(t *testing.T) {
 	}
 	trace.RegisterExporter(exporter)
 
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	for i := 0; i < 35; i++ {
 		_, span := trace.StartSpan(context.Background(), "span")
 		span.End()

diff --git a/exporter/zipkin/example/main.go b/exporter/zipkin/example/main.go
@@ -42,7 +42,7 @@ func main() {
 	trace.RegisterExporter(exporter)
 
 	// For example purposes, sample every trace.
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	ctx := context.Background()
 	foo(ctx)

diff --git a/plugin/ocgrpc/trace_test.go b/plugin/ocgrpc/trace_test.go
@@ -33,7 +33,7 @@ func (t *testExporter) ExportSpan(s *trace.SpanData) {
 }
 
 func TestStreaming(t *testing.T) {
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	te := testExporter{make(chan *trace.SpanData)}
 	trace.RegisterExporter(&te)
 	defer trace.UnregisterExporter(&te)
@@ -76,7 +76,7 @@ func TestStreaming(t *testing.T) {
 }
 
 func TestStreamingFail(t *testing.T) {
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	te := testExporter{make(chan *trace.SpanData)}
 	trace.RegisterExporter(&te)
 	defer trace.UnregisterExporter(&te)
@@ -117,7 +117,7 @@ func TestStreamingFail(t *testing.T) {
 }
 
 func TestSingle(t *testing.T) {
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	te := testExporter{make(chan *trace.SpanData)}
 	trace.RegisterExporter(&te)
 	defer trace.UnregisterExporter(&te)
@@ -150,7 +150,7 @@ func TestServerSpanDuration(t *testing.T) {
 	trace.RegisterExporter(&te)
 	defer trace.UnregisterExporter(&te)
 
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	ctx := context.Background()
 	const sleep = 100 * time.Millisecond
@@ -174,7 +174,7 @@ loop:
 }
 
 func TestSingleFail(t *testing.T) {
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	te := testExporter{make(chan *trace.SpanData)}
 	trace.RegisterExporter(&te)
 	defer trace.UnregisterExporter(&te)

diff --git a/plugin/ochttp/propagation_test.go b/plugin/ochttp/propagation_test.go
@@ -36,7 +36,7 @@ func TestRoundTripAllFormats(t *testing.T) {
 	}
 
 	ctx := context.Background()
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	ctx, span := trace.StartSpan(ctx, "test")
 	sc := span.SpanContext()
 	wantStr := fmt.Sprintf("trace_id=%x, span_id=%x, options=%d", sc.TraceID, sc.SpanID, sc.TraceOptions)

diff --git a/plugin/ochttp/trace_test.go b/plugin/ochttp/trace_test.go
@@ -172,7 +172,7 @@ func (c *collector) ExportSpan(s *trace.SpanData) {
 }
 
 func TestEndToEnd(t *testing.T) {
-	trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 	tc := []struct {
 		name            string

diff --git a/trace/benchmark_test.go b/trace/benchmark_test.go
@@ -94,12 +94,12 @@ func BenchmarkSpanID_DotString(b *testing.B) {
 func traceBenchmark(b *testing.B, fn func(*testing.B)) {
 	b.Run("AlwaysSample", func(b *testing.B) {
 		b.ReportAllocs()
-		SetConfig(Config{DefaultSampler: AlwaysSample()})
+		ApplyConfig(Config{DefaultSampler: AlwaysSample()})
 		fn(b)
 	})
 	b.Run("NeverSample", func(b *testing.B) {
 		b.ReportAllocs()
-		SetConfig(Config{DefaultSampler: NeverSample()})
+		ApplyConfig(Config{DefaultSampler: NeverSample()})
 		fn(b)
 	})
 }
diff --git a/trace/config.go b/trace/config.go
@@ -24,12 +24,14 @@ type Config struct {
 	DefaultSampler Sampler
 }
 
-// SetConfig sets the global tracing configuration.
-func SetConfig(cfg Config) {
+// ApplyConfig applies changes to the global tracing configuration.
+//
+// Fields not provided in the given config are going to be preserved.
+func ApplyConfig(cfg Config) {
+	mu.Lock()
 	if cfg.DefaultSampler == nil {
-		cfg.DefaultSampler = newDefaultSampler()
+		cfg.DefaultSampler = config.DefaultSampler
 	}
-	mu.Lock()
 	// TODO(jbd): Reduce the global contention on config.
 	config = cfg
 	mu.Unlock()

diff --git a/trace/config_test.go b/trace/config_test.go
@@ -0,0 +1,28 @@
+// Copyright 2018, OpenCensus Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package trace
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestApplyZeroConfig(t *testing.T) {
+	cfg := config
+	ApplyConfig(Config{})
+	if got, want := reflect.ValueOf(config.DefaultSampler).Pointer(), reflect.ValueOf(cfg.DefaultSampler).Pointer(); got != want {
+		t.Fatalf("config.DefaultSampler = %#v; want %#v", got, want)
+	}
+}
diff --git a/trace/doc.go b/trace/doc.go
@@ -28,10 +28,10 @@ one of the provided exporters or write your own.
     trace.RegisterExporter(anExporter)
 
 By default, traces will be sampled relatively rarely. To change the sampling
-frequency for your entire program, call SetConfig. Use a ProbabilitySampler
+frequency for your entire program, call ApplyConfig. Use a ProbabilitySampler
 to sample a subset of traces, or use AlwaysSample to collect a trace on every run:
 
-    trace.SetConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
+    trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
 
 Adding Spans to a Trace

diff --git a/trace/sampling.go b/trace/sampling.go
@@ -26,7 +26,7 @@ func newDefaultSampler() Sampler {
 
 // SetDefaultSampler sets the default sampler used when creating new spans.
 //
-// Deprecated: Use SetConfig.
+// Deprecated: Use ApplyConfig.
 func SetDefaultSampler(sampler Sampler) {
 }
 

diff --git a/trace/trace_test.go b/trace/trace_test.go
@@ -29,7 +29,7 @@ var (
 
 func init() {
 	// no random sampling, but sample children of sampled spans.
-	SetConfig(Config{DefaultSampler: ProbabilitySampler(0)})
+	ApplyConfig(Config{DefaultSampler: ProbabilitySampler(0)})
 }
 
 func TestStrings(t *testing.T) {
@@ -154,7 +154,7 @@ func TestSampling(t *testing.T) {
 			AlwaysSample(),
 			ProbabilitySampler(0),
 		} {
-			SetConfig(Config{DefaultSampler: defaultSampler})
+			ApplyConfig(Config{DefaultSampler: defaultSampler})
 			sampler := NeverSample()
 			if test.parentTraceOptions == 1 {
 				sampler = AlwaysSample()
@@ -174,7 +174,7 @@ func TestSampling(t *testing.T) {
 			}
 		}
 	}
-	SetConfig(Config{DefaultSampler: ProbabilitySampler(0)}) // reset the default sampler.
+	ApplyConfig(Config{DefaultSampler: ProbabilitySampler(0)}) // reset the default sampler.
 }
 
 func TestProbabilitySampler(t *testing.T) {