#872 Make metric test helpers public (#1040)

* Make metric test helpers public

* Move everything metric test related to api/metric/metrictest

* Unify metric measurement assertions

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

api/global/internal/meter_test.go

@@ -24,45 +24,12 @@ import (
 	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/api/global/internal"
 	"go.opentelemetry.io/otel/api/metric"
-	metrictest "go.opentelemetry.io/otel/internal/metric"
+	metrictest "go.opentelemetry.io/otel/api/metric/metrictest"
 	"go.opentelemetry.io/otel/label"
 )
 
 var Must = metric.Must
 
-// Note: Maybe this should be factored into ../../../internal/metric?
-type measured struct {
-	Name                   string
-	InstrumentationName    string
-	InstrumentationVersion string
-	Labels                 map[label.Key]label.Value
-	Number                 metric.Number
-}
-
-func asStructs(batches []metrictest.Batch) []measured {
-	var r []measured
-	for _, batch := range batches {
-		for _, m := range batch.Measurements {
-			r = append(r, measured{
-				Name:                   m.Instrument.Descriptor().Name(),
-				InstrumentationName:    m.Instrument.Descriptor().InstrumentationName(),
-				InstrumentationVersion: m.Instrument.Descriptor().InstrumentationVersion(),
-				Labels:                 asMap(batch.Labels...),
-				Number:                 m.Number,
-			})
-		}
-	}
-	return r
-}
-
-func asMap(kvs ...label.KeyValue) map[label.Key]label.Value {
-	m := map[label.Key]label.Value{}
-	for _, label := range kvs {
-		m[label.Key] = label.Value
-	}
-	return m
-}
-
 var asInt = metric.NewInt64Number
 var asFloat = metric.NewFloat64Number
 
