From ca06bbe2536d9f40c5c1da171a272564a0784b3d Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 26 Jul 2023 20:19:52 +0000 Subject: [PATCH] do not append _total if the metric already ends in _total --- CHANGELOG.md | 1 + exporters/prometheus/exporter.go | 42 +++++++++++++++------------ exporters/prometheus/exporter_test.go | 29 ++++++++++++++++++ 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 691ed9e8c4f8..2c446acd2891 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - If a Reader's AggregationSelector return nil or DefaultAggregation the pipeline will use the default aggregation. (#4350) - Log a suggested view that fixes instrument conflicts in `go.opentelemetry.io/otel/sdk/metric`. (#4349) - Fix possible panic, deadlock and race condition in batch span processor in `go.opentelemetry.io/otel/sdk/trace`. (#4353) +- Do not append _total if the counter already ends in total `go.opentelemetry.io/otel/exporter/prometheus`. (#4373) ## [1.16.0/0.39.0] 2023-05-18 diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 04f26e7cb4af..b6d3fa27341d 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -189,10 +189,11 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { } for _, m := range scopeMetrics.Metrics { - typ, name := c.metricTypeAndName(m) + typ := c.metricType(m) if typ == nil { continue } + name := c.getName(m, typ) drop, help := c.validateMetrics(name, m.Description, typ) if drop { @@ -336,17 +337,23 @@ var unitSuffixes = map[string]string{ } // getName returns the sanitized name, prefixed with the namespace and suffixed with unit. -func (c *collector) getName(m metricdata.Metrics) string { +func (c *collector) getName(m metricdata.Metrics, typ *dto.MetricType) string { name := sanitizeName(m.Name) + addCounterSuffix := !c.withoutCounterSuffixes && *typ == dto.MetricType_COUNTER + if addCounterSuffix { + // Remove the _total suffix here, as we will re-add the total suffix + // later, and it needs to come after the unit suffix. + name = strings.TrimSuffix(name, counterSuffix) + } if c.namespace != "" { name = c.namespace + name } - if c.withoutUnits { - return name - } - if suffix, ok := unitSuffixes[m.Unit]; ok { + if suffix, ok := unitSuffixes[m.Unit]; !c.withoutUnits && ok { name += suffix } + if addCounterSuffix { + name += counterSuffix + } return name } @@ -403,27 +410,24 @@ func sanitizeName(n string) string { return b.String() } -func (c *collector) metricTypeAndName(m metricdata.Metrics) (*dto.MetricType, string) { - name := c.getName(m) - +func (c *collector) metricType(m metricdata.Metrics) *dto.MetricType { switch v := m.Data.(type) { case metricdata.Histogram[int64], metricdata.Histogram[float64]: - return dto.MetricType_HISTOGRAM.Enum(), name + return dto.MetricType_HISTOGRAM.Enum() case metricdata.Sum[float64]: - if v.IsMonotonic && !c.withoutCounterSuffixes { - return dto.MetricType_COUNTER.Enum(), name + counterSuffix + if v.IsMonotonic { + return dto.MetricType_COUNTER.Enum() } - return dto.MetricType_GAUGE.Enum(), name + return dto.MetricType_GAUGE.Enum() case metricdata.Sum[int64]: - if v.IsMonotonic && !c.withoutCounterSuffixes { - return dto.MetricType_COUNTER.Enum(), name + counterSuffix + if v.IsMonotonic { + return dto.MetricType_COUNTER.Enum() } - return dto.MetricType_GAUGE.Enum(), name + return dto.MetricType_GAUGE.Enum() case metricdata.Gauge[int64], metricdata.Gauge[float64]: - return dto.MetricType_GAUGE.Enum(), name + return dto.MetricType_GAUGE.Enum() } - - return nil, "" + return nil } func (c *collector) scopeInfo(scope instrumentation.Scope) (prometheus.Metric, error) { diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index fb234aa1f26b..80eab25d7003 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -71,6 +71,35 @@ func TestPrometheusExporter(t *testing.T) { counter.Add(ctx, 5, otelmetric.WithAttributeSet(attrs2)) }, }, + { + name: "counter that already has a total suffix", + expectedFile: "testdata/counter.txt", + recordMetrics: func(ctx context.Context, meter otelmetric.Meter) { + opt := otelmetric.WithAttributes( + attribute.Key("A").String("B"), + attribute.Key("C").String("D"), + attribute.Key("E").Bool(true), + attribute.Key("F").Int(42), + ) + counter, err := meter.Float64Counter( + "foo.total", + otelmetric.WithDescription("a simple counter"), + otelmetric.WithUnit("ms"), + ) + require.NoError(t, err) + counter.Add(ctx, 5, opt) + counter.Add(ctx, 10.3, opt) + counter.Add(ctx, 9, opt) + + attrs2 := attribute.NewSet( + attribute.Key("A").String("D"), + attribute.Key("C").String("B"), + attribute.Key("E").Bool(true), + attribute.Key("F").Int(42), + ) + counter.Add(ctx, 5, otelmetric.WithAttributeSet(attrs2)) + }, + }, { name: "counter with suffixes disabled", expectedFile: "testdata/counter_disabled_suffix.txt",