From 510910e92d5cc8d18c38e4cc88f1bc7c48f6465f Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 19 Oct 2022 15:25:18 -0400 Subject: [PATCH] Add unit suffixes to prometheus metric names (#3352) * add unit suffixes to prometheus metric names * Update CHANGELOG.md Co-authored-by: Tyler Yahn * remove unneccessary variable Co-authored-by: Tyler Yahn --- CHANGELOG.md | 3 +- exporters/prometheus/confg_test.go | 11 ++++- exporters/prometheus/config.go | 16 +++++++ exporters/prometheus/exporter.go | 43 ++++++++++++++----- exporters/prometheus/exporter_test.go | 47 +++++++++++++-------- exporters/prometheus/testdata/counter.txt | 6 +-- exporters/prometheus/testdata/gauge.txt | 6 +-- exporters/prometheus/testdata/histogram.txt | 30 ++++++------- 8 files changed, 111 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d56a2f5f579..cb9993a6fc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `"go.opentelemetry.io/otel/exporters/prometheus".New` now also returns an error indicating the failure to register the exporter with Prometheus. (#3239) - The prometheus exporter will no longer try to enumerate the metrics it will send to prometheus on startup. This fixes the `reader is not registered` warning currently emitted on startup. (#3291 #3342) -- The `go.opentelemetry.io/otel/exporters/prometheus` exporter now correctly adds _total suffixes to counter metrics. (#3360) +- The `go.opentelemetry.io/otel/exporters/prometheus` exporter now adds a unit suffix to metric names. + This can be disabled with the `WithoutUnits()` option. (#3352) ### Fixed diff --git a/exporters/prometheus/confg_test.go b/exporters/prometheus/confg_test.go index 84b55f0221b..0f3278c61df 100644 --- a/exporters/prometheus/confg_test.go +++ b/exporters/prometheus/confg_test.go @@ -89,11 +89,20 @@ func TestNewConfig(t *testing.T) { disableTargetInfo: true, }, }, + { + name: "unit suffixes disabled", + options: []Option{ + WithoutUnits(), + }, + wantConfig: config{ + registerer: prometheus.DefaultRegisterer, + withoutUnits: true, + }, + }, } for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { cfg := newConfig(tt.options...) - // tested by TestConfigManualReaderOptions cfg.aggregation = nil diff --git a/exporters/prometheus/config.go b/exporters/prometheus/config.go index 3079278da33..31b34ccf426 100644 --- a/exporters/prometheus/config.go +++ b/exporters/prometheus/config.go @@ -24,6 +24,7 @@ import ( type config struct { registerer prometheus.Registerer disableTargetInfo bool + withoutUnits bool aggregation metric.AggregationSelector } @@ -89,3 +90,18 @@ func WithoutTargetInfo() Option { return cfg }) } + +// WithoutUnits 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 WithoutUnits() Option { + return optionFunc(func(cfg config) config { + cfg.withoutUnits = true + return cfg + }) +} diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 8135578e42d..95c7ef93bb8 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -27,6 +27,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" "go.opentelemetry.io/otel/sdk/resource" @@ -50,6 +51,7 @@ type collector struct { reader metric.Reader disableTargetInfo bool + withoutUnits bool targetInfo *metricData createTargetInfoOnce sync.Once } @@ -70,6 +72,7 @@ func New(opts ...Option) (*Exporter, error) { collector := &collector{ reader: reader, disableTargetInfo: cfg.disableTargetInfo, + withoutUnits: cfg.withoutUnits, } if err := cfg.registerer.Register(collector); err != nil { @@ -151,15 +154,15 @@ func (c *collector) getMetricData(metrics metricdata.ResourceMetrics) []*metricD 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, c.getName(m))...) case metricdata.Sum[int64]: - allMetrics = append(allMetrics, getSumMetricData(v, m)...) + allMetrics = append(allMetrics, getSumMetricData(v, m, c.getName(m))...) case metricdata.Sum[float64]: - allMetrics = append(allMetrics, getSumMetricData(v, m)...) + allMetrics = append(allMetrics, getSumMetricData(v, m, c.getName(m))...) case metricdata.Gauge[int64]: - allMetrics = append(allMetrics, getGaugeMetricData(v, m)...) + allMetrics = append(allMetrics, getGaugeMetricData(v, m, c.getName(m))...) case metricdata.Gauge[float64]: - allMetrics = append(allMetrics, getGaugeMetricData(v, m)...) + allMetrics = append(allMetrics, getGaugeMetricData(v, m, c.getName(m))...) } } } @@ -167,12 +170,12 @@ func (c *collector) getMetricData(metrics metricdata.ResourceMetrics) []*metricD return allMetrics } -func getHistogramMetricData(histogram metricdata.Histogram, m metricdata.Metrics) []*metricData { +func getHistogramMetricData(histogram metricdata.Histogram, m metricdata.Metrics, name string) []*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(name, m.Description, keys, nil) buckets := make(map[float64]uint64, len(dp.Bounds)) cumulativeCount := uint64(0) @@ -194,15 +197,15 @@ 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, name string) []*metricData { valueType := prometheus.CounterValue if !sum.IsMonotonic { valueType = prometheus.GaugeValue } dataPoints := make([]*metricData, 0, len(sum.DataPoints)) for _, dp := range sum.DataPoints { - name := sanitizeName(m.Name) if sum.IsMonotonic { + // Add _total suffix for counters name += counterSuffix } keys, values := getAttrs(dp.Attributes) @@ -219,11 +222,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, name string) []*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(name, m.Description, keys, nil) md := &metricData{ name: m.Name, description: desc, @@ -289,6 +292,24 @@ 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 (c *collector) getName(m metricdata.Metrics) string { + name := sanitizeName(m.Name) + if c.withoutUnits { + return name + } + 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 4c8594038fc..0aa0b3d34d6 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" @@ -39,7 +40,7 @@ func TestPrometheusExporter(t *testing.T) { emptyResource bool customResouceAttrs []attribute.KeyValue recordMetrics func(ctx context.Context, meter otelmetric.Meter) - withoutTargetInfo bool + options []Option expectedFile string }{ { @@ -52,7 +53,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...) @@ -67,10 +72,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...) }, }, { @@ -81,7 +90,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...) @@ -92,6 +105,7 @@ func TestPrometheusExporter(t *testing.T) { { name: "sanitized attributes to labels", expectedFile: "testdata/sanitized_labels.txt", + options: []Option{WithoutUnits()}, recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { attrs := []attribute.KeyValue{ // exact match, value should be overwritten @@ -102,7 +116,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...) @@ -177,9 +196,9 @@ func TestPrometheusExporter(t *testing.T) { }, }, { - name: "without target_info", - withoutTargetInfo: true, - expectedFile: "testdata/without_target_info.txt", + name: "without target_info", + options: []Option{WithoutTargetInfo()}, + expectedFile: "testdata/without_target_info.txt", recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { attrs := []attribute.KeyValue{ attribute.Key("A").String("B"), @@ -200,13 +219,7 @@ func TestPrometheusExporter(t *testing.T) { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() registry := prometheus.NewRegistry() - - opts := []Option{WithRegisterer(registry)} - if tc.withoutTargetInfo { - opts = append(opts, WithoutTargetInfo()) - } - - exporter, err := New(opts...) + 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 7142094afe6..e8d7f654440 100755 --- a/exporters/prometheus/testdata/counter.txt +++ b/exporters/prometheus/testdata/counter.txt @@ -1,6 +1,6 @@ -# HELP foo_total a simple counter -# TYPE foo_total counter -foo_total{A="B",C="D",E="true",F="42"} 24.3 +# HELP foo_milliseconds_total a simple counter +# TYPE foo_milliseconds_total counter +foo_milliseconds_total{A="B",C="D",E="true",F="42"} 24.3 # HELP target_info Target metadata # TYPE target_info gauge target_info{service_name="prometheus_test",telemetry_sdk_language="go",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="latest"} 1 diff --git a/exporters/prometheus/testdata/gauge.txt b/exporters/prometheus/testdata/gauge.txt index f13629508ab..cd75e5a6507 100644 --- a/exporters/prometheus/testdata/gauge.txt +++ b/exporters/prometheus/testdata/gauge.txt @@ -1,6 +1,6 @@ -# HELP bar a fun little gauge -# TYPE bar gauge -bar{A="B",C="D"} 75 +# HELP bar_ratio a fun little gauge +# TYPE bar_ratio gauge +bar_ratio{A="B",C="D"} .75 # HELP target_info Target metadata # TYPE target_info gauge target_info{service_name="prometheus_test",telemetry_sdk_language="go",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="latest"} 1 diff --git a/exporters/prometheus/testdata/histogram.txt b/exporters/prometheus/testdata/histogram.txt index 495aa6f5160..3f4a1366049 100644 --- a/exporters/prometheus/testdata/histogram.txt +++ b/exporters/prometheus/testdata/histogram.txt @@ -1,18 +1,18 @@ -# 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 # HELP target_info Target metadata # TYPE target_info gauge target_info{service_name="prometheus_test",telemetry_sdk_language="go",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="latest"} 1