Skip to content

Commit

Permalink
return errors when naming unsupported types
Browse files Browse the repository at this point in the history
  • Loading branch information
@dashpole
dashpole committed May 3, 2022
1 parent 8cd3b4e commit e4d07ca
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 49 deletions.
2 changes: 1 addition & 1 deletion exporter/collector/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 8 additions & 9 deletions exporter/collector/googlemanagedprometheus/naming.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
60 changes: 47 additions & 13 deletions exporter/collector/googlemanagedprometheus/naming_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
94 changes: 70 additions & 24 deletions exporter/collector/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.Info("Unable to get metric name for summary 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{
Expand All @@ -413,7 +417,7 @@ func (m *metricMapper) summaryPointToTimeSeries(
}},
}},
Metric: &metricpb.Metric{
Type: m.metricNameToType(sumName, metric),
Type: sumName,
Labels: mergeLabels(
attributesToLabels(point.Attributes()),
extraLabels,
Expand All @@ -435,7 +439,7 @@ func (m *metricMapper) summaryPointToTimeSeries(
}},
}},
Metric: &metricpb.Metric{
Type: m.metricNameToType(countName, metric),
Type: countName,
Labels: mergeLabels(
attributesToLabels(point.Attributes()),
extraLabels,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.Info("Unable to get metric name for histogram 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())
Expand Down Expand Up @@ -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,
Expand All @@ -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.Info("Unable to get metric name for exponential histogram 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())
Expand Down Expand Up @@ -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,
Expand All @@ -732,6 +746,11 @@ func (m *metricMapper) sumPointToTimeSeries(
// prometheus.
return nil
}
name, err := m.metricNameToType(metric.Name(), metric)
if err != nil {
m.obs.log.Info("Unable to get metric name for sum 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())
Expand Down Expand Up @@ -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,
Expand All @@ -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.Info("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)

Expand All @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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.Info("Unable to get metric name for 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{
Expand All @@ -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.Info("Unable to get metric name for metric.", 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 {
Expand Down
8 changes: 6 additions & 2 deletions exporter/collector/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down Expand Up @@ -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)
})
}
}
Expand Down

0 comments on commit e4d07ca

Please sign in to comment.