Skip to content

Commit

Permalink
prometheus exporter: validate metric types and help/descriptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@kristinapathak
kristinapathak committed Nov 13, 2024
1 parent 6eece9b commit 9b54425
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 35 deletions.
70 changes: 56 additions & 14 deletions exporter/prometheusexporter/collector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import (
"sort"

"github.com/prometheus/client_golang/prometheus"
dto "github.com/prometheus/client_model/go"
"github.com/prometheus/common/model"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
conventions "go.opentelemetry.io/collector/semconv/v1.25.0"
"go.uber.org/zap"
"google.golang.org/protobuf/proto"

prometheustranslator "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus"
)
Expand All @@ -30,6 +32,7 @@ type collector struct {
addMetricSuffixes bool
namespace string
constLabels prometheus.Labels
metricFamilies map[string]*dto.MetricFamily
}

func newCollector(config *Config, logger *zap.Logger) *collector {
Expand All @@ -40,6 +43,7 @@ func newCollector(config *Config, logger *zap.Logger) *collector {
sendTimestamps: config.SendTimestamps,
constLabels: config.ConstLabels,
addMetricSuffixes: config.AddMetricSuffixes,
metricFamilies: make(map[string]*dto.MetricFamily),
}
}

Expand Down Expand Up @@ -105,7 +109,13 @@ func (c *collector) convertMetric(metric pmetric.Metric, resourceAttrs pcommon.M
return nil, errUnknownMetricType
}