@@ -107,56 +74,56 @@ func TestDirect(t *testing.T) {
 
 	mock.RunAsyncInstruments()
 
-	measurements := asStructs(mock.MeasurementBatches)
+	measurements := metrictest.AsStructs(mock.MeasurementBatches)
 
 	require.EqualValues(t,
-		[]measured{
+		[]metrictest.Measured{
 			{
 				Name:                   "test.counter",
 				InstrumentationName:    "test1",
 				InstrumentationVersion: "semver:v1.0.0",
-				Labels:                 asMap(labels1...),
+				Labels:                 metrictest.LabelsToMap(labels1...),
 				Number:                 asInt(1),
 			},
 			{
 				Name:                   "test.valuerecorder",
 				InstrumentationName:    "test1",
 				InstrumentationVersion: "semver:v1.0.0",
-				Labels:                 asMap(labels1...),
+				Labels:                 metrictest.LabelsToMap(labels1...),
 				Number:                 asFloat(3),
 			},
 			{
 				Name:                "test.second",
 				InstrumentationName: "test2",
-				Labels:              asMap(labels3...),
+				Labels:              metrictest.LabelsToMap(labels3...),
 				Number:              asFloat(3),
 			},
 			{
 				Name:                   "test.valueobserver.float",
 				InstrumentationName:    "test1",
 				InstrumentationVersion: "semver:v1.0.0",
-				Labels:                 asMap(labels1...),
+				Labels:                 metrictest.LabelsToMap(labels1...),
 				Number:                 asFloat(1),
 			},
 			{
 				Name:                   "test.valueobserver.float",
 				InstrumentationName:    "test1",
 				InstrumentationVersion: "semver:v1.0.0",
-				Labels:                 asMap(labels2...),
+				Labels:                 metrictest.LabelsToMap(labels2...),
 				Number:                 asFloat(2),
 			},
 			{
 				Name:                   "test.valueobserver.int",
 				InstrumentationName:    "test1",
 				InstrumentationVersion: "semver:v1.0.0",
-				Labels:                 asMap(labels1...),
+				Labels:                 metrictest.LabelsToMap(labels1...),
 				Number:                 asInt(1),
 			},
 			{
 				Name:                   "test.valueobserver.int",
 				InstrumentationName:    "test1",
 				InstrumentationVersion: "semver:v1.0.0",
-				Labels:                 asMap(labels2...),
+				Labels:                 metrictest.LabelsToMap(labels2...),
 				Number:                 asInt(2),
 			},
 		},
@@ -190,21 +157,21 @@ func TestBound(t *testing.T) {
 	boundM.Record(ctx, 3)
 
 	require.EqualValues(t,
-		[]measured{
+		[]metrictest.Measured{
 			{
 				Name:                "test.counter",
 				InstrumentationName: "test",
-				Labels:              asMap(labels1...),
+				Labels:              metrictest.LabelsToMap(labels1...),
 				Number:              asFloat(1),
 			},
 			{
 				Name:                "test.valuerecorder",
 				InstrumentationName: "test",
-				Labels:              asMap(labels1...),
+				Labels:              metrictest.LabelsToMap(labels1...),
 				Number:              asInt(3),
 			},
 		},
-		asStructs(mock.MeasurementBatches))
+		metrictest.AsStructs(mock.MeasurementBatches))
 
 	boundC.Unbind()
 	boundM.Unbind()
@@ -347,13 +314,13 @@ func TestRecordBatchMock(t *testing.T) {
 	meter.RecordBatch(context.Background(), nil, counter.Measurement(1))
 
 	require.EqualValues(t,
-		[]measured{
+		[]metrictest.Measured{
 			{
 				Name:                "test.counter",
 				InstrumentationName: "builtin",
-				Labels:              asMap(),
+				Labels:              metrictest.LabelsToMap(),
 				Number:              asInt(1),
 			},
 		},
-		asStructs(mock.MeasurementBatches))
+		metrictest.AsStructs(mock.MeasurementBatches))
 }

api/metric/api_test.go

@@ -17,12 +17,12 @@ package metric_test
 import (
 	"context"
 	"errors"
-	"fmt"
 	"testing"
 
 	"go.opentelemetry.io/otel/api/metric"
+	"go.opentelemetry.io/otel/api/metric/metrictest"
+	mockTest "go.opentelemetry.io/otel/api/metric/metrictest"
 	"go.opentelemetry.io/otel/api/unit"
-	mockTest "go.opentelemetry.io/otel/internal/metric"
 	"go.opentelemetry.io/otel/label"
 
 	"github.com/google/go-cmp/cmp"
@@ -32,6 +32,33 @@ import (
 
 var Must = metric.Must
 
+func checkSyncBatches(ctx context.Context, t *testing.T, labels []label.KeyValue, mock *mockTest.MeterImpl, nkind metric.NumberKind, mkind metric.Kind, instrument metric.InstrumentImpl, expected ...float64) {
+	t.Helper()
+
+	batchesCount := len(mock.MeasurementBatches)
+	if len(mock.MeasurementBatches) != len(expected) {
+		t.Errorf("Expected %d recorded measurement batches, got %d", batchesCount, len(mock.MeasurementBatches))
+	}
+	recorded := metrictest.AsStructs(mock.MeasurementBatches)
+
+	for i, batch := range mock.MeasurementBatches {
+		if len(batch.Measurements) != 1 {
+			t.Errorf("Expected 1 measurement in batch %d, got %d", i, len(batch.Measurements))
+		}
+
+		measurement := batch.Measurements[0]
+		descriptor := measurement.Instrument.Descriptor()
+
+		expected := metrictest.Measured{
+			Name:                descriptor.Name(),
+			InstrumentationName: descriptor.InstrumentationName(),
+			Labels:              metrictest.LabelsToMap(labels...),
+			Number:              metrictest.ResolveNumberByKind(t, nkind, expected[i]),
+		}
+		require.Equal(t, expected, recorded[i])
+	}
+}
+
 func TestOptions(t *testing.T) {
 	type testcase struct {
 		name string
@@ -104,7 +131,7 @@ func TestCounter(t *testing.T) {
 		boundInstrument := c.Bind(labels...)
 		boundInstrument.Add(ctx, -742)
 		meter.RecordBatch(ctx, labels, c.Measurement(42))
-		checkSyncBatches(t, ctx, labels, mockSDK, metric.Float64NumberKind, metric.CounterKind, c.SyncImpl(),
+		checkSyncBatches(ctx, t, labels, mockSDK, metric.Float64NumberKind, metric.CounterKind, c.SyncImpl(),
 			1994.1, -742, 42,
 		)
 	})
@@ -117,7 +144,7 @@ func TestCounter(t *testing.T) {
 		boundInstrument := c.Bind(labels...)
 		boundInstrument.Add(ctx, 4200)
 		meter.RecordBatch(ctx, labels, c.Measurement(420000))
-		checkSyncBatches(t, ctx, labels, mockSDK, metric.Int64NumberKind, metric.CounterKind, c.SyncImpl(),
+		checkSyncBatches(ctx, t, labels, mockSDK, metric.Int64NumberKind, metric.CounterKind, c.SyncImpl(),
 			42, 4200, 420000,
 		)
 
@@ -131,7 +158,7 @@ func TestCounter(t *testing.T) {
 		boundInstrument := c.Bind(labels...)
 		boundInstrument.Add(ctx, -100)
 		meter.RecordBatch(ctx, labels, c.Measurement(42))
-		checkSyncBatches(t, ctx, labels, mockSDK, metric.Int64NumberKind, metric.UpDownCounterKind, c.SyncImpl(),
+		checkSyncBatches(ctx, t, labels, mockSDK, metric.Int64NumberKind, metric.UpDownCounterKind, c.SyncImpl(),
 			100, -100, 42,
 		)
 	})
@@ -144,7 +171,7 @@ func TestCounter(t *testing.T) {
 		boundInstrument := c.Bind(labels...)
 		boundInstrument.Add(ctx, -76)
 		meter.RecordBatch(ctx, labels, c.Measurement(-100.1))
-		checkSyncBatches(t, ctx, labels, mockSDK, metric.Float64NumberKind, metric.UpDownCounterKind, c.SyncImpl(),
+		checkSyncBatches(ctx, t, labels, mockSDK, metric.Float64NumberKind, metric.UpDownCounterKind, c.SyncImpl(),
 			100.1, -76, -100.1,
 		)
 	})
@@ -160,7 +187,7 @@ func TestValueRecorder(t *testing.T) {
 		boundInstrument := m.Bind(labels...)
 		boundInstrument.Record(ctx, 0)
 		meter.RecordBatch(ctx, labels, m.Measurement(-100.5))
-		checkSyncBatches(t, ctx, labels, mockSDK, metric.Float64NumberKind, metric.ValueRecorderKind, m.SyncImpl(),
+		checkSyncBatches(ctx, t, labels, mockSDK, metric.Float64NumberKind, metric.ValueRecorderKind, m.SyncImpl(),
 			42, 0, -100.5,
 		)
 	})
@@ -173,7 +200,7 @@ func TestValueRecorder(t *testing.T) {
 		boundInstrument := m.Bind(labels...)
 		boundInstrument.Record(ctx, 80)
 		meter.RecordBatch(ctx, labels, m.Measurement(0))
-		checkSyncBatches(t, ctx, labels, mockSDK, metric.Int64NumberKind, metric.ValueRecorderKind, m.SyncImpl(),
+		checkSyncBatches(ctx, t, labels, mockSDK, metric.Int64NumberKind, metric.ValueRecorderKind, m.SyncImpl(),
 			173, 80, 0,
 		)
 	})
@@ -248,48 +275,6 @@ func TestObserverInstruments(t *testing.T) {
 	})
 }
 
-func checkSyncBatches(t *testing.T, ctx context.Context, labels []label.KeyValue, mock *mockTest.MeterImpl, nkind metric.NumberKind, mkind metric.Kind, instrument metric.InstrumentImpl, expected ...float64) {
-	t.Helper()
-	if len(mock.MeasurementBatches) != 3 {
-		t.Errorf("Expected 3 recorded measurement batches, got %d", len(mock.MeasurementBatches))
-	}
-	ourInstrument := instrument.Implementation().(*mockTest.Sync)
-	for i, got := range mock.MeasurementBatches {
-		if got.Ctx != ctx {
-			d := func(c context.Context) string {
-				return fmt.Sprintf("(ptr: %p, ctx %#v)", c, c)
-			}
-			t.Errorf("Wrong recorded context in batch %d, expected %s, got %s", i, d(ctx), d(got.Ctx))
-		}
-		if !assert.Equal(t, got.Labels, labels) {
-			t.Errorf("Wrong recorded label set in batch %d, expected %v, got %v", i, labels, got.Labels)
-		}
-		if len(got.Measurements) != 1 {
-			t.Errorf("Expected 1 measurement in batch %d, got %d", i, len(got.Measurements))
-		}
-		minMLen := 1
-		if minMLen > len(got.Measurements) {
-			minMLen = len(got.Measurements)
-		}
-		for j := 0; j < minMLen; j++ {
-			measurement := got.Measurements[j]
-			require.Equal(t, mkind, measurement.Instrument.Descriptor().MetricKind())
-
-			if measurement.Instrument.Implementation() != ourInstrument {
-				d := func(iface interface{}) string {
-					i := iface.(*mockTest.Instrument)
-					return fmt.Sprintf("(ptr: %p, instrument %#v)", i, i)
-				}
-				t.Errorf("Wrong recorded instrument in measurement %d in batch %d, expected %s, got %s", j, i, d(ourInstrument), d(measurement.Instrument.Implementation()))
-			}
-			expect := number(t, nkind, expected[i])
-			if measurement.Number.CompareNumber(nkind, expect) != 0 {
-				t.Errorf("Wrong recorded value in measurement %d in batch %d, expected %s, got %s", j, i, expect.Emit(nkind), measurement.Number.Emit(nkind))
-			}
-		}
-	}
-}
-
 func TestBatchObserverInstruments(t *testing.T) {
 	mockSDK, meter := mockTest.NewMeter()
 
@@ -328,11 +313,11 @@ func TestBatchObserverInstruments(t *testing.T) {
 
 	m1 := got.Measurements[0]
 	require.Equal(t, impl1, m1.Instrument.Implementation().(*mockTest.Async))
-	require.Equal(t, 0, m1.Number.CompareNumber(metric.Int64NumberKind, number(t, metric.Int64NumberKind, 42)))
+	require.Equal(t, 0, m1.Number.CompareNumber(metric.Int64NumberKind, mockTest.ResolveNumberByKind(t, metric.Int64NumberKind, 42)))
 
 	m2 := got.Measurements[1]
 	require.Equal(t, impl2, m2.Instrument.Implementation().(*mockTest.Async))
-	require.Equal(t, 0, m2.Number.CompareNumber(metric.Float64NumberKind, number(t, metric.Float64NumberKind, 42)))
+	require.Equal(t, 0, m2.Number.CompareNumber(metric.Float64NumberKind, mockTest.ResolveNumberByKind(t, metric.Float64NumberKind, 42)))
 }
 
 func checkObserverBatch(t *testing.T, labels []label.KeyValue, mock *mockTest.MeterImpl, nkind metric.NumberKind, mkind metric.Kind, observer metric.AsyncImpl, expected float64) {
@@ -354,21 +339,10 @@ func checkObserverBatch(t *testing.T, labels []label.KeyValue, mock *mockTest.Me
 	measurement := got.Measurements[0]
 	require.Equal(t, mkind, measurement.Instrument.Descriptor().MetricKind())
 	assert.Equal(t, o, measurement.Instrument.Implementation().(*mockTest.Async))
-	ft := number(t, nkind, expected)
+	ft := mockTest.ResolveNumberByKind(t, nkind, expected)
 	assert.Equal(t, 0, measurement.Number.CompareNumber(nkind, ft))
 }
 
-func number(t *testing.T, kind metric.NumberKind, value float64) metric.Number {
-	t.Helper()
-	switch kind {
-	case metric.Int64NumberKind:
-		return metric.NewInt64Number(int64(value))
-	case metric.Float64NumberKind:
-		return metric.NewFloat64Number(value)
-	}
-	panic("invalid number kind")
-}
-
 type testWrappedMeter struct {
 }
 

api/metric/metrictest/async.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package metric
+package metrictest
 
 import (
 	"context"

api/metric/metrictest/meter.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package metric
+package metrictest
 
 import (
 	"context"
@@ -38,6 +38,7 @@ type (
 		LibraryName  string
 	}
 
+	// MeterImpl is an OpenTelemetry Meter implementation used for testing.
 	MeterImpl struct {
 		lock sync.Mutex
 

api/metric/metrictest/mock_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package metric
+package metrictest
 
 import (
 	"os"

api/metric/metrictest/test_helpers.go

@@ -0,0 +1,70 @@
+// 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 metrictest
+
+import (
+	"testing"
+
+	"go.opentelemetry.io/otel/api/metric"
+	"go.opentelemetry.io/otel/label"
+)
+
+// Measured is the helper struct which provides flat representation of recorded measurements
+// to simplify testing
+type Measured struct {
+	Name                   string
+	InstrumentationName    string
+	InstrumentationVersion string
+	Labels                 map[label.Key]label.Value
+	Number                 metric.Number
+}
+
+// LabelsToMap converts label set to keyValue map, to be easily used in tests
+func LabelsToMap(kvs ...label.KeyValue) map[label.Key]label.Value {
+	m := map[label.Key]label.Value{}
+	for _, label := range kvs {
+		m[label.Key] = label.Value
+	}
+	return m
+}
+
+// AsStructs converts recorded batches to array of flat, readable Measured helper structures
+func AsStructs(batches []Batch) []Measured {
+	var r []Measured
+	for _, batch := range batches {
+		for _, m := range batch.Measurements {
+			r = append(r, Measured{
+				Name:                   m.Instrument.Descriptor().Name(),
+				InstrumentationName:    m.Instrument.Descriptor().InstrumentationName(),
+				InstrumentationVersion: m.Instrument.Descriptor().InstrumentationVersion(),
+				Labels:                 LabelsToMap(batch.Labels...),
+				Number:                 m.Number,
+			})
+		}
+	}
+	return r
+}
+
+// ResolveNumberByKind takes defined metric descriptor creates a concrete typed metric number
+func ResolveNumberByKind(t *testing.T, kind metric.NumberKind, value float64) metric.Number {
+	t.Helper()
+	switch kind {
+	case metric.Int64NumberKind:
+		return metric.NewInt64Number(int64(value))
+	case metric.Float64NumberKind:
+		return metric.NewFloat64Number(value)
+	}
+	panic("invalid number kind")
+}

api/metric/registry/registry_test.go

@@ -22,8 +22,8 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel/api/metric"
+	mockTest "go.opentelemetry.io/otel/api/metric/metrictest"
 	"go.opentelemetry.io/otel/api/metric/registry"
-	mockTest "go.opentelemetry.io/otel/internal/metric"
 )
 
 type (

sdk/metric/sdk.go

@@ -24,7 +24,7 @@ import (
 	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/api/metric"
 	api "go.opentelemetry.io/otel/api/metric"
-	internal "go.opentelemetry.io/otel/internal/metric"
+	internal "go.opentelemetry.io/otel/api/metric/metrictest"
 	"go.opentelemetry.io/otel/label"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator"