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

Prometheus exporter: do not append _total if the metric already ends in _total #4373

Merged
merged 1 commit into from
Aug 7, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Decouple `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal` from `go.opentelemetry.io/otel/exporters/otlp/internal` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal` using gotmpl. (#4400, #3846)
- Decouple `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp/internal` from `go.opentelemetry.io/otel/exporters/otlp/internal` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal` using gotmpl. (#4401, #3846)
- Do not block the metric SDK when OTLP metric exports are blocked in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#3925, #4395)
- Do not append _total if the counter already ends in total `go.opentelemetry.io/otel/exporter/prometheus`. (#4373)
dashpole marked this conversation as resolved.
Show resolved Hide resolved

## [1.16.0/0.39.0] 2023-05-18

Expand Down
42 changes: 23 additions & 19 deletions exporters/prometheus/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,11 @@
}

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 {
Expand Down Expand Up @@ -366,17 +367,23 @@
}

// 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
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
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 && !strings.HasSuffix(name, suffix) {
if suffix, ok := unitSuffixes[m.Unit]; ok && !c.withoutUnits && !strings.HasSuffix(name, suffix) {
name += suffix
}
if addCounterSuffix {
name += counterSuffix
}
return name
}

Expand Down Expand Up @@ -433,27 +440,24 @@
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()

Check warning on line 458 in exporters/prometheus/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/exporter.go#L458

Added line #L458 was not covered by tests
}

return nil, ""
return nil

Check warning on line 460 in exporters/prometheus/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/exporter.go#L460

Added line #L460 was not covered by tests
}

func (c *collector) scopeInfo(scope instrumentation.Scope) (prometheus.Metric, error) {
Expand Down
29 changes: 29 additions & 0 deletions exporters/prometheus/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,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("s"),
)
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",
Expand Down