Skip to content

Commit

Permalink
Fix pylint errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@fcollonval
fcollonval committed Sep 3, 2024
1 parent e8aa164 commit c29e0dd
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 42 deletions.
1 change: 0 additions & 1 deletion opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 3 additions & 5 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)


Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 1 addition & 4 deletions opentelemetry-sdk/tests/metrics/test_aggregation.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
67 changes: 38 additions & 29 deletions opentelemetry-sdk/tests/metrics/test_view_instrument_match.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

0 comments on commit c29e0dd

Please sign in to comment.