Skip to content

Commit 59a7a6d

Browse files
zeitlingerzeitlinger
authored
fix: Preserve original metric names in OM2 output (#2058)
Refs #1954. ## Summary This fixes OM2 name preservation in the text writer. - Use original metric metadata names in OM2 output instead of OM1 exposition base names. - Preserve original names for default classic histogram output instead of delegating that path to OM1. - Keep the OM2 `_info` exception for info metrics. - Add builder-based regression coverage for counters and classic/native histograms with `.unit(...)`. ## Why `openmetrics2.enabled=true` is documented as preserving metric names as written by the application. The writer still used OM1-derived names in several paths, and classic histograms delegated to the OM1 writer, so `.unit(...)` could still leak unit-suffixed names into OM2 output. ## Smoking gun `prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/OpenMetrics2TextFormatWriterTest.java` adds `counterPreservesOriginalNameWhenUnitIsConfigured()`. That test builds a real counter through the public builder API: ```java Counter.builder().name("my_counter").unit(Unit.SECONDS) ``` It then asserts that OM1 still emits the legacy name `my_counter_seconds_total`, while OM2 emits `my_counter` and does not contain `my_counter_seconds`. I verified this test fails against pre-fix `origin/main` when only the test is applied: ```text OpenMetrics2TextFormatWriterTest.counterPreservesOriginalNameWhenUnitIsConfigured Expecting actual: "# TYPE my_counter_seconds counter # UNIT my_counter_seconds seconds # HELP my_counter_seconds Test counter my_counter_seconds{method="GET"} 42.0 st@... # EOF " to contain: "# TYPE my_counter counter " ``` That is the bug this PR fixes: OM2 was still using the OM1 unit-suffixed metric name. ## Validation - `./mvnw test -pl prometheus-metrics-core,prometheus-metrics-exposition-textformats -am -Dtest=OpenMetrics2TextFormatWriterTest -Dcoverage.skip=true -Dcheckstyle.skip=true -Dsurefire.failIfNoSpecifiedTests=false` - `mise run build` - `mise run lint` - `git diff --check` Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
1 parent 3cbc665 commit 59a7a6d

4 files changed

Lines changed: 224 additions & 83 deletions

File tree

‎prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/OpenMetrics2TextFormatWriterTest.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package io.prometheus.metrics.core.metrics;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import io.prometheus.metrics.config.EscapingScheme;
6+
import io.prometheus.metrics.config.OpenMetrics2Properties;
7+
import io.prometheus.metrics.expositionformats.ExpositionFormatWriter;
8+
import io.prometheus.metrics.expositionformats.OpenMetrics2TextFormatWriter;
9+
import io.prometheus.metrics.expositionformats.OpenMetricsTextFormatWriter;
10+
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
11+
import io.prometheus.metrics.model.snapshots.Unit;
12+
import java.io.ByteArrayOutputStream;
13+
import java.io.IOException;
14+
import java.nio.charset.StandardCharsets;
15+
import org.junit.jupiter.api.Test;
16+
17+
class OpenMetrics2TextFormatWriterTest {
18+
19+
@Test
20+
void counterPreservesOriginalNameWhenUnitIsConfigured() throws IOException {
21+
Counter counter =
22+
Counter.builder()
23+
.name("my_counter")
24+
.unit(Unit.SECONDS)
25+
.help("Test counter")
26+
.labelNames("method")
27+
.build();
28+
counter.labelValues("GET").inc(42.0);
29+
MetricSnapshots snapshots = MetricSnapshots.of(counter.collect());
30+
31+
String om1Output = writeWithOM1(snapshots);
32+
String om2Output = writeWithOM2(snapshots);
33+
34+
assertThat(om1Output).contains("my_counter_seconds_total{method=\"GET\"} 42.0");
35+
assertThat(om2Output)
36+
.contains("# TYPE my_counter counter\n")
37+
.contains("# UNIT my_counter seconds\n")
38+
.contains("# HELP my_counter Test counter\n")
39+
.containsPattern("(?m)^my_counter\\{method=\"GET\"} 42\\.0 st@\\d+\\.\\d{3}$")
40+
.doesNotContain("my_counter_seconds");
41+
}
42+
43+
@Test
44+
void classicHistogramPreservesOriginalNameWhenUnitIsConfigured() throws IOException {
45+
Histogram histogram =
46+
Histogram.builder()
47+
.name("request_duration")
48+
.unit(Unit.SECONDS)
49+
.help("Request duration in seconds")
50+
.labelNames("path")
51+
.classicOnly()
52+
.classicUpperBounds(10.0)
53+
.build();
54+
histogram.labelValues("/hello").observe(3.2);
55+
MetricSnapshots snapshots = MetricSnapshots.of(histogram.collect());
56+
57+
String om1Output = writeWithOM1(snapshots);
58+
String om2Output = writeWithOM2(snapshots);
59+
60+
assertThat(om1Output).contains("request_duration_seconds_bucket");
61+
assertThat(om2Output)
62+
.contains("# TYPE request_duration histogram\n")
63+
.contains("# UNIT request_duration seconds\n")
64+
.contains("# HELP request_duration Request duration in seconds\n")
65+
.contains("request_duration_bucket{path=\"/hello\",le=\"10.0\"} 1\n")
66+
.contains("request_duration_bucket{path=\"/hello\",le=\"+Inf\"} 1\n")
67+
.contains("request_duration_count{path=\"/hello\"} 1\n")
68+
.contains("request_duration_sum{path=\"/hello\"} 3.2\n")
69+
.doesNotContain("request_duration_seconds");
70+
}
71+
72+
@Test
73+
void nativeHistogramPreservesOriginalNameWhenUnitIsConfigured() throws IOException {
74+
Histogram histogram =
75+
Histogram.builder()
76+
.name("my.request.duration")
77+
.unit(Unit.SECONDS)
78+
.help("Request duration in seconds")
79+
.labelNames("http.path")
80+
.nativeOnly()
81+
.build();
82+
histogram.labelValues("/hello").observe(3.2);
83+
MetricSnapshots snapshots = MetricSnapshots.of(histogram.collect());
84+
85+
String om2Output = writeWithNativeHistograms(snapshots);
86+
87+
assertThat(om2Output)
88+
.contains("# TYPE \"my.request.duration\" histogram\n")
89+
.contains("# UNIT \"my.request.duration\" seconds\n")
90+
.contains("# HELP \"my.request.duration\" Request duration in seconds\n")
91+
.contains("{\"my.request.duration\",\"http.path\"=\"/hello\"} {count:1,sum:3.2,")
92+
.doesNotContain("my.request.duration_seconds");
93+
}
94+
95+
private String writeWithOM1(MetricSnapshots snapshots) throws IOException {
96+
return write(snapshots, OpenMetricsTextFormatWriter.create());
97+
}
98+
99+
private String writeWithOM2(MetricSnapshots snapshots) throws IOException {
100+
return write(snapshots, OpenMetrics2TextFormatWriter.create());
101+
}
102+
103+
private String writeWithNativeHistograms(MetricSnapshots snapshots) throws IOException {
104+
OpenMetrics2TextFormatWriter writer =
105+
OpenMetrics2TextFormatWriter.builder()
106+
.setOpenMetrics2Properties(
107+
OpenMetrics2Properties.builder().nativeHistograms(true).build())
108+
.build();
109+
return write(snapshots, writer);
110+
}
111+
112+
private String write(MetricSnapshots snapshots, ExpositionFormatWriter writer)
113+
throws IOException {
114+
ByteArrayOutputStream out = new ByteArrayOutputStream();
115+
writer.write(out, snapshots, EscapingScheme.ALLOW_UTF8);
116+
return out.toString(StandardCharsets.UTF_8.name());
117+
}
118+
}

‎prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/OpenMetrics2TextFormatWriter.java‎

Lines changed: 97 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import static io.prometheus.metrics.expositionformats.TextFormatUtil.writeLong;
77
import static io.prometheus.metrics.expositionformats.TextFormatUtil.writeName;
88
import static io.prometheus.metrics.expositionformats.TextFormatUtil.writeOpenMetricsTimestamp;
9-
import static io.prometheus.metrics.model.snapshots.SnapshotEscaper.getExpositionBaseMetadataName;
9+
import static io.prometheus.metrics.model.snapshots.SnapshotEscaper.getOriginalMetadataName;
1010
import static io.prometheus.metrics.model.snapshots.SnapshotEscaper.getSnapshotLabelName;
1111

1212
import io.prometheus.metrics.config.EscapingScheme;
@@ -40,8 +40,8 @@
4040

4141
/**
4242
* Write the OpenMetrics 2.0 text format. Unlike the OM1 writer, this writer outputs metric names as
43-
* provided by the user — no {@code _total} or unit suffix appending. The {@code _info} suffix is
44-
* enforced per the OM2 spec (MUST). This is experimental and subject to change as the <a
43+
* provided by the user, without appending {@code _total} or unit suffixes. The {@code _info} suffix
44+
* is enforced per the OM2 spec (MUST). This is experimental and subject to change as the <a
4545
* href="https://github.com/prometheus/docs/blob/main/docs/specs/om/open_metrics_spec_2_0.md">OpenMetrics
4646
* 2.0 specification</a> evolves.
4747
*/
@@ -89,6 +89,7 @@ public OpenMetrics2TextFormatWriter build() {
8989
public static final String CONTENT_TYPE =
9090
"application/openmetrics-text; version=2.0.0; charset=utf-8";
9191
private final OpenMetrics2Properties openMetrics2Properties;
92+
private final boolean createdTimestampsEnabled;
9293
private final boolean exemplarsOnAllMetricTypesEnabled;
9394
private final OpenMetricsTextFormatWriter om1Writer;
9495

@@ -102,6 +103,7 @@ public OpenMetrics2TextFormatWriter(
102103
boolean createdTimestampsEnabled,
103104
boolean exemplarsOnAllMetricTypesEnabled) {
104105
this.openMetrics2Properties = openMetrics2Properties;
106+
this.createdTimestampsEnabled = createdTimestampsEnabled;
105107
this.exemplarsOnAllMetricTypesEnabled = exemplarsOnAllMetricTypesEnabled;
106108
this.om1Writer =
107109
new OpenMetricsTextFormatWriter(createdTimestampsEnabled, exemplarsOnAllMetricTypesEnabled);
@@ -170,8 +172,8 @@ public void write(OutputStream out, MetricSnapshots metricSnapshots, EscapingSch
170172
private void writeCounter(Writer writer, CounterSnapshot snapshot, EscapingScheme scheme)
171173
throws IOException {
172174
MetricMetadata metadata = snapshot.getMetadata();
173-
// OM2: use the name as provided by the user, no _total appending
174-
String counterName = getExpositionBaseMetadataName(metadata, scheme);
175+
// OM2: use the original name, no _total or unit suffix appending.
176+
String counterName = getOriginalMetadataName(metadata, scheme);
175177
writeMetadataWithName(writer, counterName, "counter", metadata);
176178
for (CounterSnapshot.CounterDataPointSnapshot data : snapshot.getDataPoints()) {
177179
writeNameAndLabels(writer, counterName, null, data.getLabels(), scheme);
@@ -192,7 +194,7 @@ private void writeCounter(Writer writer, CounterSnapshot snapshot, EscapingSchem
192194
private void writeGauge(Writer writer, GaugeSnapshot snapshot, EscapingScheme scheme)
193195
throws IOException {
194196
MetricMetadata metadata = snapshot.getMetadata();
195-
String name = getExpositionBaseMetadataName(metadata, scheme);
197+
String name = getOriginalMetadataName(metadata, scheme);
196198
writeMetadataWithName(writer, name, "gauge", metadata);
197199
for (GaugeSnapshot.GaugeDataPointSnapshot data : snapshot.getDataPoints()) {
198200
writeNameAndLabels(writer, name, null, data.getLabels(), scheme);
@@ -209,12 +211,12 @@ private void writeHistogram(Writer writer, HistogramSnapshot snapshot, EscapingS
209211
throws IOException {
210212
boolean compositeHistogram =
211213
openMetrics2Properties.getCompositeValues() || openMetrics2Properties.getNativeHistograms();
214+
MetricMetadata metadata = snapshot.getMetadata();
215+
String name = getOriginalMetadataName(metadata, scheme);
212216
if (!compositeHistogram && !openMetrics2Properties.getExemplarCompliance()) {
213-
om1Writer.writeHistogram(writer, snapshot, scheme);
217+
writeClassicHistogram(writer, name, snapshot, scheme);
214218
return;
215219
}
216-
MetricMetadata metadata = snapshot.getMetadata();
217-
String name = getExpositionBaseMetadataName(metadata, scheme);
218220
if (snapshot.isGaugeHistogram()) {
219221
writeMetadataWithName(writer, name, "gaugehistogram", metadata);
220222
for (HistogramSnapshot.HistogramDataPointSnapshot data : snapshot.getDataPoints()) {
@@ -236,6 +238,88 @@ private void writeHistogram(Writer writer, HistogramSnapshot snapshot, EscapingS
236238
}
237239
}
238240

241+
private void writeClassicHistogram(
242+
Writer writer, String name, HistogramSnapshot snapshot, EscapingScheme scheme)
243+
throws IOException {
244+
if (snapshot.isGaugeHistogram()) {
245+
writeMetadataWithName(writer, name, "gaugehistogram", snapshot.getMetadata());
246+
writeClassicHistogramDataPoints(writer, name, "_gcount", "_gsum", snapshot, scheme);
247+
} else {
248+
writeMetadataWithName(writer, name, "histogram", snapshot.getMetadata());
249+
writeClassicHistogramDataPoints(writer, name, "_count", "_sum", snapshot, scheme);
250+
}
251+
}
252+
253+
private void writeClassicHistogramDataPoints(
254+
Writer writer,
255+
String name,
256+
String countSuffix,
257+
String sumSuffix,
258+
HistogramSnapshot snapshot,
259+
EscapingScheme scheme)
260+
throws IOException {
261+
for (HistogramSnapshot.HistogramDataPointSnapshot data : snapshot.getDataPoints()) {
262+
ClassicHistogramBuckets buckets = getClassicBuckets(data);
263+
Exemplars exemplars = data.getExemplars();
264+
long cumulativeCount = 0;
265+
for (int i = 0; i < buckets.size(); i++) {
266+
cumulativeCount += buckets.getCount(i);
267+
writeNameAndLabels(
268+
writer, name, "_bucket", data.getLabels(), scheme, "le", buckets.getUpperBound(i));
269+
writeLong(writer, cumulativeCount);
270+
Exemplar exemplar;
271+
if (i == 0) {
272+
exemplar = exemplars.get(Double.NEGATIVE_INFINITY, buckets.getUpperBound(i));
273+
} else {
274+
exemplar = exemplars.get(buckets.getUpperBound(i - 1), buckets.getUpperBound(i));
275+
}
276+
writeScrapeTimestampAndExemplar(writer, data, exemplar, scheme);
277+
}
278+
if (data.hasCount() && data.hasSum()) {
279+
writeClassicCountAndSum(writer, name, data, countSuffix, sumSuffix, exemplars, scheme);
280+
}
281+
writeClassicCreated(writer, name, data, scheme);
282+
}
283+
}
284+
285+
private void writeClassicCountAndSum(
286+
Writer writer,
287+
String name,
288+
HistogramSnapshot.HistogramDataPointSnapshot data,
289+
String countSuffix,
290+
String sumSuffix,
291+
Exemplars exemplars,
292+
EscapingScheme scheme)
293+
throws IOException {
294+
writeNameAndLabels(writer, name, countSuffix, data.getLabels(), scheme);
295+
writeLong(writer, data.getCount());
296+
if (exemplarsOnAllMetricTypesEnabled) {
297+
writeScrapeTimestampAndExemplar(writer, data, exemplars.getLatest(), scheme);
298+
} else {
299+
writeScrapeTimestampAndExemplar(writer, data, null, scheme);
300+
}
301+
writeNameAndLabels(writer, name, sumSuffix, data.getLabels(), scheme);
302+
writeDouble(writer, data.getSum());
303+
writeScrapeTimestampAndExemplar(writer, data, null, scheme);
304+
}
305+
306+
private void writeClassicCreated(
307+
Writer writer,
308+
String name,
309+
HistogramSnapshot.HistogramDataPointSnapshot data,
310+
EscapingScheme scheme)
311+
throws IOException {
312+
if (createdTimestampsEnabled && data.hasCreatedTimestamp()) {
313+
writeNameAndLabels(writer, name, "_created", data.getLabels(), scheme);
314+
writeOpenMetricsTimestamp(writer, data.getCreatedTimestampMillis());
315+
if (data.hasScrapeTimestamp()) {
316+
writer.write(' ');
317+
writeOpenMetricsTimestamp(writer, data.getScrapeTimestampMillis());
318+
}
319+
writer.write('\n');
320+
}
321+
}
322+
239323
private void writeCompositeHistogramDataPoint(
240324
Writer writer,
241325
String name,
@@ -398,7 +482,7 @@ private void writeSummary(Writer writer, SummarySnapshot snapshot, EscapingSchem
398482
}
399483
boolean metadataWritten = false;
400484
MetricMetadata metadata = snapshot.getMetadata();
401-
String name = getExpositionBaseMetadataName(metadata, scheme);
485+
String name = getOriginalMetadataName(metadata, scheme);
402486
for (SummarySnapshot.SummaryDataPointSnapshot data : snapshot.getDataPoints()) {
403487
if (data.getQuantiles().size() == 0 && !data.hasCount() && !data.hasSum()) {
404488
continue;
@@ -465,7 +549,7 @@ private void writeInfo(Writer writer, InfoSnapshot snapshot, EscapingScheme sche
465549
MetricMetadata metadata = snapshot.getMetadata();
466550
// OM2 spec: Info MetricFamily name MUST end in _info.
467551
// In OM2, TYPE/HELP use the same name as the data lines.
468-
String infoName = ensureSuffix(getExpositionBaseMetadataName(metadata, scheme), "_info");
552+
String infoName = ensureSuffix(getOriginalMetadataName(metadata, scheme), "_info");
469553
writeMetadataWithName(writer, infoName, "info", metadata);
470554
for (InfoSnapshot.InfoDataPointSnapshot data : snapshot.getDataPoints()) {
471555
writeNameAndLabels(writer, infoName, null, data.getLabels(), scheme);
@@ -477,7 +561,7 @@ private void writeInfo(Writer writer, InfoSnapshot snapshot, EscapingScheme sche
477561
private void writeStateSet(Writer writer, StateSetSnapshot snapshot, EscapingScheme scheme)
478562
throws IOException {
479563
MetricMetadata metadata = snapshot.getMetadata();
480-
String name = getExpositionBaseMetadataName(metadata, scheme);
564+
String name = getOriginalMetadataName(metadata, scheme);
481565
writeMetadataWithName(writer, name, "stateset", metadata);
482566
for (StateSetSnapshot.StateSetDataPointSnapshot data : snapshot.getDataPoints()) {
483567
for (int i = 0; i < data.size(); i++) {
@@ -513,7 +597,7 @@ private void writeStateSet(Writer writer, StateSetSnapshot snapshot, EscapingSch
513597
private void writeUnknown(Writer writer, UnknownSnapshot snapshot, EscapingScheme scheme)
514598
throws IOException {
515599
MetricMetadata metadata = snapshot.getMetadata();
516-
String name = getExpositionBaseMetadataName(metadata, scheme);
600+
String name = getOriginalMetadataName(metadata, scheme);
517601
writeMetadataWithName(writer, name, "unknown", metadata);
518602
for (UnknownSnapshot.UnknownDataPointSnapshot data : snapshot.getDataPoints()) {
519603
writeNameAndLabels(writer, name, null, data.getLabels(), scheme);

0 commit comments

Comments
 (0)