From ca4eb1f403a14bea8ef17a27b656406112c83e4b Mon Sep 17 00:00:00 2001 From: Brett Beutell Date: Wed, 10 May 2023 16:29:53 +0200 Subject: [PATCH 1/5] Modify Prometheus exporter to translate non-monotonic Sums into gauges Add unit tests for translating monotonic and non-monotonic Sums to prometheus metrics Update prometheus metric translation to match spec Added a check for the aggregation temporality of a Sum. "If the aggregation temporality is cumulative and the sum is non-monotonic, it MUST be converted to a Prometheus Gauge." Update changelog --- CHANGELOG.md | 8 ++-- .../exporter/prometheus/__init__.py | 10 +++- .../tests/test_prometheus_exporter.py | 46 ++++++++++++++++++- .../src/opentelemetry/test/metrictestutil.py | 4 +- 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5896872952..a88f1c18569 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Modify Prometheus exporter to translate non-monotonic Sums into Gauges + ([#3306](https://github.com/open-telemetry/opentelemetry-python/pull/3306)) + ## Version 1.18.0/0.39b0 (2023-05-04) - Select histogram aggregation with an environment variable @@ -43,7 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Version 1.16.0/0.37b0 (2023-02-17) -- Change ``__all__`` to be statically defined. +- Change `__all__` to be statically defined. ([#3143](https://github.com/open-telemetry/opentelemetry-python/pull/3143)) - Remove the ability to set a global metric prefix for Prometheus exporter ([#3137](https://github.com/open-telemetry/opentelemetry-python/pull/3137)) @@ -66,7 +69,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Create a single resource instance ([#3118](https://github.com/open-telemetry/opentelemetry-python/pull/3118)) - ## Version 1.15.0/0.36b0 (2022-12-09) - PeriodicExportingMetricsReader with +Inf interval @@ -84,7 +86,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3027](https://github.com/open-telemetry/opentelemetry-python/pull/3027)) - Update logging to include logging api as per specification ([#3038](https://github.com/open-telemetry/opentelemetry-python/pull/3038)) -- Fix: Avoid generator in metrics _ViewInstrumentMatch.collect() +- Fix: Avoid generator in metrics \_ViewInstrumentMatch.collect() ([#3035](https://github.com/open-telemetry/opentelemetry-python/pull/3035) - [exporter-otlp-proto-grpc] add user agent string ([#3009](https://github.com/open-telemetry/opentelemetry-python/pull/3009)) diff --git a/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py b/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py index 7442b7b242d..ec09965087e 100644 --- a/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py +++ b/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py @@ -244,7 +244,13 @@ def _translate_to_prometheus( for pre_metric_family_id, label_values, value in zip( pre_metric_family_ids, label_valuess, values ): - if isinstance(metric.data, Sum): + is_non_monotonic_sum = isinstance(metric.data, Sum) and metric.data.is_monotonic == False + is_cumulative = isinstance(metric.data, Sum) and metric.data.aggregation_temporality == AggregationTemporality.CUMULATIVE + + # 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. + should_convert_sum_to_gauge = (is_non_monotonic_sum and is_cumulative) + + if isinstance(metric.data, Sum) and not should_convert_sum_to_gauge: metric_family_id = "|".join( [pre_metric_family_id, CounterMetricFamily.__name__] @@ -262,7 +268,7 @@ def _translate_to_prometheus( metric_family_id_metric_family[ metric_family_id ].add_metric(labels=label_values, value=value) - elif isinstance(metric.data, Gauge): + elif isinstance(metric.data, Gauge) or should_convert_sum_to_gauge: metric_family_id = "|".join( [pre_metric_family_id, GaugeMetricFamily.__name__] diff --git a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py index 1180fac6141..08e4625d4b1 100644 --- a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py +++ b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py @@ -120,7 +120,7 @@ def test_histogram_to_prometheus(self): ), ) - def test_sum_to_prometheus(self): + def test_monotonic_sum_to_prometheus(self): labels = {"environment@": "staging", "os": "Windows"} metric = _generate_sum( "test@sum", @@ -163,6 +163,50 @@ def test_sum_to_prometheus(self): prometheus_metric.samples[0].labels["os"], "Windows" ) + def test_non_monotonic_sum_to_prometheus(self): + labels = {"environment@": "staging", "os": "Windows"} + metric = _generate_sum( + "test@sum", + 123, + attributes=labels, + description="testdesc", + unit="testunit", + is_monotonic=False + ) + + metrics_data = MetricsData( + resource_metrics=[ + ResourceMetrics( + resource=Mock(), + scope_metrics=[ + ScopeMetrics( + scope=Mock(), + metrics=[metric], + schema_url="schema_url", + ) + ], + schema_url="schema_url", + ) + ] + ) + + collector = _CustomCollector() + collector.add_metrics_data(metrics_data) + + for prometheus_metric in collector.collect(): + self.assertEqual(type(prometheus_metric), GaugeMetricFamily) + self.assertEqual(prometheus_metric.name, "test_sum_testunit") + self.assertEqual(prometheus_metric.documentation, "testdesc") + self.assertTrue(len(prometheus_metric.samples) == 1) + self.assertEqual(prometheus_metric.samples[0].value, 123) + self.assertTrue(len(prometheus_metric.samples[0].labels) == 2) + self.assertEqual( + prometheus_metric.samples[0].labels["environment_"], "staging" + ) + self.assertEqual( + prometheus_metric.samples[0].labels["os"], "Windows" + ) + def test_gauge_to_prometheus(self): labels = {"environment@": "dev", "os": "Unix"} metric = _generate_gauge( diff --git a/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py b/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py index 895904af03f..d0f9e09d541 100644 --- a/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py +++ b/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py @@ -39,7 +39,7 @@ def _generate_metric( def _generate_sum( - name, value, attributes=None, description=None, unit=None + name, value, attributes=None, description=None, unit=None, is_monotonic=True ) -> Metric: if attributes is None: attributes = BoundedAttributes(attributes={"a": 1, "b": True}) @@ -55,7 +55,7 @@ def _generate_sum( ) ], aggregation_temporality=AggregationTemporality.CUMULATIVE, - is_monotonic=True, + is_monotonic=is_monotonic, ), description=description, unit=unit, From 0ceeea29e1f446fa172cdb0b1838fe99c6eab3cb Mon Sep 17 00:00:00 2001 From: Brett Beutell Date: Wed, 17 May 2023 23:57:16 +0200 Subject: [PATCH 2/5] Revert CHANGELOG.md formatting changes --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a88f1c18569..3deebb75afd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Version 1.16.0/0.37b0 (2023-02-17) -- Change `__all__` to be statically defined. +- Change ``__all__`` to be statically defined. ([#3143](https://github.com/open-telemetry/opentelemetry-python/pull/3143)) - Remove the ability to set a global metric prefix for Prometheus exporter ([#3137](https://github.com/open-telemetry/opentelemetry-python/pull/3137)) @@ -86,7 +86,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3027](https://github.com/open-telemetry/opentelemetry-python/pull/3027)) - Update logging to include logging api as per specification ([#3038](https://github.com/open-telemetry/opentelemetry-python/pull/3038)) -- Fix: Avoid generator in metrics \_ViewInstrumentMatch.collect() +- Fix: Avoid generator in metrics _ViewInstrumentMatch.collect() ([#3035](https://github.com/open-telemetry/opentelemetry-python/pull/3035) - [exporter-otlp-proto-grpc] add user agent string ([#3009](https://github.com/open-telemetry/opentelemetry-python/pull/3009)) From 7551e265d043dc9aeddaa998f459979b08439f18 Mon Sep 17 00:00:00 2001 From: Brett Beutell Date: Fri, 23 Jun 2023 17:47:29 +0200 Subject: [PATCH 3/5] Reformat code that violated linting rules --- .../exporter/prometheus/__init__.py | 25 +++++++++++++++---- .../tests/test_prometheus_exporter.py | 2 +- .../src/opentelemetry/test/metrictestutil.py | 7 +++++- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py b/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py index ec09965087e..f849c767c92 100644 --- a/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py +++ b/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py @@ -244,13 +244,25 @@ def _translate_to_prometheus( for pre_metric_family_id, label_values, value in zip( pre_metric_family_ids, label_valuess, values ): - is_non_monotonic_sum = isinstance(metric.data, Sum) and metric.data.is_monotonic == False - is_cumulative = isinstance(metric.data, Sum) and metric.data.aggregation_temporality == AggregationTemporality.CUMULATIVE + is_non_monotonic_sum = ( + isinstance(metric.data, Sum) + and metric.data.is_monotonic == False + ) + is_cumulative = ( + isinstance(metric.data, Sum) + and metric.data.aggregation_temporality + == AggregationTemporality.CUMULATIVE + ) # 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. - should_convert_sum_to_gauge = (is_non_monotonic_sum and is_cumulative) + should_convert_sum_to_gauge = ( + is_non_monotonic_sum and is_cumulative + ) - if isinstance(metric.data, Sum) and not should_convert_sum_to_gauge: + if ( + isinstance(metric.data, Sum) + and not should_convert_sum_to_gauge + ): metric_family_id = "|".join( [pre_metric_family_id, CounterMetricFamily.__name__] @@ -268,7 +280,10 @@ def _translate_to_prometheus( metric_family_id_metric_family[ metric_family_id ].add_metric(labels=label_values, value=value) - elif isinstance(metric.data, Gauge) or should_convert_sum_to_gauge: + elif ( + isinstance(metric.data, Gauge) + or should_convert_sum_to_gauge + ): metric_family_id = "|".join( [pre_metric_family_id, GaugeMetricFamily.__name__] diff --git a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py index 08e4625d4b1..cc260a5f693 100644 --- a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py +++ b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py @@ -171,7 +171,7 @@ def test_non_monotonic_sum_to_prometheus(self): attributes=labels, description="testdesc", unit="testunit", - is_monotonic=False + is_monotonic=False, ) metrics_data = MetricsData( diff --git a/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py b/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py index d0f9e09d541..ff25b092a66 100644 --- a/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py +++ b/tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py @@ -39,7 +39,12 @@ def _generate_metric( def _generate_sum( - name, value, attributes=None, description=None, unit=None, is_monotonic=True + name, + value, + attributes=None, + description=None, + unit=None, + is_monotonic=True, ) -> Metric: if attributes is None: attributes = BoundedAttributes(attributes={"a": 1, "b": True}) From 7bbc3f61c08a158124b34cf51acd1cdd2cc2794b Mon Sep 17 00:00:00 2001 From: Brett Beutell Date: Mon, 26 Jun 2023 15:03:35 +0200 Subject: [PATCH 4/5] Fix flake8 linting error --- .../src/opentelemetry/exporter/prometheus/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py b/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py index f849c767c92..506fb5dde8b 100644 --- a/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py +++ b/exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/__init__.py @@ -246,7 +246,7 @@ def _translate_to_prometheus( ): is_non_monotonic_sum = ( isinstance(metric.data, Sum) - and metric.data.is_monotonic == False + and metric.data.is_monotonic is False ) is_cumulative = ( isinstance(metric.data, Sum) From 22f98f98807490cef5d66665fcc90947555fc05f Mon Sep 17 00:00:00 2001 From: Brett Beutell Date: Fri, 14 Jul 2023 13:59:06 +0200 Subject: [PATCH 5/5] Fix test_non_monotonic_sum_to_prometheus after changes from #3279 --- .../tests/test_prometheus_exporter.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py index 1a648012c44..a9ab05d01e9 100644 --- a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py +++ b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py @@ -128,7 +128,7 @@ def test_histogram_to_prometheus(self): def test_monotonic_sum_to_prometheus(self): labels = {"environment@": "staging", "os": "Windows"} metric = _generate_sum( - "test@sum", + "test@sum_monotonic", 123, attributes=labels, description="testdesc", @@ -156,7 +156,9 @@ def test_monotonic_sum_to_prometheus(self): for prometheus_metric in collector.collect(): self.assertEqual(type(prometheus_metric), CounterMetricFamily) - self.assertEqual(prometheus_metric.name, "test_sum_testunit") + self.assertEqual( + prometheus_metric.name, "test_sum_monotonic_testunit" + ) self.assertEqual(prometheus_metric.documentation, "testdesc") self.assertTrue(len(prometheus_metric.samples) == 1) self.assertEqual(prometheus_metric.samples[0].value, 123) @@ -171,7 +173,7 @@ def test_monotonic_sum_to_prometheus(self): def test_non_monotonic_sum_to_prometheus(self): labels = {"environment@": "staging", "os": "Windows"} metric = _generate_sum( - "test@sum", + "test@sum_nonmonotonic", 123, attributes=labels, description="testdesc", @@ -195,12 +197,14 @@ def test_non_monotonic_sum_to_prometheus(self): ] ) - collector = _CustomCollector() + collector = _CustomCollector(disable_target_info=True) collector.add_metrics_data(metrics_data) for prometheus_metric in collector.collect(): self.assertEqual(type(prometheus_metric), GaugeMetricFamily) - self.assertEqual(prometheus_metric.name, "test_sum_testunit") + self.assertEqual( + prometheus_metric.name, "test_sum_nonmonotonic_testunit" + ) self.assertEqual(prometheus_metric.documentation, "testdesc") self.assertTrue(len(prometheus_metric.samples) == 1) self.assertEqual(prometheus_metric.samples[0].value, 123)