From 763026905f7e0a0510a6a7ad98593792995bf9d4 Mon Sep 17 00:00:00 2001 From: JBD Date: Wed, 28 Mar 2018 17:24:56 -0700 Subject: [PATCH] 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. --- examples/http/helloworld_client/main.go | 2 +- examples/http/helloworld_server/main.go | 2 +- exporter/jaeger/example/main.go | 2 +- exporter/stackdriver/stackdriver_test.go | 4 ++-- exporter/stackdriver/trace_test.go | 2 +- exporter/zipkin/example/main.go | 2 +- plugin/ocgrpc/trace_test.go | 10 ++++----- plugin/ochttp/propagation_test.go | 2 +- plugin/ochttp/trace_test.go | 2 +- trace/benchmark_test.go | 4 ++-- trace/config.go | 10 +++++---- trace/config_test.go | 28 ++++++++++++++++++++++++ trace/doc.go | 4 ++-- trace/sampling.go | 2 +- trace/trace_test.go | 6 ++--- 15 files changed, 56 insertions(+), 26 deletions(-) create mode 100644 trace/config_test.go diff --git a/examples/http/helloworld_client/main.go b/examples/http/helloworld_client/main.go index 362c126cd..3b3186b52 100644 --- 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 index 592a7e769..799f362cf 100644 --- 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 index 4dcfd1297..1caec456e 100644 --- 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 index 29db01b22..5b5020ae7 100644 --- 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 index 4a20a28a4..c8a30b588 100644 --- 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 index b0da404d2..24ec878f7 100644 --- 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 index 7a7ef1174..7c2243db7 100644 --- 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 index 0fcd3d918..f4645db39 100644 --- 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 index 12d65e0ef..5db9652fa 100644 --- 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 index e7e5a6ff6..7e86d64d9 100644 --- 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 index c1c2dc962..52dd7bbff 100644 --- 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 new file mode 100644 index 000000000..72233a160 --- /dev/null +++ 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 index ea5693c3a..2501271ef 100644 --- 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 index 97bf9e62c..0dde31957 100644 --- 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 index a68c5b1fd..2e6db2801 100644 --- 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) {