Skip to content

Commit ed1f8ba

Browse files
[release/v1.34.x] Restore prometheus metric name mapper tests, fix regressions (#6141)
Co-authored-by: jack-berg <[email protected]>
1 parent 0201c61 commit ed1f8ba

File tree

5 files changed

+264
-34
lines changed

5 files changed

+264
-34
lines changed

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,7 @@ private CounterSnapshot convertLongCounter(
183183
MetricMetadata metadata,
184184
InstrumentationScopeInfo scope,
185185
Collection<LongPointData> dataPoints) {
186-
List<CounterDataPointSnapshot> data =
187-
new ArrayList<CounterDataPointSnapshot>(dataPoints.size());
186+
List<CounterDataPointSnapshot> data = new ArrayList<>(dataPoints.size());
188187
for (LongPointData longData : dataPoints) {
189188
data.add(
190189
new CounterDataPointSnapshot(
@@ -449,8 +448,14 @@ private static MetricMetadata convertMetadata(MetricData metricData) {
449448
String help = metricData.getDescription();
450449
Unit unit = PrometheusUnitsHelper.convertUnit(metricData.getUnit());
451450
if (unit != null && !name.endsWith(unit.toString())) {
452-
name += "_" + unit;
451+
// Need to re-sanitize metric name since unit may contain illegal characters
452+
name = sanitizeMetricName(name + "_" + unit);
453+
}
454+
// Repeated __ are not allowed according to spec, although this is allowed in prometheus
455+
while (name.contains("__")) {
456+
name = name.replace("__", "_");
453457
}
458+
454459
return new MetricMetadata(name, help, unit);
455460
}
456461

@@ -538,7 +543,8 @@ private static MetricMetadata mergeMetadata(MetricMetadata a, MetricMetadata b)
538543
"Conflicting metrics: Multiple metrics with name "
539544
+ name
540545
+ " but different units found. Dropping the one with unit "
541-
+ b.getUnit());
546+
+ b.getUnit()
547+
+ ".");
542548
return null;
543549
}
544550
return new MetricMetadata(name, help, unit);

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusUnitsHelper.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,7 @@ private static void initUnit(String otelName, String pluralName, String singular
6969

7070
@Nullable
7171
static Unit convertUnit(String otelUnit) {
72-
if (otelUnit.isEmpty() || otelUnit.equals("1")) {
73-
// The spec says "1" should be translated to "ratio", but this is not implemented in the Java
74-
// SDK.
72+
if (otelUnit.isEmpty()) {
7573
return null;
7674
}
7775
if (otelUnit.contains("{")) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.exporter.prometheus;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
10+
import io.opentelemetry.api.common.Attributes;
11+
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
12+
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
13+
import io.opentelemetry.sdk.metrics.data.MetricData;
14+
import io.opentelemetry.sdk.metrics.data.MetricDataType;
15+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableGaugeData;
16+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableHistogramData;
17+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableHistogramPointData;
18+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableLongPointData;
19+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableMetricData;
20+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableSumData;
21+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableSummaryData;
22+
import io.opentelemetry.sdk.metrics.internal.data.ImmutableSummaryPointData;
23+
import io.opentelemetry.sdk.resources.Resource;
24+
import io.prometheus.metrics.expositionformats.ExpositionFormats;
25+
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
26+
import java.io.ByteArrayOutputStream;
27+
import java.io.IOException;
28+
import java.nio.charset.StandardCharsets;
29+
import java.util.Arrays;
30+
import java.util.Collections;
31+
import java.util.regex.Matcher;
32+
import java.util.regex.Pattern;
33+
import java.util.stream.Stream;
34+
import org.junit.jupiter.params.ParameterizedTest;
35+
import org.junit.jupiter.params.provider.Arguments;
36+
import org.junit.jupiter.params.provider.MethodSource;
37+
38+
class Otel2PrometheusConverterTest {
39+
40+
private static final Pattern PATTERN =
41+
Pattern.compile(
42+
"# HELP (?<help>.*)\n# TYPE (?<type>.*)\n(?<metricName>.*)\\{otel_scope_name=\"scope\"}(.|\\n)*");
43+
44+
private final Otel2PrometheusConverter converter = new Otel2PrometheusConverter(true);
45+
46+
@ParameterizedTest
47+
@MethodSource("metricMetadataArgs")
48+
void metricMetadata(
49+
MetricData metricData, String expectedType, String expectedHelp, String expectedMetricName)
50+
throws IOException {
51+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
52+
MetricSnapshots snapshots = converter.convert(Collections.singletonList(metricData));
53+
ExpositionFormats.init().getPrometheusTextFormatWriter().write(baos, snapshots);
54+
String expositionFormat = new String(baos.toByteArray(), StandardCharsets.UTF_8);
55+
56+
// Uncomment to debug exposition format output
57+
// System.out.println(expositionFormat);
58+
59+
Matcher matcher = PATTERN.matcher(expositionFormat);
60+
assertThat(matcher.matches()).isTrue();
61+
assertThat(matcher.group("help")).isEqualTo(expectedHelp);
62+
assertThat(matcher.group("type")).isEqualTo(expectedType);
63+
// Note: Summaries and histograms produce output which matches METRIC_NAME_PATTERN multiple
64+
// times. The pattern ends up matching against the first.
65+
assertThat(matcher.group("metricName")).isEqualTo(expectedMetricName);
66+
}
67+
68+
private static Stream<Arguments> metricMetadataArgs() {
69+
return Stream.of(
70+
// the unity unit "1" is translated to "ratio"
71+
Arguments.of(
72+
createSampleMetricData("sample", "1", MetricDataType.LONG_GAUGE),
73+
"sample_ratio gauge",
74+
"sample_ratio description",
75+
"sample_ratio"),
76+
// unit is appended to metric name
77+
Arguments.of(
78+
createSampleMetricData("sample", "unit", MetricDataType.LONG_GAUGE),
79+
"sample_unit gauge",
80+
"sample_unit description",
81+
"sample_unit"),
82+
// units in curly braces are dropped
83+
Arguments.of(
84+
createSampleMetricData("sample", "1{dropped}", MetricDataType.LONG_GAUGE),
85+
"sample_ratio gauge",
86+
"sample_ratio description",
87+
"sample_ratio"),
88+
// monotonic sums always include _total suffix
89+
Arguments.of(
90+
createSampleMetricData("sample", "unit", MetricDataType.LONG_SUM),
91+
"sample_unit_total counter",
92+
"sample_unit_total description",
93+
"sample_unit_total"),
94+
Arguments.of(
95+
createSampleMetricData("sample", "1", MetricDataType.LONG_SUM),
96+
"sample_ratio_total counter",
97+
"sample_ratio_total description",
98+
"sample_ratio_total"),
99+
// units expressed as numbers other than 1 are retained
100+
Arguments.of(
101+
createSampleMetricData("sample", "2", MetricDataType.LONG_SUM),
102+
"sample_2_total counter",
103+
"sample_2_total description",
104+
"sample_2_total"),
105+
Arguments.of(
106+
createSampleMetricData("metric_name", "2", MetricDataType.SUMMARY),
107+
"metric_name_2 summary",
108+
"metric_name_2 description",
109+
"metric_name_2_count"),
110+
// unsupported characters are translated to "_", repeated "_" are dropped
111+
Arguments.of(
112+
createSampleMetricData("s%%ple", "%/min", MetricDataType.SUMMARY),
113+
"s_ple_percent_per_minute summary",
114+
"s_ple_percent_per_minute description",
115+
"s_ple_percent_per_minute_count"),
116+
// metric unit is not appended if the name already contains the unit
117+
Arguments.of(
118+
createSampleMetricData("metric_name_total", "total", MetricDataType.LONG_SUM),
119+
"metric_name_total counter",
120+
"metric_name_total description",
121+
"metric_name_total"),
122+
// total suffix is stripped because total is a reserved suffixed for monotonic sums
123+
Arguments.of(
124+
createSampleMetricData("metric_name_total", "total", MetricDataType.SUMMARY),
125+
"metric_name summary",
126+
"metric_name description",
127+
"metric_name_count"),
128+
// if metric name ends with unit the unit is omitted
129+
Arguments.of(
130+
createSampleMetricData("metric_name_ratio", "1", MetricDataType.LONG_GAUGE),
131+
"metric_name_ratio gauge",
132+
"metric_name_ratio description",
133+
"metric_name_ratio"),
134+
Arguments.of(
135+
createSampleMetricData("metric_name_ratio", "1", MetricDataType.SUMMARY),
136+
"metric_name_ratio summary",
137+
"metric_name_ratio description",
138+
"metric_name_ratio_count"),
139+
Arguments.of(
140+
createSampleMetricData("metric_hertz", "hertz", MetricDataType.LONG_GAUGE),
141+
"metric_hertz gauge",
142+
"metric_hertz description",
143+
"metric_hertz"),
144+
Arguments.of(
145+
createSampleMetricData("metric_hertz", "hertz", MetricDataType.LONG_SUM),
146+
"metric_hertz_total counter",
147+
"metric_hertz_total description",
148+
"metric_hertz_total"),
149+
// if metric name ends with unit the unit is omitted - order matters
150+
Arguments.of(
151+
createSampleMetricData("metric_total_hertz", "hertz_total", MetricDataType.LONG_SUM),
152+
"metric_total_hertz_hertz_total counter",
153+
"metric_total_hertz_hertz_total description",
154+
"metric_total_hertz_hertz_total"),
155+
// metric name cannot start with a number
156+
Arguments.of(
157+
createSampleMetricData("2_metric_name", "By", MetricDataType.SUMMARY),
158+
"_metric_name_bytes summary",
159+
"_metric_name_bytes description",
160+
"_metric_name_bytes_count"));
161+
}
162+
163+
static MetricData createSampleMetricData(
164+
String metricName, String metricUnit, MetricDataType metricDataType) {
165+
switch (metricDataType) {
166+
case SUMMARY:
167+
return ImmutableMetricData.createDoubleSummary(
168+
Resource.getDefault(),
169+
InstrumentationScopeInfo.create("scope"),
170+
metricName,
171+
"description",
172+
metricUnit,
173+
ImmutableSummaryData.create(
174+
Collections.singletonList(
175+
ImmutableSummaryPointData.create(
176+
0, 1, Attributes.empty(), 1, 1, Collections.emptyList()))));
177+
case LONG_SUM:
178+
return ImmutableMetricData.createLongSum(
179+
Resource.getDefault(),
180+
InstrumentationScopeInfo.create("scope"),
181+
metricName,
182+
"description",
183+
metricUnit,
184+
ImmutableSumData.create(
185+
true,
186+
AggregationTemporality.CUMULATIVE,
187+
Collections.singletonList(
188+
ImmutableLongPointData.create(0, 1, Attributes.empty(), 1L))));
189+
case LONG_GAUGE:
190+
return ImmutableMetricData.createLongGauge(
191+
Resource.getDefault(),
192+
InstrumentationScopeInfo.create("scope"),
193+
metricName,
194+
"description",
195+
metricUnit,
196+
ImmutableGaugeData.create(
197+
Collections.singletonList(
198+
ImmutableLongPointData.create(0, 1, Attributes.empty(), 1L))));
199+
case HISTOGRAM:
200+
return ImmutableMetricData.createDoubleHistogram(
201+
Resource.getDefault(),
202+
InstrumentationScopeInfo.create("scope"),
203+
metricName,
204+
"description",
205+
metricUnit,
206+
ImmutableHistogramData.create(
207+
AggregationTemporality.CUMULATIVE,
208+
Collections.singletonList(
209+
ImmutableHistogramPointData.create(
210+
0,
211+
1,
212+
Attributes.empty(),
213+
1,
214+
false,
215+
-1,
216+
false,
217+
-1,
218+
Collections.singletonList(1.0),
219+
Arrays.asList(0L, 1L)))));
220+
default:
221+
throw new IllegalArgumentException("Unsupported metric data type: " + metricDataType);
222+
}
223+
}
224+
}

exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java

+28-26
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,12 @@ void fetchPrometheus() {
115115
.isEqualTo("text/plain; version=0.0.4; charset=utf-8");
116116
assertThat(response.contentUtf8())
117117
.isEqualTo(
118-
"# HELP grpc_name_total long_description\n"
119-
+ "# TYPE grpc_name_total counter\n"
120-
+ "grpc_name_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
121-
+ "# HELP http_name_total double_description\n"
122-
+ "# TYPE http_name_total counter\n"
123-
+ "http_name_total{kp=\"vp\",otel_scope_name=\"http\",otel_scope_version=\"version\"} 3.5\n"
118+
"# HELP grpc_name_unit_total long_description\n"
119+
+ "# TYPE grpc_name_unit_total counter\n"
120+
+ "grpc_name_unit_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
121+
+ "# HELP http_name_unit_total double_description\n"
122+
+ "# TYPE http_name_unit_total counter\n"
123+
+ "http_name_unit_total{kp=\"vp\",otel_scope_name=\"http\",otel_scope_version=\"version\"} 3.5\n"
124124
+ "# TYPE target_info gauge\n"
125125
+ "target_info{kr=\"vr\"} 1\n");
126126
}
@@ -142,12 +142,14 @@ void fetchOpenMetrics() {
142142
.isEqualTo("application/openmetrics-text; version=1.0.0; charset=utf-8");
143143
assertThat(response.contentUtf8())
144144
.isEqualTo(
145-
"# TYPE grpc_name counter\n"
146-
+ "# HELP grpc_name long_description\n"
147-
+ "grpc_name_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
148-
+ "# TYPE http_name counter\n"
149-
+ "# HELP http_name double_description\n"
150-
+ "http_name_total{kp=\"vp\",otel_scope_name=\"http\",otel_scope_version=\"version\"} 3.5\n"
145+
"# TYPE grpc_name_unit counter\n"
146+
+ "# UNIT grpc_name_unit unit\n"
147+
+ "# HELP grpc_name_unit long_description\n"
148+
+ "grpc_name_unit_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
149+
+ "# TYPE http_name_unit counter\n"
150+
+ "# UNIT http_name_unit unit\n"
151+
+ "# HELP http_name_unit double_description\n"
152+
+ "http_name_unit_total{kp=\"vp\",otel_scope_name=\"http\",otel_scope_version=\"version\"} 3.5\n"
151153
+ "# TYPE target info\n"
152154
+ "target_info{kr=\"vr\"} 1\n"
153155
+ "# EOF\n");
@@ -157,7 +159,7 @@ void fetchOpenMetrics() {
157159
void fetchFiltered() {
158160
AggregatedHttpResponse response =
159161
client
160-
.get("/?name[]=grpc_name_total&name[]=bears_total&name[]=target_info")
162+
.get("/?name[]=grpc_name_unit_total&name[]=bears_total&name[]=target_info")
161163
.aggregate()
162164
.join();
163165
assertThat(response.status()).isEqualTo(HttpStatus.OK);
@@ -166,9 +168,9 @@ void fetchFiltered() {
166168
assertThat(response.contentUtf8())
167169
.isEqualTo(
168170
""
169-
+ "# HELP grpc_name_total long_description\n"
170-
+ "# TYPE grpc_name_total counter\n"
171-
+ "grpc_name_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
171+
+ "# HELP grpc_name_unit_total long_description\n"
172+
+ "# TYPE grpc_name_unit_total counter\n"
173+
+ "grpc_name_unit_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
172174
+ "# TYPE target_info gauge\n"
173175
+ "target_info{kr=\"vr\"} 1\n");
174176
}
@@ -189,12 +191,12 @@ void fetchPrometheusCompressed() throws IOException {
189191
String content = new String(ByteStreams.toByteArray(gis), StandardCharsets.UTF_8);
190192
assertThat(content)
191193
.isEqualTo(
192-
"# HELP grpc_name_total long_description\n"
193-
+ "# TYPE grpc_name_total counter\n"
194-
+ "grpc_name_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
195-
+ "# HELP http_name_total double_description\n"
196-
+ "# TYPE http_name_total counter\n"
197-
+ "http_name_total{kp=\"vp\",otel_scope_name=\"http\",otel_scope_version=\"version\"} 3.5\n"
194+
"# HELP grpc_name_unit_total long_description\n"
195+
+ "# TYPE grpc_name_unit_total counter\n"
196+
+ "grpc_name_unit_total{kp=\"vp\",otel_scope_name=\"grpc\",otel_scope_version=\"version\"} 5.0\n"
197+
+ "# HELP http_name_unit_total double_description\n"
198+
+ "# TYPE http_name_unit_total counter\n"
199+
+ "http_name_unit_total{kp=\"vp\",otel_scope_name=\"http\",otel_scope_version=\"version\"} 3.5\n"
198200
+ "# TYPE target_info gauge\n"
199201
+ "target_info{kr=\"vr\"} 1\n");
200202
}
@@ -252,7 +254,7 @@ void fetch_DuplicateMetrics() {
252254
InstrumentationScopeInfo.create("scope3"),
253255
"foo_unit_total",
254256
"unused",
255-
"unit",
257+
"",
256258
ImmutableGaugeData.create(
257259
Collections.singletonList(
258260
ImmutableLongPointData.create(123, 456, Attributes.empty(), 3))))));
@@ -272,7 +274,7 @@ void fetch_DuplicateMetrics() {
272274
// Validate conflict warning message
273275
assertThat(logs.getEvents()).hasSize(1);
274276
logs.assertContains(
275-
"Conflicting metric name foo_unit: Found one metric with type counter and one of type gauge. Dropping the one with type gauge.");
277+
"Conflicting metrics: Multiple metrics with name foo_unit but different units found. Dropping the one with unit null.");
276278
}
277279

278280
@Test
@@ -318,7 +320,7 @@ private static List<MetricData> generateTestData() {
318320
InstrumentationScopeInfo.builder("grpc").setVersion("version").build(),
319321
"grpc.name",
320322
"long_description",
321-
"1",
323+
"unit",
322324
ImmutableSumData.create(
323325
/* isMonotonic= */ true,
324326
AggregationTemporality.CUMULATIVE,
@@ -330,7 +332,7 @@ private static List<MetricData> generateTestData() {
330332
InstrumentationScopeInfo.builder("http").setVersion("version").build(),
331333
"http.name",
332334
"double_description",
333-
"1",
335+
"unit",
334336
ImmutableSumData.create(
335337
/* isMonotonic= */ true,
336338
AggregationTemporality.CUMULATIVE,

exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusUnitsHelperTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private static Stream<Arguments> providePrometheusOTelUnitEquivalentPairs() {
7070
// Unit not found - Case sensitive
7171
Arguments.of("S", "S"),
7272
// Special case - 1
73-
Arguments.of("1", null),
73+
Arguments.of("1", "ratio"),
7474
// Special Case - Drop metric units in {}
7575
Arguments.of("{packets}", null),
7676
// Special Case - Dropped metric units only in {}

0 commit comments

Comments
 (0)