Add misc changes for metrics correctness

internal/goldendataset/metric_gen.go

@@ -28,6 +28,8 @@ import (
 type MetricCfg struct {
 	// The type of metric to generate
 	MetricDescriptorType pdata.MetricType
+	// A prefix for every metric name
+	MetricNamePrefix string
 	// The number of instrumentation library metrics per resource
 	NumILMPerResource int
 	// The size of the MetricSlice and number of Metrics
@@ -48,9 +50,12 @@ type MetricCfg struct {
 	StepSize uint64
 }
 
+// DefaultCfg produces a MetricCfg with default values. These should be good enough to produce sane
+// (but boring) metrics, and can be used as a starting point for making alterations.
 func DefaultCfg() MetricCfg {
 	return MetricCfg{
 		MetricDescriptorType: pdata.MetricTypeInt64,
+		MetricNamePrefix:     "",
 		NumILMPerResource:    1,
 		NumMetricsPerILM:     1,
 		NumPtLabels:          1,
@@ -63,11 +68,25 @@ func DefaultCfg() MetricCfg {
 	}
 }
 
+// DefaultMetricData produces MetricData with a default config.
 func DefaultMetricData() data.MetricData {
 	return MetricDataFromCfg(DefaultCfg())
 }
 
+// MetricDataFromCfg produces MetricData with the passed-in config.
 func MetricDataFromCfg(cfg MetricCfg) data.MetricData {
+	return newMetricGenerator().genMetricDataFromCfg(cfg)
+}
+
+type metricGenerator struct {
+	metricID int
+}
+
+func newMetricGenerator() *metricGenerator {
+	return &metricGenerator{}
+}
+
+func (g *metricGenerator) genMetricDataFromCfg(cfg MetricCfg) data.MetricData {
 	md := data.NewMetricData()
 	rms := md.ResourceMetrics()
 	rms.Resize(cfg.NumResourceMetrics)
@@ -81,27 +100,27 @@ func MetricDataFromCfg(cfg MetricCfg) data.MetricData {
 				pdata.NewAttributeValueString(fmt.Sprintf("resource-attr-val-%d", j)),
 			)
 		}
-		populateIlm(cfg, rm)
+		g.populateIlm(cfg, rm)
 	}
 	return md
 }
 
-func populateIlm(cfg MetricCfg, rm pdata.ResourceMetrics) {
+func (g *metricGenerator) populateIlm(cfg MetricCfg, rm pdata.ResourceMetrics) {
 	ilms := rm.InstrumentationLibraryMetrics()
 	ilms.Resize(cfg.NumILMPerResource)
 	for i := 0; i < cfg.NumILMPerResource; i++ {
 		ilm := ilms.At(i)
-		populateMetrics(cfg, ilm)
+		g.populateMetrics(cfg, ilm)
 	}
 }
 
-func populateMetrics(cfg MetricCfg, ilm pdata.InstrumentationLibraryMetrics) {
+func (g *metricGenerator) populateMetrics(cfg MetricCfg, ilm pdata.InstrumentationLibraryMetrics) {
 	metrics := ilm.Metrics()
 	metrics.Resize(cfg.NumMetricsPerILM)
 	for i := 0; i < cfg.NumMetricsPerILM; i++ {
 		metric := metrics.At(i)
 		metric.InitEmpty()
-		populateMetricDesc(cfg, metric)
+		g.populateMetricDesc(cfg, metric)
 		switch cfg.MetricDescriptorType {
 		case pdata.MetricTypeInt64, pdata.MetricTypeMonotonicInt64:
 			populateIntPoints(cfg, metric)
@@ -115,10 +134,11 @@ func populateMetrics(cfg MetricCfg, ilm pdata.InstrumentationLibraryMetrics) {
 	}
 }
 
-func populateMetricDesc(cfg MetricCfg, metric pdata.Metric) {
+func (g *metricGenerator) populateMetricDesc(cfg MetricCfg, metric pdata.Metric) {
 	desc := metric.MetricDescriptor()
 	desc.InitEmpty()
-	desc.SetName("my-md-name")
+	desc.SetName(fmt.Sprintf("%smetric_%d", cfg.MetricNamePrefix, g.metricID))
+	g.metricID++
 	desc.SetDescription("my-md-description")
 	desc.SetUnit("my-md-units")
 	desc.SetType(cfg.MetricDescriptorType)

internal/goldendataset/metric_gen_test.go

@@ -42,7 +42,7 @@ func TestGenDefault(t *testing.T) {
 	require.Equal(t, 1, ms.Len())
 	pdm := ms.At(0)
 	desc := pdm.MetricDescriptor()
-	require.Equal(t, "my-md-name", desc.Name())
+	require.Equal(t, "metric_0", desc.Name())
 	require.Equal(t, "my-md-description", desc.Description())
 	require.Equal(t, "my-md-units", desc.Unit())
 

internal/goldendataset/pict_metric_gen.go

@@ -15,6 +15,8 @@
 package goldendataset
 
 import (
+	"fmt"
+
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/internal/data"
 )
@@ -37,6 +39,7 @@ func GenerateMetricDatas(metricPairsFile string) ([]data.MetricData, error) {
 			NumPtLabels:     PICTNumPtLabels(values[2]),
 		}
 		cfg := pictToCfg(metricInputs)
+		cfg.MetricNamePrefix = fmt.Sprintf("pict_%d_", i)
 		md := MetricDataFromCfg(cfg)
 		out = append(out, md)
 	}

testbed/testbed/metric_diff.go → testbed/correctness/metrics/metric_diff.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package testbed
+package metrics
 
 import (
 	"fmt"
@@ -26,13 +26,13 @@ import (
 // testing. Two MetricDatas, when compared, could produce a list of MetricDiffs containing all of their
 // differences, which could be used to correct the differences between the expected and actual values.
 type MetricDiff struct {
-	expectedValue interface{}
-	actualValue   interface{}
-	msg           string
+	ExpectedValue interface{}
+	ActualValue   interface{}
+	Msg           string
 }
 
 func (mf MetricDiff) String() string {
-	return fmt.Sprintf("{msg='%v' expected=[%v] actual=[%v]}\n", mf.msg, mf.expectedValue, mf.actualValue)
+	return fmt.Sprintf("{msg='%v' expected=[%v] actual=[%v]}\n", mf.Msg, mf.ExpectedValue, mf.ActualValue)
 }
 
 func pdmToPDRM(pdm []pdata.Metrics) (out []pdata.ResourceMetrics) {
@@ -51,9 +51,9 @@ func diffRMSlices(sent []pdata.ResourceMetrics, recd []pdata.ResourceMetrics) []
 	var diffs []*MetricDiff
 	if len(sent) != len(recd) {
 		return []*MetricDiff{{
-			expectedValue: len(sent),
-			actualValue:   len(recd),
-			msg:           "Sent vs received ResourceMetrics not equal length",
+			ExpectedValue: len(sent),
+			ActualValue:   len(recd),
+			Msg:           "Sent vs received ResourceMetrics not equal length",
 		}}
 	}
 	for i := 0; i < len(sent); i++ {
@@ -105,12 +105,12 @@ func diffMetrics(diffs []*MetricDiff, expected pdata.MetricSlice, actual pdata.M
 		return diffs
 	}
 	for i := 0; i < expected.Len(); i++ {
-		diffs = diffMetric(diffs, expected.At(i), actual.At(i))
+		diffs = DiffMetric(diffs, expected.At(i), actual.At(i))
 	}
 	return diffs
 }
 
-func diffMetric(diffs []*MetricDiff, expected pdata.Metric, actual pdata.Metric) []*MetricDiff {
+func DiffMetric(diffs []*MetricDiff, expected pdata.Metric, actual pdata.Metric) []*MetricDiff {
 	diffs = diffMetricDescriptor(diffs, expected.MetricDescriptor(), actual.MetricDescriptor())
 	diffs = diffInt64Pts(diffs, expected.Int64DataPoints(), actual.Int64DataPoints())
 	diffs = diffDoublePts(diffs, expected.DoubleDataPoints(), actual.DoubleDataPoints())
@@ -305,9 +305,9 @@ func diffResource(diffs []*MetricDiff, expected pdata.Resource, actual pdata.Res
 func diffAttrs(diffs []*MetricDiff, expected pdata.AttributeMap, actual pdata.AttributeMap) []*MetricDiff {
 	if !reflect.DeepEqual(expected, actual) {
 		diffs = append(diffs, &MetricDiff{
-			expectedValue: attrMapToString(expected),
-			actualValue:   attrMapToString(actual),
-			msg:           "Resource attributes",
+			ExpectedValue: attrMapToString(expected),
+			ActualValue:   attrMapToString(actual),
+			Msg:           "Resource attributes",
 		})
 	}
 	return diffs
@@ -326,9 +326,9 @@ func diffValues(
 ) ([]*MetricDiff, bool) {
 	if expected != actual {
 		return append(diffs, &MetricDiff{
-			msg:           msg,
-			expectedValue: expected,
-			actualValue:   actual,
+			Msg:           msg,
+			ExpectedValue: expected,
+			ActualValue:   actual,
 		}), true
 	}
 	return diffs, false

testbed/testbed/metric_diff_test.go → testbed/correctness/metrics/metric_diff_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package testbed
+package metrics
 
 import (
 	"testing"

testbed/testbed/child_process.go

@@ -114,10 +114,10 @@ type ChildProcess struct {
 }
 
 type StartParams struct {
-	name         string
-	logFilePath  string
-	cmd          string
-	cmdArgs      []string
+	Name         string
+	LogFilePath  string
+	Cmd          string
+	CmdArgs      []string
 	resourceSpec *ResourceSpec
 }
 
@@ -167,23 +167,23 @@ func (cp *ChildProcess) PrepareConfig(configStr string) (configCleanup func(), e
 // cmdArgs is the command line arguments to pass to the process.
 func (cp *ChildProcess) Start(params StartParams) (receiverAddr string, err error) {
 
-	cp.name = params.name
+	cp.name = params.Name
 	cp.doneSignal = make(chan struct{})
 	cp.resourceSpec = params.resourceSpec
 
-	log.Printf("Starting %s (%s)", cp.name, params.cmd)
+	log.Printf("Starting %s (%s)", cp.name, params.Cmd)
 
 	// Prepare log file
 	var logFile *os.File
-	logFile, err = os.Create(params.logFilePath)
+	logFile, err = os.Create(params.LogFilePath)
 	if err != nil {
-		return receiverAddr, fmt.Errorf("cannot create %s: %s", params.logFilePath, err.Error())
+		return receiverAddr, fmt.Errorf("cannot create %s: %s", params.LogFilePath, err.Error())
 	}
-	log.Printf("Writing %s log to %s", cp.name, params.logFilePath)
+	log.Printf("Writing %s log to %s", cp.name, params.LogFilePath)
 
 	// Prepare to start the process.
 	// #nosec
-	args := params.cmdArgs
+	args := params.CmdArgs
 	if !containsConfig(args) {
 		if cp.configFileName == "" {
 			configFile := path.Join("testdata", "agent-config.yaml")
@@ -195,21 +195,21 @@ func (cp *ChildProcess) Start(params StartParams) (receiverAddr string, err erro
 		args = append(args, "--config")
 		args = append(args, cp.configFileName)
 	}
-	cp.cmd = exec.Command(params.cmd, args...)
+	cp.cmd = exec.Command(params.Cmd, args...)
 
 	// Capture standard output and standard error.
 	stdoutIn, err := cp.cmd.StdoutPipe()
 	if err != nil {
-		return receiverAddr, fmt.Errorf("cannot capture stdout of %s: %s", params.cmd, err.Error())
+		return receiverAddr, fmt.Errorf("cannot capture stdout of %s: %s", params.Cmd, err.Error())
 	}
 	stderrIn, err := cp.cmd.StderrPipe()
 	if err != nil {
-		return receiverAddr, fmt.Errorf("cannot capture stderr of %s: %s", params.cmd, err.Error())
+		return receiverAddr, fmt.Errorf("cannot capture stderr of %s: %s", params.Cmd, err.Error())
 	}
 
 	// Start the process.
 	if err = cp.cmd.Start(); err != nil {
-		return receiverAddr, fmt.Errorf("cannot start executable at %s: %s", params.cmd, err.Error())
+		return receiverAddr, fmt.Errorf("cannot start executable at %s: %s", params.Cmd, err.Error())
 	}
 
 	cp.startTime = time.Now()

testbed/testbed/mock_backend.go

@@ -23,6 +23,7 @@ import (
 
 	"go.uber.org/atomic"
 
+	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/consumer/consumerdata"
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/consumer/pdatautil"
@@ -124,7 +125,7 @@ func (mb *MockBackend) GetStats() string {
 
 // DataItemsReceived returns total number of received spans and metrics.
 func (mb *MockBackend) DataItemsReceived() uint64 {
-	return mb.tc.spansReceived.Load() + mb.mc.metricsReceived.Load()
+	return mb.tc.numSpansReceived.Load() + mb.mc.numMetricsReceived.Load()
 }
 
 // ClearReceivedItems clears the list of received traces and metrics. Note: counters
@@ -172,13 +173,20 @@ func (mb *MockBackend) ConsumeMetricOld(md consumerdata.MetricsData) {
 	}
 }
 
+type TraceDualConsumer interface {
+	consumer.TraceConsumer
+	consumer.TraceConsumerOld
+}
+
+var _ TraceDualConsumer = (*MockTraceConsumer)(nil)
+
 type MockTraceConsumer struct {
-	spansReceived atomic.Uint64
-	backend       *MockBackend
+	numSpansReceived atomic.Uint64
+	backend          *MockBackend
 }
 
 func (tc *MockTraceConsumer) ConsumeTraces(_ context.Context, td pdata.Traces) error {
-	tc.spansReceived.Add(uint64(td.SpanCount()))
+	tc.numSpansReceived.Add(uint64(td.SpanCount()))
 
 	rs := td.ResourceSpans()
 	for i := 0; i < rs.Len(); i++ {
@@ -214,7 +222,7 @@ func (tc *MockTraceConsumer) ConsumeTraces(_ context.Context, td pdata.Traces) e
 }
 
 func (tc *MockTraceConsumer) ConsumeTraceData(_ context.Context, td consumerdata.TraceData) error {
-	tc.spansReceived.Add(uint64(len(td.Spans)))
+	tc.numSpansReceived.Add(uint64(len(td.Spans)))
 
 	for _, span := range td.Spans {
 		var spanSeqnum int64
@@ -243,29 +251,28 @@ func (tc *MockTraceConsumer) ConsumeTraceData(_ context.Context, td consumerdata
 	return nil
 }
 
+type MetricsDualConsumer interface {
+	consumer.MetricsConsumer
+	consumer.MetricsConsumerOld
+}
+
+var _ MetricsDualConsumer = (*MockMetricConsumer)(nil)
+
 type MockMetricConsumer struct {
-	metricsReceived atomic.Uint64
-	backend         *MockBackend
+	numMetricsReceived atomic.Uint64
+	backend            *MockBackend
 }
 
 func (mc *MockMetricConsumer) ConsumeMetrics(_ context.Context, md pdata.Metrics) error {
 	_, dataPoints := pdatautil.MetricAndDataPointCount(md)
-	mc.metricsReceived.Add(uint64(dataPoints))
+	mc.numMetricsReceived.Add(uint64(dataPoints))
 	mc.backend.ConsumeMetric(md)
 	return nil
 }
 
 func (mc *MockMetricConsumer) ConsumeMetricsData(_ context.Context, md consumerdata.MetricsData) error {
-	dataPoints := 0
-	for _, metric := range md.Metrics {
-		for _, ts := range metric.Timeseries {
-			dataPoints += len(ts.Points)
-		}
-	}
-
-	mc.metricsReceived.Add(uint64(dataPoints))
-
+	_, dataPoints := pdatautil.TimeseriesAndPointCount(md)
+	mc.numMetricsReceived.Add(uint64(dataPoints))
 	mc.backend.ConsumeMetricOld(md)
-
 	return nil
 }

testbed/testbed/otelcol_runner.go

@@ -115,7 +115,7 @@ func (ipp *InProcessCollector) Start(args StartParams) (receiverAddr string, err
 	if err != nil {
 		return receiverAddr, err
 	}
-	ipp.svc.Command().SetArgs(args.cmdArgs)
+	ipp.svc.Command().SetArgs(args.CmdArgs)
 
 	ipp.appDone = make(chan struct{})
 	go func() {