Merge branch 'main' into prometheus_exporter

Author: fcollonval

CHANGELOG.md

@@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#4222](https://github.com/open-telemetry/opentelemetry-python/pull/4222))
 - Record logger name as the instrumentation scope name
   ([#4208](https://github.com/open-telemetry/opentelemetry-python/pull/4208))
+- Fix memory leak in exporter and reader
+  ([#4224](https://github.com/open-telemetry/opentelemetry-python/pull/4224))
+- Drop `OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION` environment variable
+  ([#4217](https://github.com/open-telemetry/opentelemetry-python/pull/4217))
 
 ## Version 1.27.0/0.48b0 (2024-08-28)
 

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

@@ -88,7 +88,6 @@
 from opentelemetry.sdk.environment_variables import (
     OTEL_EXPORTER_PROMETHEUS_HOST,
     OTEL_EXPORTER_PROMETHEUS_PORT,
-    OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION,
 )
 from opentelemetry.sdk.metrics import (
     Counter,
@@ -255,18 +254,7 @@ def _translate_to_prometheus(
 
             metric_description = metric.description or ""
 
-            # TODO(#3929): remove this opt-out option
-            disable_unit_normalization = (
-                environ.get(
-                    OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION,
-                    "false",
-                ).lower()
-                == "true"
-            )
-            if disable_unit_normalization:
-                metric_unit = metric.unit
-            else:
-                metric_unit = map_unit(metric.unit)
+            metric_unit = map_unit(metric.unit)
 
             for number_data_point in metric.data.data_points:
                 label_keys = []

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

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from os import environ
 from textwrap import dedent
 from unittest import TestCase
 from unittest.mock import Mock, patch
@@ -31,9 +30,6 @@
     PrometheusMetricReader,
     _CustomCollector,
 )
-from opentelemetry.sdk.environment_variables import (
-    OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION,
-)
 from opentelemetry.sdk.metrics import Exemplar, MeterProvider
 from opentelemetry.sdk.metrics.export import (
     AggregationTemporality,
@@ -623,25 +619,6 @@ def test_metric_name_with_unit(self):
             ),
         )
 
-    # TODO(#3929): remove this opt-out option
-    @patch.dict(
-        environ,
-        {
-            OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION: "true"
-        },
-    )
-    def test_metric_name_with_unit_normalization_disabled(self):
-        self.verify_text_format(
-            _generate_sum(name="test_unit_not_normalized", value=1, unit="s"),
-            dedent(
-                """\
-                # HELP test_unit_not_normalized_s_total foo
-                # TYPE test_unit_not_normalized_s_total counter
-                test_unit_not_normalized_s_total{a="1",b="true"} 1.0
-                """
-            ),
-        )
-
     def test_semconv(self):
         """Tests that a few select semconv metrics get converted to the expected prometheus
         text format"""

opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py

@@ -780,21 +780,3 @@
 This is an experimental environment variable and the name of this variable and its behavior can
 change in a non-backwards compatible way.
 """
-
-
-# TODO(#3929): remove this opt-out option
-OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION = (
-    "OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION"
-)
-"""
-.. envvar:: OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION
-
-The :envvar:`OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION` environment
-variable allows you to opt-out of the unit normalization described `in the specification
-<https://github.com/open-telemetry/opentelemetry-specification/blob/v1.33.0/specification/compatibility/prometheus_and_openmetrics.md#metric-metadata-1>`_.
-
-Default: False
-
-This is an temporary environment variable that provides backward compatibility but will be
-removed in the future https://github.com/open-telemetry/opentelemetry-python/issues/3929.
-"""

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py

@@ -11,7 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
+import weakref
 from atexit import register, unregister
 from logging import getLogger
 from os import environ
@@ -386,7 +386,7 @@ class MeterProvider(APIMeterProvider):
     """
 
     _all_metric_readers_lock = Lock()
-    _all_metric_readers = set()
+    _all_metric_readers = weakref.WeakSet()
 
     def __init__(
         self,

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py

@@ -14,6 +14,7 @@
 
 import math
 import os
+import weakref
 from abc import ABC, abstractmethod
 from enum import Enum
 from logging import getLogger
@@ -488,7 +489,11 @@ def __init__(
             )
             self._daemon_thread.start()
             if hasattr(os, "register_at_fork"):
-                os.register_at_fork(after_in_child=self._at_fork_reinit)  # pylint: disable=protected-access
+                weak_at_fork = weakref.WeakMethod(self._at_fork_reinit)
+
+                os.register_at_fork(
+                    after_in_child=lambda: weak_at_fork()()  # pylint: disable=unnecessary-lambda, protected-access
+                )
         elif self._export_interval_millis <= 0:
             raise ValueError(
                 f"interval value {self._export_interval_millis} is invalid \

opentelemetry-sdk/tests/metrics/integration_test/test_provider_shutdown.py

@@ -0,0 +1,86 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import gc
+import time
+import weakref
+from typing import Sequence
+from unittest import TestCase
+
+from opentelemetry.sdk.metrics import MeterProvider
+from opentelemetry.sdk.metrics.export import (
+    Metric,
+    MetricExporter,
+    MetricExportResult,
+    PeriodicExportingMetricReader,
+)
+
+
+class FakeMetricsExporter(MetricExporter):
+    def __init__(
+        self, wait=0, preferred_temporality=None, preferred_aggregation=None
+    ):
+        self.wait = wait
+        self.metrics = []
+        self._shutdown = False
+        super().__init__(
+            preferred_temporality=preferred_temporality,
+            preferred_aggregation=preferred_aggregation,
+        )
+
+    def export(
+        self,
+        metrics_data: Sequence[Metric],
+        timeout_millis: float = 10_000,
+        **kwargs,
+    ) -> MetricExportResult:
+        time.sleep(self.wait)
+        self.metrics.extend(metrics_data)
+        return True
+
+    def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None:
+        self._shutdown = True
+
+    def force_flush(self, timeout_millis: float = 10_000) -> bool:
+        return True
+
+
+class TestMeterProviderShutdown(TestCase):
+    def test_meter_provider_shutdown_cleans_up_successfully(self):
+        def create_and_shutdown():
+            exporter = FakeMetricsExporter()
+            exporter_wr = weakref.ref(exporter)
+
+            reader = PeriodicExportingMetricReader(exporter)
+            reader_wr = weakref.ref(reader)
+
+            provider = MeterProvider(metric_readers=[reader])
+            provider_wr = weakref.ref(provider)
+
+            provider.shutdown()
+
+            return exporter_wr, reader_wr, provider_wr
+
+        # When: the provider is shutdown
+        (
+            exporter_weakref,
+            reader_weakref,
+            provider_weakref,
+        ) = create_and_shutdown()
+        gc.collect()
+
+        # Then: the provider, exporter and reader should be garbage collected
+        self.assertIsNone(exporter_weakref())
+        self.assertIsNone(reader_weakref())
+        self.assertIsNone(provider_weakref())

opentelemetry-sdk/tests/metrics/test_metrics.py

@@ -14,7 +14,7 @@
 
 # pylint: disable=protected-access,no-self-use
 
-
+import weakref
 from logging import WARNING
 from time import sleep
 from typing import Iterable, Sequence
@@ -66,7 +66,7 @@ def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None:
 
 class TestMeterProvider(ConcurrencyTestBase, TestCase):
     def tearDown(self):
-        MeterProvider._all_metric_readers = set()
+        MeterProvider._all_metric_readers = weakref.WeakSet()
 
     @patch.object(Resource, "create")
     def test_init_default(self, resource_patch):