[pkg/pdatatest] Do not ignore timestamps in CompareMetrics, use optio…

…ns (open-telemetry#17983)

It's pretty common to ignore timestamps when using CompareMetrics in tests for scrapers, but it's not obvious behavior for other use cases. We should provide options to explicitly ignore timestamps. That will make the Compare... functions provide clean comparison with not implicit behavior.

.chloggen/pmetrictest-dont-ignore-timestamps.yaml

@@ -0,0 +1,11 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: pkg/pdatatest
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Do not ignore timestamps implicitly, provide explicit options for that.
+
+# One or more tracking issues related to the change
+issues: [17865]

pkg/pdatatest/README.md

@@ -23,7 +23,8 @@ func TestMetricsScraper(t *testing.T) {
  expectedFile, err := readMetrics(filepath.Join("testdata", "expected.json"))
  require.NoError(err)
 
- require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics))
+ require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreStartTimestamp(), 
+ pmetrictest.IgnoreTimestamp()))))
 }
 ```
 

pkg/pdatatest/pmetrictest/metrics.go

@@ -355,6 +355,12 @@ func compareNumberDataPointSlices(expected, actual pmetric.NumberDataPointSlice)
 // CompareNumberDataPoint compares each part of two given NumberDataPoints and returns
 // an error if they don't match. The error describes what didn't match.
 func CompareNumberDataPoint(expected, actual pmetric.NumberDataPoint) error {
+ if expected.StartTimestamp() != actual.StartTimestamp() {
+ return fmt.Errorf("metric datapoint StartTimestamp doesn't match expected: %d, actual: %d", expected.StartTimestamp(), actual.StartTimestamp())
+ }
+ if expected.Timestamp() != actual.Timestamp() {
+ return fmt.Errorf("metric datapoint Timestamp doesn't match expected: %d, actual: %d", expected.Timestamp(), actual.Timestamp())
+ }
  if expected.ValueType() != actual.ValueType() {
  return fmt.Errorf("metric datapoint types don't match: expected type: %s, actual type: %s", expected.ValueType(), actual.ValueType())
  }

pkg/pdatatest/pmetrictest/metrics_test.go

@@ -510,10 +510,35 @@ func TestCompareMetrics(t *testing.T) {
  ),
  },
  },
+ {
+ name: "ignore-start-timestamp",
+ compareOptions: []CompareMetricsOption{
+ IgnoreStartTimestamp(),
+ },
+ withoutOptions: internal.Expectation{
+ Err: multierr.Combine(
+ errors.New("ResourceMetrics with attributes map[] does not match expected"),
+ errors.New(`ScopeMetrics with scope name "" does not match expected`),
+ errors.New("metric gauge.one does not match expected"),
+ errors.New("datapoint with attributes: map[], does not match expected"),
+ errors.New("metric datapoint StartTimestamp doesn't match expected: 0, actual: 11651379494838206464"),
+ ),
+ Reason: "Timestamps are always ignored, so no error is expected.",
+ },
+ },
  {
  name: "ignore-timestamp",
+ compareOptions: []CompareMetricsOption{
+ IgnoreTimestamp(),
+ },
  withoutOptions: internal.Expectation{
- Err: nil,
+ Err: multierr.Combine(
+ errors.New("ResourceMetrics with attributes map[] does not match expected"),
+ errors.New(`ScopeMetrics with scope name "" does not match expected`),
+ errors.New("metric gauge.one does not match expected"),
+ errors.New("datapoint with attributes: map[], does not match expected"),
+ errors.New("metric datapoint Timestamp doesn't match expected: 0, actual: 11651379494838206464"),
+ ),
  Reason: "Timestamps are always ignored, so no error is expected.",
  },
  },

pkg/pdatatest/pmetrictest/options.go

@@ -17,6 +17,7 @@ package pmetrictest // import "github.com/open-telemetry/opentelemetry-collector
 import (
  "bytes"
  "fmt"
+ "time"
 
  "go.opentelemetry.io/collector/pdata/pcommon"
  "go.opentelemetry.io/collector/pdata/pmetric"
@@ -89,6 +90,90 @@ func maskDataPointSliceValues(dataPoints pmetric.NumberDataPointSlice) {
  }
 }
 
+// IgnoreTimestamp is a CompareMetricsOption that clears Timestamp fields on all the data points.
+func IgnoreTimestamp() CompareMetricsOption {
+ return compareMetricsOptionFunc(func(expected, actual pmetric.Metrics) {
+ now := pcommon.NewTimestampFromTime(time.Now())
+ maskTimestamp(expected, now)
+ maskTimestamp(actual, now)
+ })
+}
+
+func maskTimestamp(metrics pmetric.Metrics, ts pcommon.Timestamp) {
+ rms := metrics.ResourceMetrics()
+ for i := 0; i < rms.Len(); i++ {
+ for j := 0; j < rms.At(i).ScopeMetrics().Len(); j++ {
+ for k := 0; k < rms.At(i).ScopeMetrics().At(j).Metrics().Len(); k++ {
+ m := rms.At(i).ScopeMetrics().At(j).Metrics().At(k)
+ switch m.Type() {
+ case pmetric.MetricTypeGauge:
+ for l := 0; l < m.Gauge().DataPoints().Len(); l++ {
+ m.Gauge().DataPoints().At(l).SetTimestamp(ts)
+ }
+ case pmetric.MetricTypeSum:
+ for l := 0; l < m.Sum().DataPoints().Len(); l++ {
+ m.Sum().DataPoints().At(l).SetTimestamp(ts)
+ }
+ case pmetric.MetricTypeHistogram:
+ for l := 0; l < m.Histogram().DataPoints().Len(); l++ {
+ m.Histogram().DataPoints().At(l).SetTimestamp(ts)
+ }
+ case pmetric.MetricTypeExponentialHistogram:
+ for l := 0; l < m.ExponentialHistogram().DataPoints().Len(); l++ {
+ m.ExponentialHistogram().DataPoints().At(l).SetTimestamp(ts)
+ }
+ case pmetric.MetricTypeSummary:
+ for l := 0; l < m.Summary().DataPoints().Len(); l++ {
+ m.Summary().DataPoints().At(l).SetTimestamp(ts)
+ }
+ }
+ }
+ }
+ }
+}
+
+// IgnoreStartTimestamp is a CompareMetricsOption that clears StartTimestamp fields on all the data points.
+func IgnoreStartTimestamp() CompareMetricsOption {
+ return compareMetricsOptionFunc(func(expected, actual pmetric.Metrics) {
+ now := pcommon.NewTimestampFromTime(time.Now())
+ maskStartTimestamp(expected, now)
+ maskStartTimestamp(actual, now)
+ })
+}
+
+func maskStartTimestamp(metrics pmetric.Metrics, ts pcommon.Timestamp) {
+ rms := metrics.ResourceMetrics()
+ for i := 0; i < rms.Len(); i++ {
+ for j := 0; j < rms.At(i).ScopeMetrics().Len(); j++ {
+ for k := 0; k < rms.At(i).ScopeMetrics().At(j).Metrics().Len(); k++ {
+ m := rms.At(i).ScopeMetrics().At(j).Metrics().At(k)
+ switch m.Type() {
+ case pmetric.MetricTypeGauge:
+ for l := 0; l < m.Gauge().DataPoints().Len(); l++ {
+ m.Gauge().DataPoints().At(l).SetStartTimestamp(ts)
+ }
+ case pmetric.MetricTypeSum:
+ for l := 0; l < m.Sum().DataPoints().Len(); l++ {
+ m.Sum().DataPoints().At(l).SetStartTimestamp(ts)
+ }
+ case pmetric.MetricTypeHistogram:
+ for l := 0; l < m.Histogram().DataPoints().Len(); l++ {
+ m.Histogram().DataPoints().At(l).SetStartTimestamp(ts)
+ }
+ case pmetric.MetricTypeExponentialHistogram:
+ for l := 0; l < m.ExponentialHistogram().DataPoints().Len(); l++ {
+ m.ExponentialHistogram().DataPoints().At(l).SetStartTimestamp(ts)
+ }
+ case pmetric.MetricTypeSummary:
+ for l := 0; l < m.Summary().DataPoints().Len(); l++ {
+ m.Summary().DataPoints().At(l).SetStartTimestamp(ts)
+ }
+ }
+ }
+ }
+ }
+}
+
 // IgnoreMetricAttributeValue is a CompareMetricsOption that clears value of the metric attribute.
 func IgnoreMetricAttributeValue(attributeName string, metricNames ...string) CompareMetricsOption {
  return compareMetricsOptionFunc(func(expected, actual pmetric.Metrics) {

pkg/pdatatest/pmetrictest/testdata/ignore-start-timestamp/actual.json

@@ -0,0 +1,34 @@
+{
+ "resourceMetrics": [
+ {
+ "scopeMetrics": [
+ {
+ "metrics": [
+ {
+ "name": "gauge.one",
+ "gauge": {
+ "dataPoints": [
+ {
+ "asDouble": 123.456,
+ "startTimeUnixNano": "11651379494838206464"
+ }
+ ]
+ }
+ },
+ {
+ "name": "sum.one",
+ "sum": {
+ "dataPoints": [
+ {
+ "asInt": 123,
+ "startTimeUnixNano": "11651379494838206464"
+ }
+ ]
+ }
+ }
+ ]
+ }
+ ]
+ }
+ ]
+}

pkg/pdatatest/pmetrictest/testdata/ignore-start-timestamp/expected.json

@@ -0,0 +1,34 @@
+{
+ "resourceMetrics": [
+ {
+ "scopeMetrics": [
+ {
+ "metrics": [
+ {
+ "name": "gauge.one",
+ "gauge": {
+ "dataPoints": [
+ {
+ "asDouble": 123.456,
+ "startTimeUnixNano": "0"
+ }
+ ]
+ }
+ },
+ {
+ "name": "sum.one",
+ "sum": {
+ "dataPoints": [
+ {
+ "asInt": 123,
+ "startTimeUnixNano": "0"
+ }
+ ]
+ }
+ }
+ ]
+ }
+ ]
+ }
+ ]
+}

receiver/aerospikereceiver/integration_test.go

@@ -324,7 +324,7 @@ func TestAerospikeIntegration(t *testing.T) {
 
  require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreMetricValues(),
  pmetrictest.IgnoreResourceAttributeValue("aerospike.node.name"),
- pmetrictest.IgnoreMetricDataPointsOrder()))
+ pmetrictest.IgnoreMetricDataPointsOrder(), pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp()))
 
  // now do a run in cluster mode
  cfg.CollectClusterMetrics = true
@@ -348,6 +348,6 @@ func TestAerospikeIntegration(t *testing.T) {
 
  require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreMetricValues(),
  pmetrictest.IgnoreResourceAttributeValue("aerospike.node.name"),
- pmetrictest.IgnoreMetricDataPointsOrder()))
+ pmetrictest.IgnoreMetricDataPointsOrder(), pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp()))
 
 }

receiver/aerospikereceiver/scraper_test.go

@@ -154,7 +154,7 @@ func TestScrape_CollectClusterMetrics(t *testing.T) {
 
  expectedMetrics := expectedMB.Emit()
  require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreResourceMetricsOrder(),
- pmetrictest.IgnoreMetricDataPointsOrder()))
+ pmetrictest.IgnoreMetricDataPointsOrder(), pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp()))
 
  require.NoError(t, receiver.shutdown(context.Background()))
 

receiver/apachereceiver/integration_test.go

@@ -78,7 +78,7 @@ func TestApacheIntegration(t *testing.T) {
  require.NoError(t, err)
 
  require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreMetricValues(),
- pmetrictest.IgnoreMetricDataPointsOrder()))
+ pmetrictest.IgnoreMetricDataPointsOrder(), pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp()))
 }
 
 func getContainer(t *testing.T, req testcontainers.ContainerRequest) testcontainers.Container {

receiver/apachereceiver/scraper_test.go

@@ -62,7 +62,7 @@ func TestScraper(t *testing.T) {
 
  // The port is random, so we shouldn't check if this value matches.
  require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics,
- pmetrictest.IgnoreMetricDataPointsOrder()))
+ pmetrictest.IgnoreMetricDataPointsOrder(), pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp()))
 }
 
 func TestScraperFailedStart(t *testing.T) {

receiver/bigipreceiver/integration_test.go

@@ -67,7 +67,9 @@ func TestBigIpIntegration(t *testing.T) {
  expectedMetrics, err := golden.ReadMetrics(expectedFile)
  require.NoError(t, err)
 
- require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreResourceMetricsOrder(), pmetrictest.IgnoreMetricValues()))
+ require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics,
+ pmetrictest.IgnoreResourceMetricsOrder(), pmetrictest.IgnoreMetricValues(),
+ pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp()))
 }
 
 const (

receiver/bigipreceiver/scraper_test.go

@@ -272,7 +272,9 @@ func TestScaperScrape(t *testing.T) {
 
  expectedMetrics := tc.expectedMetricGen(t)
 
- err = pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreResourceMetricsOrder())
+ err = pmetrictest.CompareMetrics(expectedMetrics, actualMetrics,
+ pmetrictest.IgnoreResourceMetricsOrder(), pmetrictest.IgnoreStartTimestamp(),
+ pmetrictest.IgnoreTimestamp())
  require.NoError(t, err)
  })
  }