Skip to content

Add default aggregation #2543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ocelotl merged 6 commits into from
Mar 29, 2022
Merged

Add default aggregation #2543

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d18fa2e
Add default aggregation
ocelotl Mar 22, 2022
ea68a5d
Add changelog
ocelotl Mar 25, 2022
841159c
Raise exception
ocelotl Mar 25, 2022
087160c
Fix lint
ocelotl Mar 28, 2022
8e475ff
Small fixes
ocelotl Mar 28, 2022
648c76f
Add more fixes
ocelotl Mar 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Add more fixes
  • Loading branch information
@ocelotl
ocelotl committed Mar 28, 2022
commit 648c76f33ced56d719ccca2183a8a97f3dd1f565
23 changes: 10 additions & 13 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from opentelemetry._metrics.instrument import (
Asynchronous,
Counter,
Histogram,
Instrument,
ObservableCounter,
ObservableGauge,
Expand All @@ -37,13 +38,9 @@
_Monotonic,
)
from opentelemetry.sdk._metrics.measurement import Measurement
from opentelemetry.sdk._metrics.point import (
AggregationTemporality,
Gauge,
Histogram,
PointT,
Sum,
)
from opentelemetry.sdk._metrics.point import AggregationTemporality, Gauge
from opentelemetry.sdk._metrics.point import Histogram as HistogramPoint
from opentelemetry.sdk._metrics.point import PointT, Sum
from opentelemetry.util._time import _time_ns

_PointVarT = TypeVar("_PointVarT", bound=PointT)
Expand Down Expand Up @@ -129,7 +126,7 @@ def _create_aggregation(self, instrument: Instrument) -> _Aggregation:
if isinstance(instrument, ObservableGauge):
return _LastValueAggregation()

raise Exception("Invalid instrument type found")
raise Exception(f"Invalid instrument type {type(instrument)} found")


class _SumAggregation(_Aggregation[Sum]):
Expand Down Expand Up @@ -219,7 +216,7 @@ def collect(self) -> Optional[Gauge]:
)