func (c *collector) getMetricMetadata(metric pmetric.Metric, attributes pcommon.Map, resourceAttrs pcommon.Map) (*prometheus.Desc, []string) {
func (c *collector) getMetricMetadata(metric pmetric.Metric, mType *dto.MetricType, attributes pcommon.Map, resourceAttrs pcommon.Map) (*prometheus.Desc, []string, error) {
name := prometheustranslator.BuildCompliantName(metric, c.namespace, c.addMetricSuffixes)
help, err := c.validateMetrics(name, metric.Description(), mType)
if err != nil {
return nil, nil, err
}

keys := make([]string, 0, attributes.Len()+2) // +2 for job and instance labels.
values := make([]string, 0, attributes.Len()+2)

Expand All @@ -124,18 +134,17 @@ func (c *collector) getMetricMetadata(metric pmetric.Metric, attributes pcommon.
values = append(values, instance)
}

return prometheus.NewDesc(
prometheustranslator.BuildCompliantName(metric, c.namespace, c.addMetricSuffixes),
metric.Description(),
keys,
c.constLabels,
), values
return prometheus.NewDesc(name, help, keys, c.constLabels), values, nil
}

func (c *collector) convertGauge(metric pmetric.Metric, resourceAttrs pcommon.Map) (prometheus.Metric, error) {
ip := metric.Gauge().DataPoints().At(0)

desc, attributes := c.getMetricMetadata(metric, ip.Attributes(), resourceAttrs)
desc, attributes, err := c.getMetricMetadata(metric, dto.MetricType_GAUGE.Enum(), ip.Attributes(), resourceAttrs)
if err != nil {
return nil, err
}

var value float64
switch ip.ValueType() {
case pmetric.NumberDataPointValueTypeInt:
Expand Down Expand Up @@ -163,11 +172,16 @@ func (c *collector) convertSum(metric pmetric.Metric, resourceAttrs pcommon.Map)
ip := metric.Sum().DataPoints().At(0)

metricType := prometheus.GaugeValue
mType := dto.MetricType_GAUGE.Enum()
if metric.Sum().IsMonotonic() {
metricType = prometheus.CounterValue
mType = dto.MetricType_COUNTER.Enum()
}

desc, attributes := c.getMetricMetadata(metric, ip.Attributes(), resourceAttrs)
desc, attributes, err := c.getMetricMetadata(metric, mType, ip.Attributes(), resourceAttrs)
if err != nil {
return nil, err
}
var value float64
switch ip.ValueType() {
case pmetric.NumberDataPointValueTypeInt:
Expand All @@ -183,7 +197,6 @@ func (c *collector) convertSum(metric pmetric.Metric, resourceAttrs pcommon.Map)
}

var m prometheus.Metric
var err error
if metricType == prometheus.CounterValue && ip.StartTimestamp().AsTime().Unix() > 0 {
m, err = prometheus.NewConstMetricWithCreatedTimestamp(desc, metricType, value, ip.StartTimestamp().AsTime(), attributes...)
} else {
Expand Down Expand Up @@ -219,9 +232,11 @@ func (c *collector) convertSummary(metric pmetric.Metric, resourceAttrs pcommon.
quantiles[qvj.Quantile()] = qvj.Value()
}

desc, attributes := c.getMetricMetadata(metric, point.Attributes(), resourceAttrs)
desc, attributes, err := c.getMetricMetadata(metric, dto.MetricType_SUMMARY.Enum(), point.Attributes(), resourceAttrs)
if err != nil {
return nil, err
}
var m prometheus.Metric
var err error
if point.StartTimestamp().AsTime().Unix() > 0 {
m, err = prometheus.NewConstSummaryWithCreatedTimestamp(desc, point.Count(), point.Sum(), quantiles, point.StartTimestamp().AsTime(), attributes...)
} else {
Expand All @@ -238,7 +253,10 @@ func (c *collector) convertSummary(metric pmetric.Metric, resourceAttrs pcommon.

func (c *collector) convertDoubleHistogram(metric pmetric.Metric, resourceAttrs pcommon.Map) (prometheus.Metric, error) {
ip := metric.Histogram().DataPoints().At(0)
desc, attributes := c.getMetricMetadata(metric, ip.Attributes(), resourceAttrs)
desc, attributes, err := c.getMetricMetadata(metric, dto.MetricType_HISTOGRAM.Enum(), ip.Attributes(), resourceAttrs)
if err != nil {
return nil, err
}

indicesMap := make(map[float64]int)
buckets := make([]float64, 0, ip.BucketCounts().Len())
Expand Down Expand Up @@ -267,7 +285,6 @@ func (c *collector) convertDoubleHistogram(metric pmetric.Metric, resourceAttrs
exemplars := convertExemplars(ip.Exemplars())

var m prometheus.Metric
var err error
if ip.StartTimestamp().AsTime().Unix() > 0 {
m, err = prometheus.NewConstHistogramWithCreatedTimestamp(desc, ip.Count(), ip.Sum(), points, ip.StartTimestamp().AsTime(), attributes...)
} else {
Expand Down Expand Up @@ -405,3 +422,28 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
c.logger.Debug(fmt.Sprintf("metric served: %s", m.Desc().String()))
}
}

func (c *collector) validateMetrics(name, description string, metricType *dto.MetricType) (help string, err error) {
emf, exist := c.metricFamilies[name]
if !exist {
c.metricFamilies[name] = &dto.MetricFamily{
Name: proto.String(name),
Help: proto.String(description),
Type: metricType,
}
return "", nil
}
if emf.GetType() != *metricType {
return "", fmt.Errorf("instrument type conflict, using existing type definition. instrument: %s, existing: %s, dropped: %s", name, emf.GetType(), *metricType)
}
if emf.GetHelp() != description {
c.logger.Info(
"Instrument description conflict, using existing",
zap.String("instrument", name),
zap.String("existing", emf.GetHelp()),
zap.String("dropped", description),
)
return emf.GetHelp(), nil
}
return "", nil
}
108 changes: 87 additions & 21 deletions exporter/prometheusexporter/collector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import (
"time"

"github.com/prometheus/client_golang/prometheus"
dto "github.com/prometheus/client_model/go"
io_prometheus_client "github.com/prometheus/client_model/go"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
conventions "go.opentelemetry.io/collector/semconv/v1.25.0"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/timestamppb"

prometheustranslator "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus"
Expand Down Expand Up @@ -47,7 +49,8 @@ func TestConvertInvalidDataType(t *testing.T) {
[]pmetric.Metric{metric},
pcommon.NewMap(),
},
logger: zap.NewNop(),
logger: zap.NewNop(),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

_, err := c.convertMetric(metric, pcommon.NewMap())
Expand All @@ -66,25 +69,82 @@ func TestConvertInvalidDataType(t *testing.T) {
}
}

func TestConvertInvalidMetric(t *testing.T) {
for _, mType := range []pmetric.MetricType{
pmetric.MetricTypeHistogram,
pmetric.MetricTypeSum,
pmetric.MetricTypeGauge,
} {
metric := pmetric.NewMetric()
switch mType {
case pmetric.MetricTypeGauge:
metric.SetEmptyGauge().DataPoints().AppendEmpty()
case pmetric.MetricTypeSum:
metric.SetEmptySum().DataPoints().AppendEmpty()
case pmetric.MetricTypeHistogram:
metric.SetEmptyHistogram().DataPoints().AppendEmpty()
}
c := collector{}
func TestConvertMetric(t *testing.T) {
tests := []struct {
description string
mName string
mType pmetric.MetricType
mfs map[string]*io_prometheus_client.MetricFamily
err bool
}{
{
description: "invalid histogram metric",
mType: pmetric.MetricTypeHistogram,
mfs: make(map[string]*io_prometheus_client.MetricFamily),
err: true,
},
{
description: "invalid sum metric",
mType: pmetric.MetricTypeSum,
mfs: make(map[string]*io_prometheus_client.MetricFamily),
err: true,
},
{
description: "invalid gauge metric",
mType: pmetric.MetricTypeGauge,
mfs: make(map[string]*io_prometheus_client.MetricFamily),
err: true,
},
{
description: "metric type conflict",
mName: "testgauge",
mType: pmetric.MetricTypeGauge,
mfs: map[string]*io_prometheus_client.MetricFamily{
"testgauge": {
Name: proto.String("testgauge"),
Type: dto.MetricType_COUNTER.Enum(),
},
},
err: true,
},
{
description: "metric description conflict",
mName: "testgauge",
mType: pmetric.MetricTypeGauge,
mfs: map[string]*io_prometheus_client.MetricFamily{
"testgauge": {
Name: proto.String("testgauge"),
Type: dto.MetricType_GAUGE.Enum(),
Help: proto.String("test help value"),
},
},
err: false,
},
}
for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
metric := pmetric.NewMetric()
metric.SetName(tt.mName)
switch tt.mType {
case pmetric.MetricTypeGauge:
metric.SetEmptyGauge().DataPoints().AppendEmpty()
case pmetric.MetricTypeSum:
metric.SetEmptySum().DataPoints().AppendEmpty()
case pmetric.MetricTypeHistogram:
metric.SetEmptyHistogram().DataPoints().AppendEmpty()
}
c := collector{
logger: zap.NewNop(),
metricFamilies: tt.mfs,
}

_, err := c.convertMetric(metric, pcommon.NewMap())
require.Error(t, err)
_, err := c.convertMetric(metric, pcommon.NewMap())
if tt.err {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}

Expand Down Expand Up @@ -163,7 +223,8 @@ func TestConvertDoubleHistogramExemplar(t *testing.T) {
metrics: []pmetric.Metric{metric},
resourceAttributes: pMap,
},
logger: zap.NewNop(),
logger: zap.NewNop(),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

pbMetric, _ := c.convertDoubleHistogram(metric, pMap)
Expand Down Expand Up @@ -205,7 +266,8 @@ func TestConvertMonotonicSumExemplar(t *testing.T) {
metrics: []pmetric.Metric{metric},
resourceAttributes: pMap,
},
logger: zap.NewNop(),
logger: zap.NewNop(),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

promMetric, _ := c.convertSum(metric, pMap)
Expand Down Expand Up @@ -260,6 +322,7 @@ func TestCollectMetricsLabelSanitize(t *testing.T) {
},
sendTimestamps: false,
logger: zap.New(&loggerCore),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

ch := make(chan prometheus.Metric, 1)
Expand Down Expand Up @@ -468,6 +531,7 @@ func TestCollectMetrics(t *testing.T) {
},
sendTimestamps: sendTimestamp,
logger: zap.NewNop(),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

ch := make(chan prometheus.Metric, 1)
Expand Down Expand Up @@ -591,6 +655,7 @@ func TestAccumulateHistograms(t *testing.T) {
},
sendTimestamps: sendTimestamp,
logger: zap.NewNop(),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

ch := make(chan prometheus.Metric, 1)
Expand Down Expand Up @@ -701,6 +766,7 @@ func TestAccumulateSummary(t *testing.T) {
},
sendTimestamps: sendTimestamp,
logger: zap.NewNop(),
metricFamilies: make(map[string]*io_prometheus_client.MetricFamily),
}

ch := make(chan prometheus.Metric, 1)
Expand Down

0 comments on commit 9b54425

Please sign in to comment.