Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit suffixes to prometheus metric names #3352

Merged
merged 3 commits into from Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
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.
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
This can be disabled with the `WithoutUnits()` option. (#3352)

### Fixed

Expand Down
11 changes: 10 additions & 1 deletion exporters/prometheus/confg_test.go
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
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.
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
// 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
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
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
@@ -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
@@ -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
@@ -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