Skip to content

Commit 62e9ad3

Browse files
Antrakosaabmass
andauthored
Fix duplicate HELP/TYPE declarations in Prometheus exporter (#4869)
Fixes #4868 The Prometheus exporter was generating duplicate HELP and TYPE declarations for metrics with varying label sets, violating the Prometheus format specification and causing rejection by Prometheus Pushgateway with the error: "second HELP line for metric name." Changes: - Modified metric family ID to exclude label keys, using only metric name, description, and unit - Implemented two-pass processing: first pass collects all unique label keys across data points, second pass builds label values with empty strings for missing labels - Ensured single metric family per metric type with consolidated label keys - Updated test expectations to verify single HELP/TYPE declaration with proper empty string handling for missing labels This aligns with the Go implementation approach and ensures compatibility with Prometheus Pushgateway and other strict validators. Co-authored-by: Aaron Abbott <[email protected]>
1 parent 784442f commit 62e9ad3

File tree

3 files changed

+106
-100
lines changed

3 files changed

+106
-100
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212

1313
## Unreleased
1414

15+
- `opentelemetry-exporter-prometheus`: Fix duplicate HELP/TYPE declarations for metrics with different label sets
16+
([#4868](https://github.com/open-telemetry/opentelemetry-python/issues/4868))
1517
- Allow loading all resource detectors by setting `OTEL_EXPERIMENTAL_RESOURCE_DETECTORS` to `*`
1618
([#4819](https://github.com/open-telemetry/opentelemetry-python/pull/4819))
1719
- `opentelemetry-sdk`: Fix the type hint of the `_metrics_data` property to allow `None`

exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py

Lines changed: 99 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -225,36 +225,23 @@ def _translate_to_prometheus(
225225

226226
for metric in metrics:
227227
label_values_data_points = []
228-
label_keys_data_points = []
229228
values = []
230229

231-
per_metric_family_ids = []
232-
233230
metric_name = sanitize_full_name(metric.name)
234231
metric_description = metric.description or ""
235232
metric_unit = map_unit(metric.unit)
236233

234+
# First pass: collect all unique label keys across all data points
235+
all_label_keys_set = set()
236+
data_point_attributes = []
237237
for number_data_point in metric.data.data_points:
238-
label_keys = []
239-
label_values = []
240-
241-
for key, value in sorted(number_data_point.attributes.items()):
242-
label_keys.append(sanitize_attribute(key))
243-
label_values.append(self._check_value(value))
244-
245-
per_metric_family_ids.append(
246-
"|".join(
247-
[
248-
metric_name,
249-
metric_description,
250-
"%".join(label_keys),
251-
metric_unit,
252-
]
253-
)
254-
)
238+
attrs = {}
239+
for key, value in number_data_point.attributes.items():
240+
sanitized_key = sanitize_attribute(key)
241+
all_label_keys_set.add(sanitized_key)
242+
attrs[sanitized_key] = self._check_value(value)
243+
data_point_attributes.append(attrs)
255244

256-
label_values_data_points.append(label_values)
257-
label_keys_data_points.append(label_keys)
258245
if isinstance(number_data_point, HistogramDataPoint):
259246
values.append(
260247
{
@@ -268,87 +255,106 @@ def _translate_to_prometheus(
268255
else:
269256
values.append(number_data_point.value)
270257

271-
for per_metric_family_id, label_keys, label_values, value in zip(
272-
per_metric_family_ids,
273-
label_keys_data_points,
274-
label_values_data_points,
275-
values,
276-
):
277-
is_non_monotonic_sum = (
278-
isinstance(metric.data, Sum)
279-
and metric.data.is_monotonic is False
280-
)
281-
is_cumulative = (
282-
isinstance(metric.data, Sum)
283-
and metric.data.aggregation_temporality
284-
== AggregationTemporality.CUMULATIVE
285-
)
258+
# Sort label keys for consistent ordering
259+
all_label_keys = sorted(all_label_keys_set)
286260

287-
# The prometheus compatibility spec for sums says: If the aggregation temporality is cumulative and the sum is non-monotonic, it MUST be converted to a Prometheus Gauge.
288-
should_convert_sum_to_gauge = (
289-
is_non_monotonic_sum and is_cumulative
290-
)
261+
# Second pass: build label values with empty strings for missing labels
262+
for attrs in data_point_attributes:
263+
label_values = []
264+
for key in all_label_keys:
265+
label_values.append(attrs.get(key, ""))
266+
label_values_data_points.append(label_values)
291267

292-
if (
293-
isinstance(metric.data, Sum)
294-
and not should_convert_sum_to_gauge
295-
):
296-
metric_family_id = "|".join(
297-
[per_metric_family_id, CounterMetricFamily.__name__]
298-
)
268+
# Create metric family ID without label keys
269+
per_metric_family_id = "|".join(
270+
[
271+
metric_name,
272+
metric_description,
273+
metric_unit,
274+
]
275+
)
276+
277+
is_non_monotonic_sum = (
278+
isinstance(metric.data, Sum)
279+
and metric.data.is_monotonic is False
280+
)
281+
is_cumulative = (
282+
isinstance(metric.data, Sum)
283+
and metric.data.aggregation_temporality
284+
== AggregationTemporality.CUMULATIVE
285+
)
286+
287+
# The prometheus compatibility spec for sums says: If the aggregation temporality is cumulative and the sum is non-monotonic, it MUST be converted to a Prometheus Gauge.
288+
should_convert_sum_to_gauge = (
289+
is_non_monotonic_sum and is_cumulative
290+
)
291+
292+
if (
293+
isinstance(metric.data, Sum)
294+
and not should_convert_sum_to_gauge
295+
):
296+
metric_family_id = "|".join(
297+
[per_metric_family_id, CounterMetricFamily.__name__]
298+
)
299299

300-
if metric_family_id not in metric_family_id_metric_family:
301-
metric_family_id_metric_family[metric_family_id] = (
302-
CounterMetricFamily(
303-
name=metric_name,
304-
documentation=metric_description,
305-
labels=label_keys,
306-
unit=metric_unit,
307-
)
300+
if metric_family_id not in metric_family_id_metric_family:
301+
metric_family_id_metric_family[metric_family_id] = (
302+
CounterMetricFamily(
303+
name=metric_name,
304+
documentation=metric_description,
305+
labels=all_label_keys,
306+
unit=metric_unit,
308307
)
308+
)
309+
for label_values, value in zip(
310+
label_values_data_points, values
311+
):
309312
metric_family_id_metric_family[
310313
metric_family_id
311314
].add_metric(labels=label_values, value=value)
312-
elif (
313-
isinstance(metric.data, Gauge)
314-
or should_convert_sum_to_gauge
315-
):
316-
metric_family_id = "|".join(
317-
[per_metric_family_id, GaugeMetricFamily.__name__]
318-
)
315+
elif isinstance(metric.data, Gauge) or should_convert_sum_to_gauge:
316+
metric_family_id = "|".join(
317+
[per_metric_family_id, GaugeMetricFamily.__name__]
318+
)
319319

320-
if (
321-
metric_family_id
322-
not in metric_family_id_metric_family.keys()
323-
):
324-
metric_family_id_metric_family[metric_family_id] = (
325-
GaugeMetricFamily(
326-
name=metric_name,
327-
documentation=metric_description,
328-
labels=label_keys,
329-
unit=metric_unit,
330-
)
320+
if (
321+
metric_family_id
322+
not in metric_family_id_metric_family.keys()
323+
):
324+
metric_family_id_metric_family[metric_family_id] = (
325+
GaugeMetricFamily(
326+
name=metric_name,
327+
documentation=metric_description,
328+
labels=all_label_keys,
329+
unit=metric_unit,
331330
)
331+
)
332+
for label_values, value in zip(
333+
label_values_data_points, values
334+
):
332335
metric_family_id_metric_family[
333336
metric_family_id
334337
].add_metric(labels=label_values, value=value)
335-
elif isinstance(metric.data, Histogram):
336-
metric_family_id = "|".join(
337-
[per_metric_family_id, HistogramMetricFamily.__name__]
338-
)
338+
elif isinstance(metric.data, Histogram):
339+
metric_family_id = "|".join(
340+
[per_metric_family_id, HistogramMetricFamily.__name__]
341+
)
339342

340-
if (
341-
metric_family_id
342-
not in metric_family_id_metric_family.keys()
343-
):
344-
metric_family_id_metric_family[metric_family_id] = (
345-
HistogramMetricFamily(
346-
name=metric_name,
347-
documentation=metric_description,
348-
labels=label_keys,
349-
unit=metric_unit,
350-
)
343+
if (
344+
metric_family_id
345+
not in metric_family_id_metric_family.keys()
346+
):
347+
metric_family_id_metric_family[metric_family_id] = (
348+
HistogramMetricFamily(
349+
name=metric_name,
350+
documentation=metric_description,
351+
labels=all_label_keys,
352+
unit=metric_unit,
351353
)
354+
)
355+
for label_values, value in zip(
356+
label_values_data_points, values
357+
):
352358
metric_family_id_metric_family[
353359
metric_family_id
354360
].add_metric(
@@ -358,10 +364,10 @@ def _translate_to_prometheus(
358364
),
359365
sum_value=value["sum"],
360366
)
361-
else:
362-
_logger.warning(
363-
"Unsupported metric data. %s", type(metric.data)
364-
)
367+
else:
368+
_logger.warning(
369+
"Unsupported metric data. %s", type(metric.data)
370+
)
365371

366372
# pylint: disable=no-self-use
367373
def _check_value(self, value: Union[int, float, str, Sequence]) -> str:

exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -684,13 +684,11 @@ def test_multiple_data_points_with_different_label_sets(self):
684684
http_server_request_duration_seconds_bucket{http_target="/foobar",le="+Inf",net_host_port="8080"} 6.0
685685
http_server_request_duration_seconds_count{http_target="/foobar",net_host_port="8080"} 6.0
686686
http_server_request_duration_seconds_sum{http_target="/foobar",net_host_port="8080"} 579.0
687-
# HELP http_server_request_duration_seconds test multiple label sets
688-
# TYPE http_server_request_duration_seconds histogram
689-
http_server_request_duration_seconds_bucket{le="123.0",net_host_port="8080"} 1.0
690-
http_server_request_duration_seconds_bucket{le="456.0",net_host_port="8080"} 4.0
691-
http_server_request_duration_seconds_bucket{le="+Inf",net_host_port="8080"} 7.0
692-
http_server_request_duration_seconds_count{net_host_port="8080"} 7.0
693-
http_server_request_duration_seconds_sum{net_host_port="8080"} 579.0
687+
http_server_request_duration_seconds_bucket{http_target="",le="123.0",net_host_port="8080"} 1.0
688+
http_server_request_duration_seconds_bucket{http_target="",le="456.0",net_host_port="8080"} 4.0
689+
http_server_request_duration_seconds_bucket{http_target="",le="+Inf",net_host_port="8080"} 7.0
690+
http_server_request_duration_seconds_count{http_target="",net_host_port="8080"} 7.0
691+
http_server_request_duration_seconds_sum{http_target="",net_host_port="8080"} 579.0
694692
"""
695693
),
696694
)

0 commit comments

Comments
 (0)