Skip to content

Commit

Permalink
Add unit suffixes to prometheus metric names (#3352)
Browse files Browse the repository at this point in the history
* add unit suffixes to prometheus metric names

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* remove unneccessary variable

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
dashpole and MrAlias authored Oct 19, 2022
1 parent 1d9d4b2 commit 510910e
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 51 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 10 additions & 1 deletion exporters/prometheus/confg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 16 additions & 0 deletions exporters/prometheus/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
type config struct {
registerer prometheus.Registerer
disableTargetInfo bool
withoutUnits bool
aggregation metric.AggregationSelector
}

Expand Down Expand Up @@ -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
})
}
43 changes: 32 additions & 11 deletions exporters/prometheus/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -50,6 +51,7 @@ type collector struct {
reader metric.Reader

disableTargetInfo bool
withoutUnits bool
targetInfo *metricData
createTargetInfoOnce sync.Once
}
Expand All @@ -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 {
Expand Down Expand Up @@ -151,28 +154,28 @@ 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))...)
}
}
}

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)
Expand All @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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 = '_'
Expand Down
47 changes: 30 additions & 17 deletions exporters/prometheus/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}{
{
Expand All @@ -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...)
Expand All @@ -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...)
},
},
{
Expand All @@ -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...)
Expand All @@ -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
Expand All @@ -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...)
Expand Down Expand Up @@ -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"),
Expand All @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions exporters/prometheus/testdata/counter.txt
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions exporters/prometheus/testdata/gauge.txt
Original file line number Diff line number Diff line change
@@ -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
30 changes: 15 additions & 15 deletions exporters/prometheus/testdata/histogram.txt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 510910e

Please sign in to comment.