From c65c3becb066b82b84e33e781d016ed86015f6f5 Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Thu, 14 Apr 2022 17:22:03 +0200 Subject: [PATCH] Don't import testing package in production builds (#2786) * switch all tests to be on the global package * move ResetForTest to be available only during tests Co-authored-by: Chester Cheung --- CHANGELOG.md | 1 + internal/global/benchmark_test.go | 9 +++---- internal/global/propagator_test.go | 23 +++++++++--------- internal/global/state.go | 12 ---------- internal/global/trace_test.go | 38 ++++++++++++++---------------- internal/global/util_test.go | 31 ++++++++++++++++++++++++ 6 files changed, 64 insertions(+), 50 deletions(-) create mode 100644 internal/global/util_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index dcb939728e8..ebd5a72511d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Delegated instruments are unwrapped before delegating Callbacks. (#2784) - Resolve supply-chain failure for the markdown-link-checker GitHub action by calling the CLI directly. (#2834) +- Remove import of `testing` package in non-tests builds. (#2786) ## [0.29.0] - 2022-04-11 diff --git a/internal/global/benchmark_test.go b/internal/global/benchmark_test.go index f001844642c..5d4d3b9cf8b 100644 --- a/internal/global/benchmark_test.go +++ b/internal/global/benchmark_test.go @@ -12,21 +12,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -package global_test +package global import ( "context" "testing" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/internal/global" ) func BenchmarkStartEndSpanNoSDK(b *testing.B) { // Compare with BenchmarkStartEndSpan() in // ../../sdk/trace/benchmark_test.go. - global.ResetForTest(b) - t := otel.Tracer("Benchmark StartEndSpan") + ResetForTest(b) + t := TracerProvider().Tracer("Benchmark StartEndSpan") ctx := context.Background() b.ResetTimer() for i := 0; i < b.N; i++ { diff --git a/internal/global/propagator_test.go b/internal/global/propagator_test.go index b58ae445691..d13be35c715 100644 --- a/internal/global/propagator_test.go +++ b/internal/global/propagator_test.go @@ -12,23 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -package global_test +package global import ( "context" "testing" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/internal/internaltest" ) func TestTextMapPropagatorDelegation(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) ctx := context.Background() carrier := internaltest.NewTextMapCarrier(nil) // The default should be a noop. - initial := global.TextMapPropagator() + initial := TextMapPropagator() initial.Inject(ctx, carrier) ctx = initial.Extract(ctx, carrier) if !carrier.GotN(t, 0) || !carrier.SetN(t, 0) { @@ -45,7 +44,7 @@ func TestTextMapPropagatorDelegation(t *testing.T) { // The initial propagator should use the delegate after it is set as the // global. - global.SetTextMapPropagator(delegate) + SetTextMapPropagator(delegate) initial.Inject(ctx, carrier) ctx = initial.Extract(ctx, carrier) delegate.InjectedN(t, carrier, 2) @@ -53,12 +52,12 @@ func TestTextMapPropagatorDelegation(t *testing.T) { } func TestTextMapPropagatorDelegationNil(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) ctx := context.Background() carrier := internaltest.NewTextMapCarrier(nil) // The default should be a noop. - initial := global.TextMapPropagator() + initial := TextMapPropagator() initial.Inject(ctx, carrier) ctx = initial.Extract(ctx, carrier) if !carrier.GotN(t, 0) || !carrier.SetN(t, 0) { @@ -66,7 +65,7 @@ func TestTextMapPropagatorDelegationNil(t *testing.T) { } // Delegation to nil should not make a change. - global.SetTextMapPropagator(nil) + SetTextMapPropagator(nil) initial.Inject(ctx, carrier) initial.Extract(ctx, carrier) if !carrier.GotN(t, 0) || !carrier.SetN(t, 0) { @@ -75,8 +74,8 @@ func TestTextMapPropagatorDelegationNil(t *testing.T) { } func TestTextMapPropagatorFields(t *testing.T) { - global.ResetForTest(t) - initial := global.TextMapPropagator() + ResetForTest(t) + initial := TextMapPropagator() delegate := internaltest.NewTextMapPropagator("test") delegateFields := delegate.Fields() @@ -84,13 +83,13 @@ func TestTextMapPropagatorFields(t *testing.T) { if got := initial.Fields(); fieldsEqual(got, delegateFields) { t.Fatalf("testing fields (%v) matched Noop fields (%v)", delegateFields, got) } - global.SetTextMapPropagator(delegate) + SetTextMapPropagator(delegate) // Check previous returns from global not correctly delegate. if got := initial.Fields(); !fieldsEqual(got, delegateFields) { t.Errorf("global TextMapPropagator.Fields returned %v instead of delegating, want (%v)", got, delegateFields) } // Check new calls to global. - if got := global.TextMapPropagator().Fields(); !fieldsEqual(got, delegateFields) { + if got := TextMapPropagator().Fields(); !fieldsEqual(got, delegateFields) { t.Errorf("global TextMapPropagator.Fields returned %v, want (%v)", got, delegateFields) } } diff --git a/internal/global/state.go b/internal/global/state.go index 837d3c6c45e..1ad38f828ec 100644 --- a/internal/global/state.go +++ b/internal/global/state.go @@ -18,7 +18,6 @@ import ( "errors" "sync" "sync/atomic" - "testing" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" @@ -114,14 +113,3 @@ func defaultPropagatorsValue() *atomic.Value { v.Store(propagatorsHolder{tm: newTextMapPropagator()}) return v } - -// ResetForTest configures the test to restores the initial global state during -// its Cleanup step -func ResetForTest(t testing.TB) { - t.Cleanup(func() { - globalTracer = defaultTracerValue() - globalPropagators = defaultPropagatorsValue() - delegateTraceOnce = sync.Once{} - delegateTextMapPropagatorOnce = sync.Once{} - }) -} diff --git a/internal/global/trace_test.go b/internal/global/trace_test.go index fc16ff0e760..f8f9149d9e0 100644 --- a/internal/global/trace_test.go +++ b/internal/global/trace_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package global_test +package global import ( "context" @@ -22,8 +22,6 @@ import ( "github.com/stretchr/testify/assert" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/trace" ) @@ -44,7 +42,7 @@ func (fn fnTracer) Start(ctx context.Context, spanName string, opts ...trace.Spa } func TestTraceProviderDelegation(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) // Map of tracers to expected span names. expected := map[string][]string{ @@ -54,12 +52,12 @@ func TestTraceProviderDelegation(t *testing.T) { } ctx := context.Background() - gtp := otel.GetTracerProvider() + gtp := TracerProvider() tracer1 := gtp.Tracer("pre") // This is started before an SDK was registered and should be dropped. _, span1 := tracer1.Start(ctx, "span1") - otel.SetTracerProvider(fnTracerProvider{ + SetTracerProvider(fnTracerProvider{ tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { spans, ok := expected[name] assert.Truef(t, ok, "invalid tracer: %s", name) @@ -98,14 +96,14 @@ func TestTraceProviderDelegation(t *testing.T) { } func TestTraceProviderDelegates(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) // Retrieve the placeholder TracerProvider. - gtp := otel.GetTracerProvider() + gtp := TracerProvider() // Configure it with a spy. called := false - otel.SetTracerProvider(fnTracerProvider{ + SetTracerProvider(fnTracerProvider{ tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { called = true assert.Equal(t, "abc", name) @@ -118,10 +116,10 @@ func TestTraceProviderDelegates(t *testing.T) { } func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) // Retrieve the placeholder TracerProvider. - gtp := otel.GetTracerProvider() + gtp := TracerProvider() done := make(chan struct{}) quit := make(chan struct{}) @@ -142,7 +140,7 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) { // Configure it with a spy. called := int32(0) - otel.SetTracerProvider(fnTracerProvider{ + SetTracerProvider(fnTracerProvider{ tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { newVal := atomic.AddInt32(&called, 1) assert.Equal(t, "abc", name) @@ -161,10 +159,10 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) { } func TestTracerDelegatesConcurrentSafe(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) // Retrieve the placeholder TracerProvider. - gtp := otel.GetTracerProvider() + gtp := TracerProvider() tracer := gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")) done := make(chan struct{}) @@ -186,7 +184,7 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) { // Configure it with a spy. called := int32(0) - otel.SetTracerProvider(fnTracerProvider{ + SetTracerProvider(fnTracerProvider{ tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { assert.Equal(t, "abc", name) return fnTracer{ @@ -210,15 +208,15 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) { } func TestTraceProviderDelegatesSameInstance(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) // Retrieve the placeholder TracerProvider. - gtp := otel.GetTracerProvider() + gtp := TracerProvider() tracer := gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")) assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"))) assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"))) - otel.SetTracerProvider(fnTracerProvider{ + SetTracerProvider(fnTracerProvider{ tracer: func(name string, opts ...trace.TracerOption) trace.Tracer { return trace.NewNoopTracerProvider().Tracer("") }, @@ -228,7 +226,7 @@ func TestTraceProviderDelegatesSameInstance(t *testing.T) { } func TestSpanContextPropagatedWithNonRecordingSpan(t *testing.T) { - global.ResetForTest(t) + ResetForTest(t) sc := trace.NewSpanContext(trace.SpanContextConfig{ TraceID: [16]byte{0x01}, @@ -237,7 +235,7 @@ func TestSpanContextPropagatedWithNonRecordingSpan(t *testing.T) { Remote: true, }) ctx := trace.ContextWithSpanContext(context.Background(), sc) - _, span := otel.Tracer("test").Start(ctx, "test") + _, span := TracerProvider().Tracer("test").Start(ctx, "test") assert.Equal(t, sc, span.SpanContext()) assert.False(t, span.IsRecording()) diff --git a/internal/global/util_test.go b/internal/global/util_test.go new file mode 100644 index 00000000000..b066a74a20e --- /dev/null +++ b/internal/global/util_test.go @@ -0,0 +1,31 @@ +// Copyright The OpenTelemetry 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 global + +import ( + "sync" + "testing" +) + +// ResetForTest configures the test to restores the initial global state during +// its Cleanup step +func ResetForTest(t testing.TB) { + t.Cleanup(func() { + globalTracer = defaultTracerValue() + globalPropagators = defaultPropagatorsValue() + delegateTraceOnce = sync.Once{} + delegateTextMapPropagatorOnce = sync.Once{} + }) +}