From 992837f195eacbd84f0596bfaa79470dba7408e5 Mon Sep 17 00:00:00 2001
From: Tyler Yahn <MrAlias@users.noreply.github.com>
Date: Wed, 3 Mar 2021 20:09:58 +0000
Subject: [PATCH] Add TracerProvider tests to oteltest harness (#1607)

* Add TracerProvider tests to oteltest harness

* Update Tracer method docs

* Fix grammar
---
 internal/matchers/expectation.go |  6 ++++
 oteltest/harness.go              | 51 ++++++++++++++++++++++++++++++++
 sdk/trace/provider.go            |  9 ++++--
 sdk/trace/trace_test.go          | 13 ++++----
 trace/trace.go                   |  2 ++
 5 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/internal/matchers/expectation.go b/internal/matchers/expectation.go
index 806d787fe..625ee017b 100644
--- a/internal/matchers/expectation.go
+++ b/internal/matchers/expectation.go
@@ -83,6 +83,12 @@ func (e *Expectation) ToBeFalse() {
 	}
 }
 
+func (e *Expectation) NotToPanic() {
+	if actual := recover(); actual != nil {
+		e.fail(fmt.Sprintf("Expected panic\n\t%v\nto have not been raised", actual))
+	}
+}
+
 func (e *Expectation) ToSucceed() {
 	switch actual := e.actual.(type) {
 	case error:
diff --git a/oteltest/harness.go b/oteltest/harness.go
index 428094dcd..e5f7c6494 100644
--- a/oteltest/harness.go
+++ b/oteltest/harness.go
@@ -16,6 +16,7 @@ package oteltest // import "go.opentelemetry.io/otel/oteltest"
 
 import (
 	"context"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
@@ -39,6 +40,56 @@ func NewHarness(t *testing.T) *Harness {
 	}
 }
 
+// TestTracer runs validation tests for an implementation of the OpenTelemetry
+// TracerProvider API.
+func (h *Harness) TestTracerProvider(subjectFactory func() trace.TracerProvider) {
+	h.t.Run("#Start", func(t *testing.T) {
+		t.Run("allow creating an arbitrary number of TracerProvider instances", func(t *testing.T) {
+			t.Parallel()
+
+			e := matchers.NewExpecter(t)
+
+			tp1 := subjectFactory()
+			tp2 := subjectFactory()
+
+			e.Expect(tp1).NotToEqual(tp2)
+		})
+		t.Run("all methods are safe to be called concurrently", func(t *testing.T) {
+			t.Parallel()
+
+			runner := func(tp trace.TracerProvider) <-chan struct{} {
+				done := make(chan struct{})
+				go func(tp trace.TracerProvider) {
+					var wg sync.WaitGroup
+					for i := 0; i < 20; i++ {
+						wg.Add(1)
+						go func(name, version string) {
+							_ = tp.Tracer(name, trace.WithInstrumentationVersion(version))
+							wg.Done()
+						}(fmt.Sprintf("tracer %d", i%5), fmt.Sprintf("%d", i))
+					}
+					wg.Wait()
+					done <- struct{}{}
+				}(tp)
+				return done
+			}
+
+			matchers.NewExpecter(t).Expect(func() {
+				// Run with multiple TracerProvider to ensure they encapsulate
+				// their own Tracers.
+				tp1 := subjectFactory()
+				tp2 := subjectFactory()
+
+				done1 := runner(tp1)
+				done2 := runner(tp2)
+
+				<-done1
+				<-done2
+			}).NotToPanic()
+		})
+	})
+}
+
 // TestTracer runs validation tests for an implementation of the OpenTelemetry
 // Tracer API.
 func (h *Harness) TestTracer(subjectFactory func() trace.Tracer) {
diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go
index 053b40560..2810996fa 100644
--- a/sdk/trace/provider.go
+++ b/sdk/trace/provider.go
@@ -85,8 +85,13 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider {
 	return tp
 }
 
-// Tracer with the given name. If a tracer for the given name does not exist,
-// it is created first. If the name is empty, DefaultTracerName is used.
+// Tracer returns a Tracer with the given name and options. If a Tracer for
+// the given name and options does not exist it is created, otherwise the
+// existing Tracer is returned.
+//
+// If name is empty, DefaultTracerName is used instead.
+//
+// This method is safe to be called concurrently.
 func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.Tracer {
 	c := trace.NewTracerConfig(opts...)
 
diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
index bf04c2f2c..93dde2214 100644
--- a/sdk/trace/trace_test.go
+++ b/sdk/trace/trace_test.go
@@ -68,13 +68,16 @@ func init() {
 }
 
 func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) {
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: TraceIDRatioBased(0)}))
 	harness := oteltest.NewHarness(t)
-	subjectFactory := func() trace.Tracer {
-		return tp.Tracer("")
-	}
 
-	harness.TestTracer(subjectFactory)
+	harness.TestTracerProvider(func() trace.TracerProvider {
+		return NewTracerProvider(WithConfig(Config{DefaultSampler: TraceIDRatioBased(0)}))
+	})
+
+	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: TraceIDRatioBased(0)}))
+	harness.TestTracer(func() trace.Tracer {
+		return tp.Tracer("")
+	})
 }
 
 type testExporter struct {
diff --git a/trace/trace.go b/trace/trace.go
index 57ba19432..27f43e6a1 100644
--- a/trace/trace.go
+++ b/trace/trace.go
@@ -558,5 +558,7 @@ type TracerProvider interface {
 	// only if that code provides built-in instrumentation. If the
 	// instrumentationName is empty, then a implementation defined default
 	// name will be used instead.
+	//
+	// This method must be concurrency safe.
 	Tracer(instrumentationName string, opts ...TracerOption) Tracer
 }