Respect OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE only for otlp exporter

CHANGELOG.md

@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2754](https://github.com/open-telemetry/opentelemetry-python/pull/2754))
 - Fix --insecure of CLI argument
   ([#2696](https://github.com/open-telemetry/opentelemetry-python/pull/2696))
+- Use `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` only for OTLP metrics exporter
+  ([#2843](https://github.com/open-telemetry/opentelemetry-python/pull/2843))
 
 ## [1.12.0rc2-0.32b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc2) - 2022-07-04
 

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

@@ -161,9 +161,6 @@ class MetricReader(ABC):
     .. automethod:: _receive_metrics
     """
 
-    # FIXME add :std:envvar:`OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE`
-    # to the end of the documentation paragraph above.
-
     def __init__(
         self,
         preferred_temporality: Dict[type, AggregationTemporality] = None,
@@ -179,33 +176,14 @@ def __init__(
             Iterable["opentelemetry.sdk.metrics.export.Metric"],
         ] = None
 
-        if (
-            environ.get(
-                OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE,
-                "CUMULATIVE",
-            )
-            .upper()
-            .strip()
-            == "DELTA"
-        ):
-            self._instrument_class_temporality = {
-                Counter: AggregationTemporality.DELTA,
-                UpDownCounter: AggregationTemporality.CUMULATIVE,
-                Histogram: AggregationTemporality.DELTA,
-                ObservableCounter: AggregationTemporality.DELTA,
-                ObservableUpDownCounter: AggregationTemporality.CUMULATIVE,
-                ObservableGauge: AggregationTemporality.CUMULATIVE,
-            }
-
-        else:
-            self._instrument_class_temporality = {
-                Counter: AggregationTemporality.CUMULATIVE,
-                UpDownCounter: AggregationTemporality.CUMULATIVE,
-                Histogram: AggregationTemporality.CUMULATIVE,
-                ObservableCounter: AggregationTemporality.CUMULATIVE,
-                ObservableUpDownCounter: AggregationTemporality.CUMULATIVE,
-                ObservableGauge: AggregationTemporality.CUMULATIVE,
-            }
+        self._instrument_class_temporality = {
+            Counter: AggregationTemporality.CUMULATIVE,
+            UpDownCounter: AggregationTemporality.CUMULATIVE,
+            Histogram: AggregationTemporality.CUMULATIVE,
+            ObservableCounter: AggregationTemporality.CUMULATIVE,
+            ObservableUpDownCounter: AggregationTemporality.CUMULATIVE,
+            ObservableGauge: AggregationTemporality.CUMULATIVE,
+        }
 
         if preferred_temporality is not None:
             for temporality in preferred_temporality.values():
@@ -343,8 +321,32 @@ def __init__(
         export_interval_millis: Optional[float] = None,
         export_timeout_millis: Optional[float] = None,
     ) -> None:
+        _instrument_class_temporality = {}
+        # Exporter-specific logic for otlp
+        # This approach is considered "hacky" and is unavoidable for this feature
+        # It is not recommended to have exporter specific logic and string comparison
+        # in any of the metric reader classes
+        if exporter.__class__.__name__.lower().__contains__("otlp"):
+            if (
+                environ.get(
+                    OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE,
+                    "CUMULATIVE",
+                )
+                .upper()
+                .strip()
+                == "DELTA"
+            ):
+                _instrument_class_temporality = {
+                    Counter: AggregationTemporality.DELTA,
+                    UpDownCounter: AggregationTemporality.CUMULATIVE,
+                    Histogram: AggregationTemporality.DELTA,
+                    ObservableCounter: AggregationTemporality.DELTA,
+                    ObservableUpDownCounter: AggregationTemporality.CUMULATIVE,
+                    ObservableGauge: AggregationTemporality.CUMULATIVE,
+                }
+        _instrument_class_temporality.update(preferred_temporality or {})
         super().__init__(
-            preferred_temporality=preferred_temporality,
+            preferred_temporality=_instrument_class_temporality,
             preferred_aggregation=preferred_aggregation,
         )
         self._exporter = exporter

opentelemetry-sdk/tests/metrics/test_metric_reader.py

@@ -12,14 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from os import environ
 from typing import Dict, Iterable
 from unittest import TestCase
-from unittest.mock import patch
 
-from opentelemetry.sdk.environment_variables import (
-    OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE,
-)
 from opentelemetry.sdk.metrics import (
     Counter,
     Histogram,
@@ -64,82 +59,7 @@ def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None:
 
 
 class TestMetricReader(TestCase):
-    @patch.dict(
-        environ,
-        {OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "CUMULATIVE"},
-    )
-    def test_configure_temporality_cumulative(self):
-
-        dummy_metric_reader = DummyMetricReader()
-
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality.keys(),
-            set(
-                [
-                    Counter,
-                    UpDownCounter,
-                    Histogram,
-                    ObservableCounter,
-                    ObservableUpDownCounter,
-                    ObservableGauge,
-                ]
-            ),
-        )
-        for (
-            value
-        ) in dummy_metric_reader._instrument_class_temporality.values():
-            self.assertEqual(value, AggregationTemporality.CUMULATIVE)
-
-    @patch.dict(
-        environ, {OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "DELTA"}
-    )
-    def test_configure_temporality_delta(self):
-
-        dummy_metric_reader = DummyMetricReader()
-
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality.keys(),
-            set(
-                [
-                    Counter,
-                    UpDownCounter,
-                    Histogram,
-                    ObservableCounter,
-                    ObservableUpDownCounter,
-                    ObservableGauge,
-                ]
-            ),
-        )
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality[Counter],
-            AggregationTemporality.DELTA,
-        )
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality[UpDownCounter],
-            AggregationTemporality.CUMULATIVE,
-        )
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality[Histogram],
-            AggregationTemporality.DELTA,
-        )
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality[
-                ObservableCounter
-            ],
-            AggregationTemporality.DELTA,
-        )
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality[
-                ObservableUpDownCounter
-            ],
-            AggregationTemporality.CUMULATIVE,
-        )
-        self.assertEqual(
-            dummy_metric_reader._instrument_class_temporality[ObservableGauge],
-            AggregationTemporality.CUMULATIVE,
-        )
-
-    def test_configure_temporality_parameter(self):
+    def test_configure_temporality(self):
 
         dummy_metric_reader = DummyMetricReader(
             preferred_temporality={
@@ -190,7 +110,7 @@ def test_configure_temporality_parameter(self):
             AggregationTemporality.DELTA,
         )
 
-    def test_default_temporality(self):
+    def test_configure_aggregation(self):
         dummy_metric_reader = DummyMetricReader()
         self.assertEqual(
             dummy_metric_reader._instrument_class_aggregation.keys(),

opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py

@@ -13,12 +13,25 @@
 # limitations under the License.
 
 import time
+from os import environ
 from typing import Sequence
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
 from flaky import flaky
 
+from opentelemetry.sdk.environment_variables import (
+    OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE,
+)
+from opentelemetry.sdk.metrics import (
+    Counter,
+    Histogram,
+    ObservableCounter,
+    ObservableGauge,
+    ObservableUpDownCounter,
+    UpDownCounter,
+)
 from opentelemetry.sdk.metrics.export import (
+    AggregationTemporality,
     Gauge,
     Metric,
     MetricExporter,
@@ -51,6 +64,19 @@ def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None:
         self._shutdown = True
 
 
+class FakeOTLPMetricsExporter(MetricExporter):
+    def export(
+        self,
+        metrics: Sequence[Metric],
+        timeout_millis: float = 10_000,
+        **kwargs,
+    ) -> MetricExportResult:
+        pass
+
+    def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None:
+        pass
+
+
 metrics_list = [
     Metric(
         name="sum_name",
@@ -143,3 +169,96 @@ def test_shutdown_multiple_times(self):
             self.run_with_many_threads(pmr.shutdown)
             self.assertTrue("Can't shutdown multiple times", w.output[0])
         pmr.shutdown()
+
+    @patch.dict(
+        environ,
+        {OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "DELTA"},
+    )
+    def test_non_otlp_temporality_preference(self):
+        exporter = FakeMetricsExporter()
+
+        pmr = self._create_periodic_reader([], exporter)
+        for value in pmr._instrument_class_temporality.values():
+            self.assertEqual(value, AggregationTemporality.CUMULATIVE)
+
+    @patch.dict(
+        environ,
+        {OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "CUMULATIVE"},
+    )
+    def test_otlp_temporality_preference_cumulative(self):
+        exporter = FakeOTLPMetricsExporter()
+
+        pmr = self._create_periodic_reader([], exporter)
+        for value in pmr._instrument_class_temporality.values():
+            self.assertEqual(value, AggregationTemporality.CUMULATIVE)
+
+    @patch.dict(
+        environ,
+        {OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "DELTA"},
+    )
+    def test_otlp_temporality_preference_delta(self):
+        exporter = FakeOTLPMetricsExporter()
+
+        pmr = self._create_periodic_reader([], exporter)
+        self.assertEqual(
+            pmr._instrument_class_temporality[Counter],
+            AggregationTemporality.DELTA,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[UpDownCounter],
+            AggregationTemporality.CUMULATIVE,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[Histogram],
+            AggregationTemporality.DELTA,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[ObservableCounter],
+            AggregationTemporality.DELTA,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[ObservableUpDownCounter],
+            AggregationTemporality.CUMULATIVE,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[ObservableGauge],
+            AggregationTemporality.CUMULATIVE,
+        )
+
+    @patch.dict(
+        environ,
+        {OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "DELTA"},
+    )
+    def test_otlp_temporality_preference_combination(self):
+        exporter = FakeOTLPMetricsExporter()
+
+        pmr = PeriodicExportingMetricReader(
+            exporter,
+            preferred_temporality={
+                Counter: AggregationTemporality.CUMULATIVE,
+            },
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[Counter],
+            AggregationTemporality.CUMULATIVE,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[UpDownCounter],
+            AggregationTemporality.CUMULATIVE,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[Histogram],
+            AggregationTemporality.DELTA,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[ObservableCounter],
+            AggregationTemporality.DELTA,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[ObservableUpDownCounter],
+            AggregationTemporality.CUMULATIVE,
+        )
+        self.assertEqual(
+            pmr._instrument_class_temporality[ObservableGauge],
+            AggregationTemporality.CUMULATIVE,
+        )