return errors when naming unsupported types

exporter/collector/config.go

@@ -113,7 +113,7 @@ type MetricConfig struct {
 	// exporters which extend the functionality of this exporter. It allows
 	// customizing the naming of metrics. baseName already includes type
 	// suffixes for summary metrics, but does not (yet) include the domain prefix
-	GetMetricName func(baseName string, metric pdata.Metric) string
+	GetMetricName func(baseName string, metric pdata.Metric) (string, error)
 
 	// MapMonitoredResource is not exposed as an option in the configuration, but
 	// can be used by other exporters to extend the functionality of this

exporter/collector/googlemanagedprometheus/naming.go

@@ -15,32 +15,31 @@
 package googlemanagedprometheus
 
 import (
+	"fmt"
 	"strings"
 
 	"go.opentelemetry.io/collector/model/pdata"
 	"go.opentelemetry.io/collector/pdata/pmetric"
 )
 
-func GetMetricName(baseName string, metric pdata.Metric) string {
+func GetMetricName(baseName string, metric pdata.Metric) (string, error) {
 	switch metric.DataType() {
 	case pmetric.MetricDataTypeSum:
-		return baseName + "/counter"
+		return baseName + "/counter", nil
 	case pmetric.MetricDataTypeGauge:
-		return baseName + "/gauge"
+		return baseName + "/gauge", nil
 	case pmetric.MetricDataTypeSummary:
 		// summaries are sent as the following series:
 		// * Sum: prometheus.googleapis.com/<baseName>_sum/summary:counter
 		// * Count: prometheus.googleapis.com/<baseName>_count/summary
 		// * Quantiles: prometheus.googleapis.com/<baseName>/summary
 		if strings.HasSuffix(baseName, "_sum") {
-			return baseName + "/summary:counter"
+			return baseName + "/summary:counter", nil
 		}
-		return baseName + "/summary"
+		return baseName + "/summary", nil
 	case pmetric.MetricDataTypeHistogram:
-		return baseName + "/histogram"
+		return baseName + "/histogram", nil
 	default:
-		// This should never happen, as we have already dropped other data
-		// types at this point.
-		return ""
+		return "", fmt.Errorf("unsupported metric datatype: %v", metric.DataType())
 	}
 }

exporter/collector/googlemanagedprometheus/naming_test.go

@@ -24,40 +24,74 @@ import (
 func TestGetMetricName(t *testing.T) {
 	baseName := "my_metric_name"
 	for _, tc := range []struct {
-		desc     string
-		datatype pmetric.MetricDataType
-		expected string
+		desc      string
+		baseName  string
+		datatype  pmetric.MetricDataType
+		expected  string
+		expectErr bool
 	}{
 		{
 			desc:     "sum",
+			baseName: "foo_total",
 			datatype: pmetric.MetricDataTypeSum,
-			expected: baseName + "/counter",
+			expected: "foo_total/counter",
 		},
 		{
 			desc:     "gauge",
+			baseName: "bar",
 			datatype: pmetric.MetricDataTypeGauge,
-			expected: baseName + "/gauge",
+			expected: "bar/gauge",
 		},
 		{
-			desc:     "summary",
+			desc:     "summary sum",
+			baseName: "baz_sum",
 			datatype: pmetric.MetricDataTypeSummary,
-			expected: baseName + "/summary",
+			expected: "baz_sum/summary:counter",
 		},
 		{
-			desc:     "histogram",
+			desc:     "summary count",
+			baseName: "baz_count",
+			datatype: pmetric.MetricDataTypeSummary,
+			expected: "baz_count/summary",
+		},
+		{
+			desc:     "summary quantile",
+			baseName: "baz",
+			datatype: pmetric.MetricDataTypeSummary,
+			expected: "baz/summary",
+		},
+		{
+			desc:     "histogram sum",
+			baseName: "hello_sum",
 			datatype: pmetric.MetricDataTypeHistogram,
-			expected: baseName + "/histogram",
+			expected: "hello_sum/histogram",
 		},
 		{
-			desc:     "other",
-			datatype: pmetric.MetricDataTypeExponentialHistogram,
-			expected: baseName + "/exponentialhistogram",
+			desc:     "histogram count",
+			baseName: "hello_count",
+			datatype: pmetric.MetricDataTypeHistogram,
+			expected: "hello_count/histogram",
+		},
+		{
+			desc:     "histogram bucket",
+			baseName: "hello",
+			datatype: pmetric.MetricDataTypeHistogram,
+			expected: "hello/histogram",
+		},
+		{
+			desc:      "other",
+			baseName:  "other",
+			datatype:  pmetric.MetricDataTypeExponentialHistogram,
+			expectErr: true,
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			metric := pdata.NewMetric()
 			metric.SetDataType(tc.datatype)
-			got := GetMetricName(baseName, metric)
+			got, err := GetMetricName(tc.baseName, metric)
+			if tc.expectErr == (err == nil) {
+				t.Errorf("MetricName(%v, %v)=err(%v); want err: %v", baseName, tc.datatype, err, tc.expectErr)
+			}
 			if got != tc.expected {
 				t.Errorf("MetricName(%v, %v)=%v; want %v", baseName, tc.datatype, got, tc.expected)
 			}

exporter/collector/metrics.go

@@ -394,7 +394,11 @@ func (m *metricMapper) summaryPointToTimeSeries(
 		return nil
 	}
 	point = *normalizedPoint
-	sumName, countName := summaryMetricNames(metric.Name())
+	sumName, countName, quantileName, err := m.summaryMetricNames(metric)
+	if err != nil {
+		m.obs.log.Debug("Failed to get metric name for summary metric. Dropping the metric.", zap.Error(err), zap.Any("metric", metric))
+		return nil
+	}
 	startTime := timestamppb.New(point.StartTimestamp().AsTime())
 	endTime := timestamppb.New(point.Timestamp().AsTime())
 	result := []*monitoringpb.TimeSeries{
@@ -413,7 +417,7 @@ func (m *metricMapper) summaryPointToTimeSeries(
 				}},
 			}},
 			Metric: &metricpb.Metric{
-				Type: m.metricNameToType(sumName, metric),
+				Type: sumName,
 				Labels: mergeLabels(
 					attributesToLabels(point.Attributes()),
 					extraLabels,
@@ -435,7 +439,7 @@ func (m *metricMapper) summaryPointToTimeSeries(
 				}},
 			}},
 			Metric: &metricpb.Metric{
-				Type: m.metricNameToType(countName, metric),
+				Type: countName,
 				Labels: mergeLabels(
 					attributesToLabels(point.Attributes()),
 					extraLabels,
@@ -463,7 +467,7 @@ func (m *metricMapper) summaryPointToTimeSeries(
 				}},
 			}},
 			Metric: &metricpb.Metric{
-				Type: m.metricNameToType(metric.Name(), metric),
+				Type: quantileName,
 				Labels: mergeLabels(
 					attributesToLabels(point.Attributes()),
 					extraLabels,
@@ -634,6 +638,11 @@ func (m *metricMapper) histogramToTimeSeries(
 		// Drop points without a value or without a sum
 		return nil
 	}
+	name, err := m.metricNameToType(metric.Name(), metric)
+	if err != nil {
+		m.obs.log.Debug("Failed to get metric name for histogram metric. Dropping the metric.", zap.Error(err), zap.Any("metric", metric))
+		return nil
+	}
 	if hist.AggregationTemporality() == pmetric.MetricAggregationTemporalityCumulative {
 		// Normalize cumulative histogram points.
 		metricIdentifier := datapointstorage.Identifier(resource, extraLabels, metric, point.Attributes())
@@ -662,7 +671,7 @@ func (m *metricMapper) histogramToTimeSeries(
 			Value: value,
 		}},
 		Metric: &metricpb.Metric{
-			Type: m.metricNameToType(metric.Name(), metric),
+			Type: name,
 			Labels: mergeLabels(
 				attributesToLabels(point.Attributes()),
 				extraLabels,
@@ -682,6 +691,11 @@ func (m *metricMapper) exponentialHistogramToTimeSeries(
 		// Drop points without a value.
 		return nil
 	}
+	name, err := m.metricNameToType(metric.Name(), metric)
+	if err != nil {
+		m.obs.log.Debug("Failed to get metric name for exponential histogram metric. Dropping the metric.", zap.Error(err), zap.Any("metric", metric))
+		return nil
+	}
 	if exponentialHist.AggregationTemporality() == pmetric.MetricAggregationTemporalityCumulative {
 		// Normalize the histogram point.
 		metricIdentifier := datapointstorage.Identifier(resource, extraLabels, metric, point.Attributes())
@@ -709,7 +723,7 @@ func (m *metricMapper) exponentialHistogramToTimeSeries(
 			Value: value,
 		}},
 		Metric: &metricpb.Metric{
-			Type: m.metricNameToType(metric.Name(), metric),
+			Type: name,
 			Labels: mergeLabels(
 				attributesToLabels(point.Attributes()),
 				extraLabels,
@@ -732,6 +746,11 @@ func (m *metricMapper) sumPointToTimeSeries(
 		// prometheus.
 		return nil
 	}
+	name, err := m.metricNameToType(metric.Name(), metric)
+	if err != nil {
+		m.obs.log.Debug("Failed to get metric name for sum metric. Dropping the metric.", zap.Error(err), zap.Any("metric", metric))
+		return nil
+	}
 	if sum.IsMonotonic() {
 		if sum.AggregationTemporality() == pmetric.MetricAggregationTemporalityCumulative {
 			metricIdentifier := datapointstorage.Identifier(resource, extraLabels, metric, point.Attributes())
@@ -761,7 +780,7 @@ func (m *metricMapper) sumPointToTimeSeries(
 			Value: value,
 		}},
 		Metric: &metricpb.Metric{
-			Type: m.metricNameToType(metric.Name(), metric),
+			Type: name,
 			Labels: mergeLabels(
 				attributesToLabels(point.Attributes()),
 				extraLabels,
@@ -781,6 +800,11 @@ func (m *metricMapper) gaugePointToTimeSeries(
 		// Drop points without a value.
 		return nil
 	}
+	name, err := m.metricNameToType(metric.Name(), metric)
+	if err != nil {
+		m.obs.log.Debug("Unable to get metric name for gauge metric.", zap.Error(err), zap.Any("metric", metric))
+		return nil
+	}
 	metricKind := metricpb.MetricDescriptor_GAUGE
 	value, valueType := numberDataPointToValue(point)
 
@@ -796,7 +820,7 @@ func (m *metricMapper) gaugePointToTimeSeries(
 			Value: value,
 		}},
 		Metric: &metricpb.Metric{
-			Type: m.metricNameToType(metric.Name(), metric),
+			Type: name,
 			Labels: mergeLabels(
 				attributesToLabels(point.Attributes()),
 				extraLabels,
@@ -816,13 +840,17 @@ func (m *metricMapper) getMetricNamePrefix(name string) string {
 }
 
 // metricNameToType maps OTLP metric name to GCM metric type (aka name)
-func (m *metricMapper) metricNameToType(name string, metric pdata.Metric) string {
-	return path.Join(m.getMetricNamePrefix(name), m.cfg.MetricConfig.GetMetricName(name, metric))
+func (m *metricMapper) metricNameToType(name string, metric pdata.Metric) (string, error) {
+	metricName, err := m.cfg.MetricConfig.GetMetricName(name, metric)
+	if err != nil {
+		return "", err
+	}
+	return path.Join(m.getMetricNamePrefix(name), metricName), nil
 }
 
 // defaultGetMetricName does not (further) customize the baseName
-func defaultGetMetricName(baseName string, _ pdata.Metric) string {
-	return baseName
+func defaultGetMetricName(baseName string, _ pdata.Metric) (string, error) {
+	return baseName, nil
 }
 
 func numberDataPointToValue(
@@ -955,40 +983,54 @@ func (m *metricMapper) labelDescriptors(
 	return result
 }
 
-// Returns (sum, count) metric names for a summary metric.
-func summaryMetricNames(name string) (string, string) {
-	sumName := fmt.Sprintf("%s%s", name, SummarySumSuffix)
-	countName := fmt.Sprintf("%s%s", name, SummaryCountPrefix)
-	return sumName, countName
+// Returns (sum, count, quantile) metric names for a summary metric.
+func (m *metricMapper) summaryMetricNames(pm pdata.Metric) (string, string, string, error) {
+	sumName, err := m.metricNameToType(pm.Name()+SummarySumSuffix, pm)
+	if err != nil {
+		return "", "", "", err
+	}
+	countName, err := m.metricNameToType(pm.Name()+SummaryCountPrefix, pm)
+	if err != nil {
+		return "", "", "", err
+	}
+	quantileName, err := m.metricNameToType(pm.Name(), pm)
+	if err != nil {
+		return "", "", "", err
+	}
+	return sumName, countName, quantileName, nil
 }
 
 func (m *metricMapper) summaryMetricDescriptors(
 	pm pdata.Metric,
 	extraLabels labels,
 ) []*metricpb.MetricDescriptor {
-	sumName, countName := summaryMetricNames(pm.Name())
+	sumName, countName, quantileName, err := m.summaryMetricNames(pm)
+	if err != nil {
+		m.obs.log.Debug("Failed to get metric name for metric. Dropping the metric.", zap.Error(err), zap.Any("metric", pm))
+		return nil
+	}
 	labels := m.labelDescriptors(pm, extraLabels)
 	return []*metricpb.MetricDescriptor{
 		{
-			Type:        m.metricNameToType(sumName, pm),
+			Type:        sumName,
 			Labels:      labels,
 			MetricKind:  metricpb.MetricDescriptor_CUMULATIVE,
 			ValueType:   metricpb.MetricDescriptor_DOUBLE,
 			Unit:        pm.Unit(),
 			Description: pm.Description(),
-			DisplayName: sumName,
+			DisplayName: pm.Name() + SummarySumSuffix,
 		},
 		{
-			Type:        m.metricNameToType(countName, pm),
+			Type:        countName,
 			Labels:      labels,
 			MetricKind:  metricpb.MetricDescriptor_CUMULATIVE,
 			ValueType:   metricpb.MetricDescriptor_DOUBLE,
 			Unit:        pm.Unit(),
 			Description: pm.Description(),
-			DisplayName: countName,
+			DisplayName: pm.Name() + SummaryCountPrefix,
 		},
 		{
-			Type: m.metricNameToType(pm.Name(), pm),
+			Type: quantileName,
 			Labels: append(
 				labels,
 				&label.LabelDescriptor{
@@ -1013,7 +1055,11 @@ func (m *metricMapper) metricDescriptor(
 		return m.summaryMetricDescriptors(pm, extraLabels)
 	}
 	kind, typ := mapMetricPointKind(pm)
-	metricType := m.metricNameToType(pm.Name(), pm)
+	metricType, err := m.metricNameToType(pm.Name(), pm)
+	if err != nil {
+		m.obs.log.Debug("Failed to get metric name for metric. Dropping the metric descriptor.", zap.Error(err), zap.Any("metric", pm))
+		return nil
+	}
 	labels := m.labelDescriptors(pm, extraLabels)
 	// Return nil for unsupported types.
 	if kind == metricpb.MetricDescriptor_METRIC_KIND_UNSPECIFIED {

exporter/collector/metrics_test.go

@@ -1409,9 +1409,11 @@ func TestMetricNameToType(t *testing.T) {
 	mapper, shutdown := newTestMetricMapper()
 	defer shutdown()
 	metric := pdata.NewMetric()
+	got, err := mapper.metricNameToType("foo", metric)
+	assert.NoError(t, err)
 	assert.Equal(
 		t,
-		mapper.metricNameToType("foo", metric),
+		got,
 		"workload.googleapis.com/foo",
 		"Should use workload metric domain with default config",
 	)
@@ -1922,7 +1924,9 @@ func TestKnownDomains(t *testing.T) {
 				mapper.cfg.MetricConfig.KnownDomains = test.knownDomains
 			}
 			mapper.cfg.MetricConfig.Prefix = "prefix"
-			assert.Equal(t, test.metricType, mapper.metricNameToType(test.name, metric))
+			metricType, err := mapper.metricNameToType(test.name, metric)
+			assert.NoError(t, err)
+			assert.Equal(t, test.metricType, metricType)
 		})
 	}
 }