From f7645dfe952986a775b74bf1acb91bbb793411a9 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 6 Jun 2022 22:06:18 +0100 Subject: [PATCH] Check data point before yielding Fixes #2743 --- CHANGELOG.md | 2 + .../_internal/_view_instrument_match.py | 4 +- .../metrics/test_view_instrument_match.py | 72 +++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8209e3c182a..7fd1faf3311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc1-0.31b0...HEAD) +- Fix yield of `None`-valued points + ([#2745](https://github.com/open-telemetry/opentelemetry-python/pull/2745)) - Add missing `to_json` methods ([#2722](https://github.com/open-telemetry/opentelemetry-python/pull/2722) - Fix type hints for textmap `Getter` and `Setter` diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py index 026b9702350..4ada52d91b0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py @@ -130,6 +130,8 @@ def collect( with self._lock: for aggregation in self._attributes_aggregation.values(): - yield aggregation.collect( + data_point = aggregation.collect( aggregation_temporality, collection_start_nanos ) + if data_point is not None: + yield data_point diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index 839f4ccfa20..d2730086a47 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -212,6 +212,78 @@ def test_collect(self): self.assertEqual(number_data_point.attributes, {"c": "d"}) self.assertEqual(number_data_point.value, 0) + def test_data_point_check(self): + instrument1 = Counter( + "instrument1", + Mock(), + Mock(), + description="description", + unit="unit", + ) + instrument1.instrumentation_scope = self.mock_instrumentation_scope + + view_instrument_match = _ViewInstrumentMatch( + view=View( + instrument_name="instrument1", + name="name", + aggregation=DefaultAggregation(), + ), + instrument=instrument1, + instrument_class_aggregation=MagicMock( + **{ + "__getitem__.return_value": Mock( + **{ + "_create_aggregation.return_value": Mock( + **{ + "collect.side_effect": [ + Mock(), + Mock(), + None, + Mock(), + ] + } + ) + } + ) + } + ), + ) + + view_instrument_match.consume_measurement( + Measurement( + value=0, + instrument=Mock(name="instrument1"), + attributes={"c": "d", "f": "g"}, + ) + ) + view_instrument_match.consume_measurement( + Measurement( + value=0, + instrument=Mock(name="instrument1"), + attributes={"h": "i", "j": "k"}, + ) + ) + view_instrument_match.consume_measurement( + Measurement( + value=0, + instrument=Mock(name="instrument1"), + attributes={"l": "m", "n": "o"}, + ) + ) + view_instrument_match.consume_measurement( + Measurement( + value=0, + instrument=Mock(name="instrument1"), + attributes={"p": "q", "r": "s"}, + ) + ) + + result = view_instrument_match.collect( + AggregationTemporality.CUMULATIVE, 0 + ) + + self.assertEqual(len(list(result)), 3) + def test_setting_aggregation(self): instrument1 = Counter( name="instrument1",