From 4a3adaafd4f373d4da74a1856ad4d2d40c066924 Mon Sep 17 00:00:00 2001
From: Tyler Yahn <MrAlias@users.noreply.github.com>
Date: Wed, 12 Oct 2022 12:54:51 -0700
Subject: [PATCH] Replace meterRegistry with cache (#3255)

---
 sdk/metric/meter.go      | 44 ----------------------------------------
 sdk/metric/meter_test.go | 22 --------------------
 sdk/metric/provider.go   | 17 +++++++---------
 3 files changed, 7 insertions(+), 76 deletions(-)

diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go
index c6871a06716..80007b1465e 100644
--- a/sdk/metric/meter.go
+++ b/sdk/metric/meter.go
@@ -16,7 +16,6 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric"
 
 import (
 	"context"
-	"sync"
 
 	"go.opentelemetry.io/otel/metric"
 	"go.opentelemetry.io/otel/metric/instrument"
@@ -27,49 +26,6 @@ import (
 	"go.opentelemetry.io/otel/sdk/instrumentation"
 )
 
-// meterRegistry keeps a record of initialized meters for instrumentation
-// scopes. A meter is unique to an instrumentation scope and if multiple
-// requests for that meter are made a meterRegistry ensure the same instance
-// is used.
-//
-// The zero meterRegistry is empty and ready for use.
-//
-// A meterRegistry must not be copied after first use.
-//
-// All methods of a meterRegistry are safe to call concurrently.
-type meterRegistry struct {
-	sync.Mutex
-
-	meters map[instrumentation.Scope]*meter
-
-	pipes pipelines
-}
-
-// Get returns a registered meter matching the instrumentation scope if it
-// exists in the meterRegistry. Otherwise, a new meter configured for the
-// instrumentation scope is registered and then returned.
-//
-// Get is safe to call concurrently.
-func (r *meterRegistry) Get(s instrumentation.Scope) *meter {
-	r.Lock()
-	defer r.Unlock()
-
-	if r.meters == nil {
-		m := newMeter(s, r.pipes)
-		r.meters = map[instrumentation.Scope]*meter{s: m}
-		return m
-	}
-
-	m, ok := r.meters[s]
-	if ok {
-		return m
-	}
-
-	m = newMeter(s, r.pipes)
-	r.meters[s] = m
-	return m
-}
-
 // meter handles the creation and coordination of all metric instruments. A
 // meter represents a single instrumentation scope; all metric telemetry
 // produced by an instrumentation scope will use metric instruments from a
diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go
index a67aa507d06..8f6448f757e 100644
--- a/sdk/metric/meter_test.go
+++ b/sdk/metric/meter_test.go
@@ -31,28 +31,6 @@ import (
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
-func TestMeterRegistry(t *testing.T) {
-	is0 := instrumentation.Scope{Name: "zero"}
-	is1 := instrumentation.Scope{Name: "one"}
-
-	r := meterRegistry{}
-	var m0 *meter
-	t.Run("ZeroValueGetDoesNotPanic", func(t *testing.T) {
-		assert.NotPanics(t, func() { m0 = r.Get(is0) })
-		assert.Equal(t, is0, m0.Scope, "uninitialized meter returned")
-	})
-
-	m01 := r.Get(is0)
-	t.Run("GetSameMeter", func(t *testing.T) {
-		assert.Samef(t, m0, m01, "returned different meters: %v", is0)
-	})
-
-	m1 := r.Get(is1)
-	t.Run("GetDifferentMeter", func(t *testing.T) {
-		assert.NotSamef(t, m0, m1, "returned same meters: %v", is1)
-	})
-}
-
 // A meter should be able to make instruments concurrently.
 func TestMeterInstrumentConcurrency(t *testing.T) {
 	wg := &sync.WaitGroup{}
diff --git a/sdk/metric/provider.go b/sdk/metric/provider.go
index db890d1060b..ce2e5524398 100644
--- a/sdk/metric/provider.go
+++ b/sdk/metric/provider.go
@@ -26,7 +26,8 @@ import (
 // the same Views applied to them, and have their produced metric telemetry
 // passed to the configured Readers.
 type MeterProvider struct {
-	meters meterRegistry
+	pipes  pipelines
+	meters cache[instrumentation.Scope, *meter]
 
 	forceFlush, shutdown func(context.Context) error
 }
@@ -42,16 +43,9 @@ var _ metric.MeterProvider = (*MeterProvider)(nil)
 // Readers, will perform no operations.
 func NewMeterProvider(options ...Option) *MeterProvider {
 	conf := newConfig(options)
-
 	flush, sdown := conf.readerSignals()
-
-	registry := newPipelines(conf.res, conf.readers)
-
 	return &MeterProvider{
-		meters: meterRegistry{
-			pipes: registry,
-		},
-
+		pipes:      newPipelines(conf.res, conf.readers),
 		forceFlush: flush,
 		shutdown:   sdown,
 	}
@@ -72,10 +66,13 @@ func NewMeterProvider(options ...Option) *MeterProvider {
 // This method is safe to call concurrently.
 func (mp *MeterProvider) Meter(name string, options ...metric.MeterOption) metric.Meter {
 	c := metric.NewMeterConfig(options...)
-	return mp.meters.Get(instrumentation.Scope{
+	s := instrumentation.Scope{
 		Name:      name,
 		Version:   c.InstrumentationVersion(),
 		SchemaURL: c.SchemaURL(),
+	}
+	return mp.meters.Lookup(s, func() *meter {
+		return newMeter(s, mp.pipes)
 	})
 }