Repository: open-telemetry/opentelemetry-go
Pull request #1607 (2021-02-26 00:46:15)

Add TracerProvider tests to oteltest harness
Related to https://github.com/open-telemetry/opentelemetry-go/issues/1587

Ensure our `TracerProvider.Tracer` method is safe to call concurrently and that multiple `TracerProviders` can be created as required by the specification.

codecov[bot]: # [Codecov](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607?src=pr&el=h1) Report
> Merging [#1607](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607?src=pr&el=desc) (e4fdd93) into [main](https://codecov.io/gh/open-telemetry/opentelemetry-go/commit/37688ef67616aa5a99e27fb03320fd0dc87e590e?el=desc) (37688ef) will **decrease** coverage by `0.1%`.
> The diff coverage is `0.0%`.

[![Impacted file tree graph](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/graphs/tree.svg?width=650&height=150&src=pr&token=8efTmh4kvf)](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##            main   #1607     +/-   ##
=======================================
- Coverage   77.6%   77.5%   -0.2%     
=======================================
  Files        128     128             
  Lines       6635    6647     +12     
=======================================
  Hits        5155    5155             
- Misses      1233    1245     +12     
  Partials     247     247             
```


| [Impacted Files](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/matchers/expectation.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbWF0Y2hlcnMvZXhwZWN0YXRpb24uZ28=) | `0.0% <0.0%> (ø)` | |
| [oteltest/harness.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-b3RlbHRlc3QvaGFybmVzcy5nbw==) | `82.7% <0.0%> (-17.3%)` | :arrow_down: |
| [trace/trace.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-dHJhY2UvdHJhY2UuZ28=) | `86.6% <ø> (ø)` | |
| [sdk/export/metric/exportkind\_string.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-c2RrL2V4cG9ydC9tZXRyaWMvZXhwb3J0a2luZF9zdHJpbmcuZ28=) | `0.0% <0.0%> (ø)` | |
| [metric/number/kind\_string.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-bWV0cmljL251bWJlci9raW5kX3N0cmluZy5nbw==) | `33.3% <0.0%> (+16.6%)` | :arrow_up: |
| [attribute/type\_string.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-YXR0cmlidXRlL3R5cGVfc3RyaW5nLmdv) | `33.3% <0.0%> (+23.3%)` | :arrow_up: |
| [metric/instrumentkind\_string.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1607/diff?src=pr&el=tree#diff-bWV0cmljL2luc3RydW1lbnRraW5kX3N0cmluZy5nbw==) | `33.3% <0.0%> (+23.3%)` | :arrow_up: |


Patch:
diff --git a/internal/matchers/expectation.go b/internal/matchers/expectation.go
index 806d787f..625ee017 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/sdk/trace/provider.go b/sdk/trace/provider.go
index 053b4056..2810996f 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/trace/trace.go b/trace/trace.go
index 57ba1943..27f43e6a 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
 }


Test patch:
diff --git a/oteltest/harness.go b/oteltest/harness.go
index 428094dc..e5f7c649 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/trace_test.go b/sdk/trace/trace_test.go
index bf04c2f2..93dde221 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 {