From 647a8419b62dd4195b214912e16df06790c276dd Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Tue, 18 Oct 2022 00:01:25 +0000 Subject: [PATCH] add unit suffixes to prometheus metric names --- CHANGELOG.md | 1 + exporters/prometheus/confg_test.go | 34 ++++++++++---- exporters/prometheus/config.go | 18 ++++++- exporters/prometheus/exporter.go | 52 ++++++++++++++------- exporters/prometheus/exporter_test.go | 34 +++++++++++--- exporters/prometheus/testdata/counter.txt | 6 +-- exporters/prometheus/testdata/gauge.txt | 6 +-- exporters/prometheus/testdata/histogram.txt | 30 ++++++------ 8 files changed, 127 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index baab4d165f4..a3fb6db9b3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `sdktrace.TraceProvider.Shutdown` and `sdktrace.TraceProvider.ForceFlush` to not return error when no processor register. (#3268) - The `"go.opentelemetry.io/otel/exporters/prometheus".New` now also returns an error indicating the failure to register the exporter with Prometheus. (#3239) +- The `go.opentelemetry.io/otel/exporters/prometheus` exporter now adds a unit suffix to metric names. This can be disabled with the IgnoreUnits() option (#3352) ### Fixed diff --git a/exporters/prometheus/confg_test.go b/exporters/prometheus/confg_test.go index 91893d40f3a..525de2e79af 100644 --- a/exporters/prometheus/confg_test.go +++ b/exporters/prometheus/confg_test.go @@ -25,14 +25,16 @@ func TestNewConfig(t *testing.T) { registry := prometheus.NewRegistry() testCases := []struct { - name string - options []Option - wantRegisterer prometheus.Registerer + name string + options []Option + wantConfig config }{ { - name: "Default", - options: nil, - wantRegisterer: prometheus.DefaultRegisterer, + name: "Default", + options: nil, + wantConfig: config{ + registerer: prometheus.DefaultRegisterer, + }, }, { @@ -40,21 +42,35 @@ func TestNewConfig(t *testing.T) { options: []Option{ WithRegisterer(registry), }, - wantRegisterer: registry, + wantConfig: config{ + registerer: registry, + }, }, { name: "nil options do nothing", options: []Option{ WithRegisterer(nil), }, - wantRegisterer: prometheus.DefaultRegisterer, + wantConfig: config{ + registerer: prometheus.DefaultRegisterer, + }, + }, + { + name: "unit suffixes disabled", + options: []Option{ + IgnoreUnits(), + }, + wantConfig: config{ + registerer: prometheus.DefaultRegisterer, + ignoreUnits: true, + }, }, } for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { cfg := newConfig(tt.options...) - assert.Equal(t, tt.wantRegisterer, cfg.registerer) + assert.Equal(t, tt.wantConfig, cfg) }) } } diff --git a/exporters/prometheus/config.go b/exporters/prometheus/config.go index 6ee84732556..5d386d9c212 100644 --- a/exporters/prometheus/config.go +++ b/exporters/prometheus/config.go @@ -20,7 +20,8 @@ import ( // config contains options for the exporter. type config struct { - registerer prometheus.Registerer + registerer prometheus.Registerer + ignoreUnits bool } // newConfig creates a validated config configured with options. @@ -57,3 +58,18 @@ func WithRegisterer(reg prometheus.Registerer) Option { return cfg }) } + +// IgnoreUnits disables exporter's addition of unit suffixes to metric names, +// and will also prevent unit comments from being added in OpenMetrics once +// unit comments are supported. +// +// By default, metric names include a unit suffix to follow Prometheus naming +// conventions. For example, the counter metric request.duration, with unit +// milliseconds would become request_duration_milliseconds_total. +// With this option set, the name would instead be request_duration_total. +func IgnoreUnits() Option { + return optionFunc(func(cfg config) config { + cfg.ignoreUnits = true + return cfg + }) +} diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 007dc2f50d9..d8cf0538eb6 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -26,6 +26,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -40,7 +41,8 @@ var _ metric.Reader = &Exporter{} // collector is used to implement prometheus.Collector. type collector struct { - reader metric.Reader + reader metric.Reader + ignoreUnits bool } // New returns a Prometheus Exporter. @@ -53,7 +55,8 @@ func New(opts ...Option) (*Exporter, error) { reader := metric.NewManualReader() collector := &collector{ - reader: reader, + reader: reader, + ignoreUnits: cfg.ignoreUnits, } if err := cfg.registerer.Register(collector); err != nil { @@ -73,7 +76,7 @@ func (c *collector) Describe(ch chan<- *prometheus.Desc) { if err != nil { otel.Handle(err) } - for _, metricData := range getMetricData(metrics) { + for _, metricData := range getMetricData(metrics, c.ignoreUnits) { ch <- metricData.description } } @@ -87,7 +90,7 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { // TODO(#3166): convert otel resource to target_info // see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#resource-attributes-1 - for _, metricData := range getMetricData(metrics) { + for _, metricData := range getMetricData(metrics, c.ignoreUnits) { if metricData.valueType == prometheus.UntypedValue { m, err := prometheus.NewConstHistogram(metricData.description, metricData.histogramCount, metricData.histogramSum, metricData.histogramBuckets, metricData.attributeValues...) if err != nil { @@ -120,21 +123,21 @@ type metricData struct { histogramBuckets map[float64]uint64 } -func getMetricData(metrics metricdata.ResourceMetrics) []*metricData { +func getMetricData(metrics metricdata.ResourceMetrics, ignoreUnits bool) []*metricData { allMetrics := make([]*metricData, 0) for _, scopeMetrics := range metrics.ScopeMetrics { for _, m := range scopeMetrics.Metrics { switch v := m.Data.(type) { case metricdata.Histogram: - allMetrics = append(allMetrics, getHistogramMetricData(v, m)...) + allMetrics = append(allMetrics, getHistogramMetricData(v, m, ignoreUnits)...) case metricdata.Sum[int64]: - allMetrics = append(allMetrics, getSumMetricData(v, m)...) + allMetrics = append(allMetrics, getSumMetricData(v, m, ignoreUnits)...) case metricdata.Sum[float64]: - allMetrics = append(allMetrics, getSumMetricData(v, m)...) + allMetrics = append(allMetrics, getSumMetricData(v, m, ignoreUnits)...) case metricdata.Gauge[int64]: - allMetrics = append(allMetrics, getGaugeMetricData(v, m)...) + allMetrics = append(allMetrics, getGaugeMetricData(v, m, ignoreUnits)...) case metricdata.Gauge[float64]: - allMetrics = append(allMetrics, getGaugeMetricData(v, m)...) + allMetrics = append(allMetrics, getGaugeMetricData(v, m, ignoreUnits)...) } } } @@ -142,12 +145,12 @@ func getMetricData(metrics metricdata.ResourceMetrics) []*metricData { return allMetrics } -func getHistogramMetricData(histogram metricdata.Histogram, m metricdata.Metrics) []*metricData { +func getHistogramMetricData(histogram metricdata.Histogram, m metricdata.Metrics, ignoreUnits bool) []*metricData { // TODO(https://github.com/open-telemetry/opentelemetry-go/issues/3163): support exemplars dataPoints := make([]*metricData, 0, len(histogram.DataPoints)) for _, dp := range histogram.DataPoints { keys, values := getAttrs(dp.Attributes) - desc := prometheus.NewDesc(sanitizeName(m.Name), m.Description, keys, nil) + desc := prometheus.NewDesc(getName(m, ignoreUnits), m.Description, keys, nil) buckets := make(map[float64]uint64, len(dp.Bounds)) cumulativeCount := uint64(0) @@ -169,11 +172,11 @@ func getHistogramMetricData(histogram metricdata.Histogram, m metricdata.Metrics return dataPoints } -func getSumMetricData[N int64 | float64](sum metricdata.Sum[N], m metricdata.Metrics) []*metricData { +func getSumMetricData[N int64 | float64](sum metricdata.Sum[N], m metricdata.Metrics, ignoreUnits bool) []*metricData { dataPoints := make([]*metricData, 0, len(sum.DataPoints)) for _, dp := range sum.DataPoints { keys, values := getAttrs(dp.Attributes) - desc := prometheus.NewDesc(sanitizeName(m.Name), m.Description, keys, nil) + desc := prometheus.NewDesc(getName(m, ignoreUnits), m.Description, keys, nil) md := &metricData{ name: m.Name, description: desc, @@ -186,11 +189,11 @@ func getSumMetricData[N int64 | float64](sum metricdata.Sum[N], m metricdata.Met return dataPoints } -func getGaugeMetricData[N int64 | float64](gauge metricdata.Gauge[N], m metricdata.Metrics) []*metricData { +func getGaugeMetricData[N int64 | float64](gauge metricdata.Gauge[N], m metricdata.Metrics, ignoreUnits bool) []*metricData { dataPoints := make([]*metricData, 0, len(gauge.DataPoints)) for _, dp := range gauge.DataPoints { keys, values := getAttrs(dp.Attributes) - desc := prometheus.NewDesc(sanitizeName(m.Name), m.Description, keys, nil) + desc := prometheus.NewDesc(getName(m, ignoreUnits), m.Description, keys, nil) md := &metricData{ name: m.Name, description: desc, @@ -239,6 +242,23 @@ func sanitizeRune(r rune) rune { return '_' } +var unitSuffixes = map[unit.Unit]string{ + unit.Dimensionless: "_ratio", + unit.Bytes: "_bytes", + unit.Milliseconds: "_milliseconds", +} + +// getName returns the sanitized name, including unit suffix +func getName(m metricdata.Metrics, ignoreUnits bool) string { + name := sanitizeName(m.Name) + if !ignoreUnits { + if suffix, ok := unitSuffixes[m.Unit]; ok { + name += suffix + } + } + return name +} + func sanitizeName(n string) string { // This algorithm is based on strings.Map from Go 1.19. const replacement = '_' diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index 5a1a65ce58d..4a3bdaaa8a6 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -26,6 +26,7 @@ import ( "go.opentelemetry.io/otel/attribute" otelmetric "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/view" @@ -35,6 +36,7 @@ func TestPrometheusExporter(t *testing.T) { testCases := []struct { name string recordMetrics func(ctx context.Context, meter otelmetric.Meter) + options []Option expectedFile string }{ { @@ -47,7 +49,11 @@ func TestPrometheusExporter(t *testing.T) { attribute.Key("E").Bool(true), attribute.Key("F").Int(42), } - counter, err := meter.SyncFloat64().Counter("foo", instrument.WithDescription("a simple counter")) + counter, err := meter.SyncFloat64().Counter( + "foo", + instrument.WithDescription("a simple counter"), + instrument.WithUnit(unit.Milliseconds), + ) require.NoError(t, err) counter.Add(ctx, 5, attrs...) counter.Add(ctx, 10.3, attrs...) @@ -62,10 +68,14 @@ func TestPrometheusExporter(t *testing.T) { attribute.Key("A").String("B"), attribute.Key("C").String("D"), } - gauge, err := meter.SyncFloat64().UpDownCounter("bar", instrument.WithDescription("a fun little gauge")) + gauge, err := meter.SyncFloat64().UpDownCounter( + "bar", + instrument.WithDescription("a fun little gauge"), + instrument.WithUnit(unit.Dimensionless), + ) require.NoError(t, err) - gauge.Add(ctx, 100, attrs...) - gauge.Add(ctx, -25, attrs...) + gauge.Add(ctx, 1.0, attrs...) + gauge.Add(ctx, -.25, attrs...) }, }, { @@ -76,7 +86,11 @@ func TestPrometheusExporter(t *testing.T) { attribute.Key("A").String("B"), attribute.Key("C").String("D"), } - histogram, err := meter.SyncFloat64().Histogram("histogram_baz", instrument.WithDescription("a very nice histogram")) + histogram, err := meter.SyncFloat64().Histogram( + "histogram_baz", + instrument.WithDescription("a very nice histogram"), + instrument.WithUnit(unit.Bytes), + ) require.NoError(t, err) histogram.Record(ctx, 23, attrs...) histogram.Record(ctx, 7, attrs...) @@ -87,6 +101,7 @@ func TestPrometheusExporter(t *testing.T) { { name: "sanitized attributes to labels", expectedFile: "testdata/sanitized_labels.txt", + options: []Option{IgnoreUnits()}, recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { attrs := []attribute.KeyValue{ // exact match, value should be overwritten @@ -97,7 +112,12 @@ func TestPrometheusExporter(t *testing.T) { attribute.Key("C.D").String("Y"), attribute.Key("C/D").String("Z"), } - counter, err := meter.SyncFloat64().Counter("foo", instrument.WithDescription("a sanitary counter")) + counter, err := meter.SyncFloat64().Counter( + "foo", + instrument.WithDescription("a sanitary counter"), + // This unit is not added to + instrument.WithUnit(unit.Bytes), + ) require.NoError(t, err) counter.Add(ctx, 5, attrs...) counter.Add(ctx, 10.3, attrs...) @@ -139,7 +159,7 @@ func TestPrometheusExporter(t *testing.T) { ctx := context.Background() registry := prometheus.NewRegistry() - exporter, err := New(WithRegisterer(registry)) + exporter, err := New(append(tc.options, WithRegisterer(registry))...) require.NoError(t, err) customBucketsView, err := view.New( diff --git a/exporters/prometheus/testdata/counter.txt b/exporters/prometheus/testdata/counter.txt index 93776d2372b..69e463ee90c 100755 --- a/exporters/prometheus/testdata/counter.txt +++ b/exporters/prometheus/testdata/counter.txt @@ -1,3 +1,3 @@ -# HELP foo a simple counter -# TYPE foo counter -foo{A="B",C="D",E="true",F="42"} 24.3 +# HELP foo_milliseconds a simple counter +# TYPE foo_milliseconds counter +foo_milliseconds{A="B",C="D",E="true",F="42"} 24.3 diff --git a/exporters/prometheus/testdata/gauge.txt b/exporters/prometheus/testdata/gauge.txt index 889295d74e1..3183c00a1ca 100644 --- a/exporters/prometheus/testdata/gauge.txt +++ b/exporters/prometheus/testdata/gauge.txt @@ -1,3 +1,3 @@ -# HELP bar a fun little gauge -# TYPE bar counter -bar{A="B",C="D"} 75 +# HELP bar_ratio a fun little gauge +# TYPE bar_ratio counter +bar_ratio{A="B",C="D"} .75 diff --git a/exporters/prometheus/testdata/histogram.txt b/exporters/prometheus/testdata/histogram.txt index 95f202154a4..cd55e3eae84 100644 --- a/exporters/prometheus/testdata/histogram.txt +++ b/exporters/prometheus/testdata/histogram.txt @@ -1,15 +1,15 @@ -# HELP histogram_baz a very nice histogram -# TYPE histogram_baz histogram -histogram_baz_bucket{A="B",C="D",le="0"} 0 -histogram_baz_bucket{A="B",C="D",le="5"} 0 -histogram_baz_bucket{A="B",C="D",le="10"} 1 -histogram_baz_bucket{A="B",C="D",le="25"} 2 -histogram_baz_bucket{A="B",C="D",le="50"} 2 -histogram_baz_bucket{A="B",C="D",le="75"} 2 -histogram_baz_bucket{A="B",C="D",le="100"} 2 -histogram_baz_bucket{A="B",C="D",le="250"} 4 -histogram_baz_bucket{A="B",C="D",le="500"} 4 -histogram_baz_bucket{A="B",C="D",le="1000"} 4 -histogram_baz_bucket{A="B",C="D",le="+Inf"} 4 -histogram_baz_sum{A="B",C="D"} 236 -histogram_baz_count{A="B",C="D"} 4 +# HELP histogram_baz_bytes a very nice histogram +# TYPE histogram_baz_bytes histogram +histogram_baz_bytes_bucket{A="B",C="D",le="0"} 0 +histogram_baz_bytes_bucket{A="B",C="D",le="5"} 0 +histogram_baz_bytes_bucket{A="B",C="D",le="10"} 1 +histogram_baz_bytes_bucket{A="B",C="D",le="25"} 2 +histogram_baz_bytes_bucket{A="B",C="D",le="50"} 2 +histogram_baz_bytes_bucket{A="B",C="D",le="75"} 2 +histogram_baz_bytes_bucket{A="B",C="D",le="100"} 2 +histogram_baz_bytes_bucket{A="B",C="D",le="250"} 4 +histogram_baz_bytes_bucket{A="B",C="D",le="500"} 4 +histogram_baz_bytes_bucket{A="B",C="D",le="1000"} 4 +histogram_baz_bytes_bucket{A="B",C="D",le="+Inf"} 4 +histogram_baz_bytes_sum{A="B",C="D"} 236 +histogram_baz_bytes_count{A="B",C="D"} 4