class _ExplicitBucketHistogramAggregation(_Aggregation[Histogram]):
class _ExplicitBucketHistogramAggregation(_Aggregation[HistogramPoint]):
def __init__(
self,
boundaries: Sequence[float] = (
Expand Down Expand Up @@ -260,7 +257,7 @@ def aggregate(self, measurement: Measurement) -> None:

self._bucket_counts[bisect_left(self._boundaries, value)] += 1

def collect(self) -> Histogram:
def collect(self) -> HistogramPoint:
"""
Atomically return a point for the current value of the metric.
"""
Expand All @@ -275,7 +272,7 @@ def collect(self) -> Histogram:
self._start_time_unix_nano = now + 1
self._sum = 0

return Histogram(
return HistogramPoint(
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=now,
bucket_counts=tuple(value),
Expand Down Expand Up @@ -360,7 +357,7 @@ def _convert_aggregation_temporality(
is_monotonic=is_monotonic,
)

if current_point_type is Histogram:
if current_point_type is HistogramPoint:
if previous_point is None:
return replace(
current_point, aggregation_temporality=aggregation_temporality
Expand Down Expand Up @@ -394,7 +391,7 @@ def _convert_aggregation_temporality(
)
]

return Histogram(
return HistogramPoint(
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=current_point.time_unix_nano,
bucket_counts=bucket_counts,
Expand Down
4 changes: 3 additions & 1 deletion opentelemetry-sdk/src/opentelemetry/sdk/_metrics/view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ class View:
This class is not intended to be subclassed by the user.
"""

_default_aggregation = DefaultAggregation()

def __init__(
self,
instrument_type: Optional[Type[Instrument]] = None,
Expand Down Expand Up @@ -126,7 +128,7 @@ def __init__(

self._description = description
self._attribute_keys = attribute_keys
self._aggregation = aggregation or DefaultAggregation()
self._aggregation = aggregation or self._default_aggregation

# pylint: disable=too-many-return-statements
# pylint: disable=too-many-branches
Expand Down
32 changes: 17 additions & 15 deletions opentelemetry-sdk/tests/metrics/test_aggregation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@
)
from opentelemetry.sdk._metrics.instrument import (
Counter,
Histogram,
ObservableCounter,
ObservableGauge,
ObservableUpDownCounter,
UpDownCounter,
)
from opentelemetry.sdk._metrics.measurement import Measurement
from opentelemetry.sdk._metrics.point import Gauge, Histogram, Sum
from opentelemetry.sdk._metrics.point import Gauge
from opentelemetry.sdk._metrics.point import Histogram as HistogramPoint
from opentelemetry.sdk._metrics.point import Sum
from opentelemetry.util.types import Attributes


Expand Down Expand Up @@ -609,7 +612,7 @@ def test_current_point_gauge(self):
class TestHistogramConvertAggregationTemporality(TestCase):
def test_previous_point_none(self):

current_point = Histogram(
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
bucket_counts=[0, 2, 1, 2, 0],
Expand All @@ -633,15 +636,15 @@ def test_previous_point_non_cumulative(self):
with self.assertRaises(Exception):

_convert_aggregation_temporality(
Histogram(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
sum=70,
aggregation_temporality=AggregationTemporality.DELTA,
),
Histogram(
HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
bucket_counts=[0, 1, 3, 0, 0],
Expand All @@ -653,7 +656,7 @@ def test_previous_point_non_cumulative(self):
),

def test_same_aggregation_temporality_cumulative(self):
current_point = Histogram(
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
bucket_counts=[0, 3, 4, 2, 0],
Expand All @@ -663,7 +666,7 @@ def test_same_aggregation_temporality_cumulative(self):
)
self.assertEqual(
_convert_aggregation_temporality(
Histogram(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
bucket_counts=[0, 2, 1, 2, 0],
Expand All @@ -678,7 +681,7 @@ def test_same_aggregation_temporality_cumulative(self):
)

def test_same_aggregation_temporality_delta(self):
current_point = Histogram(
current_point = HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
bucket_counts=[0, 1, 3, 0, 0],
Expand All @@ -689,7 +692,7 @@ def test_same_aggregation_temporality_delta(self):

self.assertEqual(
_convert_aggregation_temporality(
Histogram(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
bucket_counts=[0, 3, 4, 2, 0],
Expand All @@ -704,7 +707,7 @@ def test_same_aggregation_temporality_delta(self):
)

def test_aggregation_temporality_to_cumulative(self):
current_point = Histogram(
current_point = HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
bucket_counts=[0, 1, 3, 0, 0],
Expand All @@ -715,7 +718,7 @@ def test_aggregation_temporality_to_cumulative(self):

self.assertEqual(
_convert_aggregation_temporality(
Histogram(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
bucket_counts=[0, 2, 1, 2, 0],
Expand All @@ -726,7 +729,7 @@ def test_aggregation_temporality_to_cumulative(self):
current_point,
AggregationTemporality.CUMULATIVE,
),
Histogram(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
bucket_counts=[0, 3, 4, 2, 0],
Expand All @@ -737,7 +740,7 @@ def test_aggregation_temporality_to_cumulative(self):
)

def test_aggregation_temporality_to_delta(self):
current_point = Histogram(
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
bucket_counts=[0, 3, 4, 2, 0],
Expand All @@ -748,7 +751,7 @@ def test_aggregation_temporality_to_delta(self):

self.assertEqual(
_convert_aggregation_temporality(
Histogram(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
bucket_counts=[0, 2, 1, 2, 0],
Expand All @@ -759,7 +762,7 @@ def test_aggregation_temporality_to_delta(self):
current_point,
AggregationTemporality.DELTA,
),
Histogram(
HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
bucket_counts=[0, 1, 3, 0, 0],
Expand Down Expand Up @@ -887,7 +890,6 @@ def test_histogram(self):
Mock(),
Mock(),
Mock(),
Mock(),
)
)
self.assertIsInstance(aggregation, _ExplicitBucketHistogramAggregation)
Expand Down