Remove bound metric instruments from the API (new champion) (#2399)

* Remove bound metric instruments from the API

* Removed dead code

* Added changelog message

* Added PR number

Co-authored-by: Joshua MacDonald <jmacd@lightstep.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

CHANGELOG.md

@@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Removed
 
 - Remove the metric Processor's ability to convert cumulative to delta aggregation temporality. (#2350)
+- Remove the metric Bound Instruments interface and implementations. (#2399)
 
 ## [1.2.0] - 2021-11-12
 

internal/metric/global/meter.go

@@ -41,10 +41,6 @@ import (
 // in the MeterProvider and Meters ensure that each instrument has a delegate
 // before the global provider is set.
 //
-// Bound instrument operations are implemented by delegating to the
-// instrument after it is registered, with a sync.Once initializer to
-// protect against races with Release().
-//
 // Metric uniqueness checking is implemented by calling the exported
 // methods of the api/metric/registry package.
 
@@ -108,19 +104,9 @@ type AsyncImpler interface {
 	AsyncImpl() sdkapi.AsyncImpl
 }
 
-type syncHandle struct {
-	delegate unsafe.Pointer // (*sdkapi.BoundInstrumentImpl)
-
-	inst   *syncImpl
-	labels []attribute.KeyValue
-
-	initialize sync.Once
-}
-
 var _ metric.MeterProvider = &meterProvider{}
 var _ sdkapi.MeterImpl = &meterImpl{}
 var _ sdkapi.InstrumentImpl = &syncImpl{}
-var _ sdkapi.BoundSyncImpl = &syncHandle{}
 var _ sdkapi.AsyncImpl = &asyncImpl{}
 
 func (inst *instrument) Descriptor() sdkapi.Descriptor {
@@ -241,28 +227,6 @@ func (inst *syncImpl) Implementation() interface{} {
 	return inst
 }
 
-func (inst *syncImpl) Bind(labels []attribute.KeyValue) sdkapi.BoundSyncImpl {
-	if implPtr := (*sdkapi.SyncImpl)(atomic.LoadPointer(&inst.delegate)); implPtr != nil {
-		return (*implPtr).Bind(labels)
-	}
-	return &syncHandle{
-		inst:   inst,
-		labels: labels,
-	}
-}
-
-func (bound *syncHandle) Unbind() {
-	bound.initialize.Do(func() {})
-
-	implPtr := (*sdkapi.BoundSyncImpl)(atomic.LoadPointer(&bound.delegate))
-
-	if implPtr == nil {
-		return
-	}
-
-	(*implPtr).Unbind()
-}
-
 // Async delegation
 
 func (m *meterImpl) NewAsyncInstrument(
@@ -325,37 +289,11 @@ func (inst *syncImpl) RecordOne(ctx context.Context, number number.Number, label
 	}
 }
 
-// Bound instrument initialization
-
-func (bound *syncHandle) RecordOne(ctx context.Context, number number.Number) {
-	instPtr := (*sdkapi.SyncImpl)(atomic.LoadPointer(&bound.inst.delegate))
-	if instPtr == nil {
-		return
-	}
-	var implPtr *sdkapi.BoundSyncImpl
-	bound.initialize.Do(func() {
-		implPtr = new(sdkapi.BoundSyncImpl)
-		*implPtr = (*instPtr).Bind(bound.labels)
-		atomic.StorePointer(&bound.delegate, unsafe.Pointer(implPtr))
-	})
-	if implPtr == nil {
-		implPtr = (*sdkapi.BoundSyncImpl)(atomic.LoadPointer(&bound.delegate))
-	}
-	// This may still be nil if instrument was created and bound
-	// without a delegate, then the instrument was set to have a
-	// delegate and unbound.
-	if implPtr == nil {
-		return
-	}
-	(*implPtr).RecordOne(ctx, number)
-}
-
 func AtomicFieldOffsets() map[string]uintptr {
 	return map[string]uintptr{
 		"meterProvider.delegate": unsafe.Offsetof(meterProvider{}.delegate),
 		"meterImpl.delegate":     unsafe.Offsetof(meterImpl{}.delegate),
 		"syncImpl.delegate":      unsafe.Offsetof(syncImpl{}.delegate),
 		"asyncImpl.delegate":     unsafe.Offsetof(asyncImpl{}.delegate),
-		"syncHandle.delegate":    unsafe.Offsetof(syncHandle{}.delegate),
 	}
 }

internal/metric/global/meter_test.go

@@ -137,97 +137,6 @@ func TestDirect(t *testing.T) {
 	)
 }
 
-func TestBound(t *testing.T) {
-	global.ResetForTest()
-
-	// Note: this test uses opposite Float64/Int64 number kinds
-	// vs. the above, to cover all the instruments.
-	ctx := context.Background()
-	glob := metricglobal.Meter(
-		"test",
-		metric.WithInstrumentationVersion("semver:test-1.0"),
-		metric.WithSchemaURL("schema://url"),
-	)
-	labels1 := []attribute.KeyValue{attribute.String("A", "B")}
-
-	counter := Must(glob).NewFloat64Counter("test.counter")
-	boundC := counter.Bind(labels1...)
-	boundC.Add(ctx, 1)
-	boundC.Add(ctx, 1)
-
-	histogram := Must(glob).NewInt64Histogram("test.histogram")
-	boundM := histogram.Bind(labels1...)
-	boundM.Record(ctx, 1)
-	boundM.Record(ctx, 2)
-
-	provider := metrictest.NewMeterProvider()
-	metricglobal.SetMeterProvider(provider)
-
-	boundC.Add(ctx, 1)
-	boundM.Record(ctx, 3)
-
-	library := metrictest.Library{
-		InstrumentationName:    "test",
-		InstrumentationVersion: "semver:test-1.0",
-		SchemaURL:              "schema://url",
-	}
-
-	require.EqualValues(t,
-		[]metrictest.Measured{
-			{
-				Name:    "test.counter",
-				Library: library,
-				Labels:  metrictest.LabelsToMap(labels1...),
-				Number:  asFloat(1),
-			},
-			{
-				Name:    "test.histogram",
-				Library: library,
-				Labels:  metrictest.LabelsToMap(labels1...),
-				Number:  asInt(3),
-			},
-		},
-		metrictest.AsStructs(provider.MeasurementBatches))
-
-	boundC.Unbind()
-	boundM.Unbind()
-}
-
-func TestUnbind(t *testing.T) {
-	// Tests Unbind with SDK never installed.
-	global.ResetForTest()
-
-	glob := metricglobal.Meter("test")
-	labels1 := []attribute.KeyValue{attribute.String("A", "B")}
-
-	counter := Must(glob).NewFloat64Counter("test.counter")
-	boundC := counter.Bind(labels1...)
-
-	histogram := Must(glob).NewInt64Histogram("test.histogram")
-	boundM := histogram.Bind(labels1...)
-
-	boundC.Unbind()
-	boundM.Unbind()
-}
-
-func TestUnbindThenRecordOne(t *testing.T) {
-	global.ResetForTest()
-
-	ctx := context.Background()
-	provider := metrictest.NewMeterProvider()
-
-	meter := metricglobal.Meter("test")
-	counter := Must(meter).NewInt64Counter("test.counter")
-	boundC := counter.Bind()
-	metricglobal.SetMeterProvider(provider)
-	boundC.Unbind()
-
-	require.NotPanics(t, func() {
-		boundC.Add(ctx, 1)
-	})
-	require.Equal(t, 0, len(provider.MeasurementBatches))
-}
-
 type meterProviderWithConstructorError struct {
 	metric.MeterProvider
 }