From c29e0dd62bf60d39562e35a93aa0d58b22081370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= <fcollonval@users.noreply.github.com> Date: Tue, 3 Sep 2024 17:55:49 +0200 Subject: [PATCH] Fix pylint errors --- .../sdk/metrics/_internal/aggregation.py | 1 - .../_internal/exemplar/exemplar_reservoir.py | 8 +-- .../sdk/metrics/_internal/view.py | 6 +- .../tests/metrics/test_aggregation.py | 5 +- .../metrics/test_view_instrument_match.py | 67 +++++++++++-------- 5 files changed, 45 insertions(+), 42 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 271e256449..39c967e4c8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -105,7 +105,6 @@ def aggregate( measurement: Measurement to aggregate should_sample_exemplar: Whether the measurement should be sampled by the exemplars reservoir or not. """ - pass @abstractmethod def collect( diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py index 2ce5d2461a..d3fc4e900e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py @@ -107,7 +107,7 @@ def offer( self.__offered = True - def collect(self, point_attributes: Attributes) -> Exemplar | None: + def collect(self, point_attributes: Attributes) -> Optional[Exemplar]: """May return an Exemplar and resets the bucket for the next sampling period.""" if not self.__offered: return None @@ -232,11 +232,9 @@ def _find_bucket_index( Raises: BucketIndexError: If no bucket index can be found. """ - pass def _reset(self) -> None: """Reset the reservoir by resetting any stateful logic after a collection cycle.""" - pass class SimpleFixedSizeExemplarReservoir(FixedSizeExemplarReservoirABC): @@ -309,9 +307,9 @@ def _find_bucket_index( attributes: Attributes, context: Context, ) -> int: - for i, boundary in enumerate(self._boundaries): + for index, boundary in enumerate(self._boundaries): if value <= boundary: - return i + return index return len(self._boundaries) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py index e9176af33e..5dd11be1f9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py @@ -35,12 +35,12 @@ def _default_reservoir_factory( - aggregationType: Type[_Aggregation], + aggregation_type: Type[_Aggregation], ) -> ExemplarReservoirBuilder: """Default reservoir factory per aggregation.""" - if issubclass(aggregationType, _ExplicitBucketHistogramAggregation): + if issubclass(aggregation_type, _ExplicitBucketHistogramAggregation): return AlignedHistogramBucketExemplarReservoir - elif issubclass(aggregationType, _ExponentialBucketHistogramAggregation): + if issubclass(aggregation_type, _ExponentialBucketHistogramAggregation): return SimpleFixedSizeExemplarReservoir return SimpleFixedSizeExemplarReservoir diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 0d3b084788..3eeb63e26c 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -704,15 +704,12 @@ def test_collection_simple_fixed_size_reservoir_with_default_reservoir( def test_collection_aligned_histogram_bucket_reservoir(self): boundaries = [5.0, 10.0, 20.0] - exemplar_reservoir_factory = ( - lambda: AlignedHistogramBucketExemplarReservoir(boundaries) - ) synchronous_sum_aggregation = _SumAggregation( Mock(), True, AggregationTemporality.DELTA, 0, - exemplar_reservoir_factory, + lambda: AlignedHistogramBucketExemplarReservoir(boundaries), ) synchronous_sum_aggregation.aggregate(measurement(2.0)) diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index a5c8c48026..fa9a0e4dc7 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -52,18 +52,18 @@ def generalized_reservoir_factory( size: int = 1, boundaries: Sequence[float] = None ) -> Callable[[Type[_Aggregation]], ExemplarReservoirBuilder]: def factory( - aggregationType: Type[_Aggregation], + aggregation_type: Type[_Aggregation], ) -> ExemplarReservoirBuilder: - if issubclass(aggregationType, _ExplicitBucketHistogramAggregation): + if issubclass(aggregation_type, _ExplicitBucketHistogramAggregation): return lambda **kwargs: AlignedHistogramBucketExemplarReservoir( boundaries=boundaries or [], **{k: v for k, v in kwargs.items() if k != "boundaries"}, ) - else: - return lambda **kwargs: SimpleFixedSizeExemplarReservoir( - size=size, - **{k: v for k, v in kwargs.items() if k != "size"}, - ) + + return lambda **kwargs: SimpleFixedSizeExemplarReservoir( + size=size, + **{k: v for k, v in kwargs.items() if k != "size"}, + ) return factory @@ -288,7 +288,11 @@ def test_collect_resets_start_time_unix_nano(self, mock_time_ns): # +1 call to _create_aggregation view_instrument_match.consume_measurement( Measurement( - value=0, instrument=instrument, attributes={"foo": "bar0"} + value=0, + time_unix_nano=time_ns(), + instrument=instrument, + attributes={"foo": "bar0"}, + context=Context(), ) ) view_instrument_match._view._aggregation._create_aggregation.assert_called_with( @@ -304,7 +308,11 @@ def test_collect_resets_start_time_unix_nano(self, mock_time_ns): # +1 call to _create_aggregation view_instrument_match.consume_measurement( Measurement( - value=0, instrument=instrument, attributes={"foo": "bar1"} + value=0, + time_unix_nano=time_ns(), + instrument=instrument, + attributes={"foo": "bar1"}, + context=Context(), ) ) view_instrument_match._view._aggregation._create_aggregation.assert_called_with( @@ -322,7 +330,11 @@ def test_collect_resets_start_time_unix_nano(self, mock_time_ns): # +1 call to create_aggregation view_instrument_match.consume_measurement( Measurement( - value=0, instrument=instrument, attributes={"foo": "bar"} + value=0, + time_unix_nano=time_ns(), + instrument=instrument, + attributes={"foo": "bar"}, + context=Context(), ) ) view_instrument_match._view._aggregation._create_aggregation.assert_called_with( @@ -331,12 +343,20 @@ def test_collect_resets_start_time_unix_nano(self, mock_time_ns): # No new calls to _create_aggregation because attributes remain the same view_instrument_match.consume_measurement( Measurement( - value=0, instrument=instrument, attributes={"foo": "bar"} + value=0, + time_unix_nano=time_ns(), + instrument=instrument, + attributes={"foo": "bar"}, + context=Context(), ) ) view_instrument_match.consume_measurement( Measurement( - value=0, instrument=instrument, attributes={"foo": "bar"} + value=0, + time_unix_nano=time_ns(), + instrument=instrument, + attributes={"foo": "bar"}, + context=Context(), ) ) # In total we have 5 calls for _create_aggregation @@ -520,8 +540,8 @@ def test_consume_measurement_with_custom_reservoir_factory(self): ) ) - data_points = view_instrument_match.collect( - AggregationTemporality.CUMULATIVE, 0 + data_points = list( + view_instrument_match.collect(AggregationTemporality.CUMULATIVE, 0) ) # Ensure only one data point is collected @@ -577,8 +597,8 @@ def test_consume_measurement_with_exemplars(self): ) # Collect the data points - data_points = view_instrument_match.collect( - AggregationTemporality.CUMULATIVE, 0 + data_points = list( + view_instrument_match.collect(AggregationTemporality.CUMULATIVE, 0) ) # Ensure only one data point is collected @@ -660,19 +680,9 @@ def test_consume_measurement_with_custom_reservoir_factory(self): ) ) - # view_instrument_match.consume_measurement( - # Measurement( - # value=30.0, # Should go into the outliners bucket - # time_unix_nano=time_ns(), - # instrument=instrument1, - # context=Context(), - # attributes={"attribute3": "value3"}, - # ) - # ) - # Collect the data points - data_points = view_instrument_match.collect( - AggregationTemporality.CUMULATIVE, 0 + data_points = list( + view_instrument_match.collect(AggregationTemporality.CUMULATIVE, 0) ) # Ensure three data points are collected, one for each bucket @@ -692,4 +702,3 @@ def test_consume_measurement_with_custom_reservoir_factory(self): self.assertEqual( data_points[2].exemplars[0].value, 15.0 ) # Third bucket - # self.assertEqual(data_points[2].exemplars[0].value, 30.0) # Outliner bucket