Skip to content
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.

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

Include min/max information in Histogram point #2581

Merged
merged 7 commits into from
Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2547](https://github.com/open-telemetry/opentelemetry-python/pull/2547))
- Update otlp-proto-grpc and otlp-proto-http exporters to have more lax requirements for `backoff` lib
([#2575](https://github.com/open-telemetry/opentelemetry-python/pull/2575))
- Add min/max to histogram point
([#2581](https://github.com/open-telemetry/opentelemetry-python/pull/2581))

## [1.10.0-0.29b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.10.0-0.29b0) - 2022-03-10

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,14 @@ def setUp(self):
"histogram": _generate_metric(
"histogram",
Histogram(
time_unix_nano=1641946016139533244,
start_time_unix_nano=1641946016139533244,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[1, 4],
sum=67,
explicit_bounds=[10.0, 20.0],
aggregation_temporality=AggregationTemporality.DELTA,
max=18,
min=8,
start_time_unix_nano=1641946016139533244,
sum=67,
time_unix_nano=1641946016139533244,
),
),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ def test_histogram_to_prometheus(self):
record = _generate_metric(
"test@name",
Histogram(
time_unix_nano=1641946016139533244,
start_time_unix_nano=1641946016139533244,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[1, 3, 2],
sum=579.0,
explicit_bounds=[123.0, 456.0],
aggregation_temporality=AggregationTemporality.CUMULATIVE,
start_time_unix_nano=1641946016139533244,
max=457,
min=1,
sum=579.0,
time_unix_nano=1641946016139533244,
),
attributes={"histo": 1},
)
Expand Down
26 changes: 20 additions & 6 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,18 +267,24 @@ def collect(self) -> HistogramPoint:
value = self._bucket_counts
start_time_unix_nano = self._start_time_unix_nano
histogram_sum = self._sum
histogram_max = self._max
histogram_min = self._min

self._bucket_counts = self._get_empty_bucket_counts()
self._start_time_unix_nano = now + 1
self._sum = 0
self._min = inf
self._max = -inf

return HistogramPoint(
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=now,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=tuple(value),
explicit_bounds=self._boundaries,
aggregation_temporality=AggregationTemporality.DELTA,
max=histogram_max,
min=histogram_min,
start_time_unix_nano=start_time_unix_nano,
sum=histogram_sum,
time_unix_nano=now,
)


Expand Down Expand Up @@ -374,9 +380,15 @@ def _convert_aggregation_temporality(
if current_point.aggregation_temporality is aggregation_temporality:
return current_point

max_ = current_point.max
min_ = current_point.min

if aggregation_temporality is AggregationTemporality.CUMULATIVE:
start_time_unix_nano = previous_point.start_time_unix_nano
sum_ = current_point.sum + previous_point.sum
# Only update min/max on delta -> cumulative
max_ = max(current_point.max, previous_point.max)
min_ = min(current_point.min, previous_point.min)
bucket_counts = [
curr_count + prev_count
for curr_count, prev_count in zip(
Expand All @@ -394,12 +406,14 @@ def _convert_aggregation_temporality(
]

return HistogramPoint(
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=current_point.time_unix_nano,
aggregation_temporality=aggregation_temporality,
bucket_counts=bucket_counts,
explicit_bounds=current_point.explicit_bounds,
max=max_,
min=min_,
start_time_unix_nano=start_time_unix_nano,
sum=sum_,
aggregation_temporality=aggregation_temporality,
time_unix_nano=current_point.time_unix_nano,
)
return None

Expand Down
12 changes: 7 additions & 5 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ class AggregationTemporality(IntEnum):

@dataclass(frozen=True)
class Sum:
aggregation_temporality: AggregationTemporality
is_monotonic: bool
start_time_unix_nano: int
time_unix_nano: int
value: Union[int, float]
aggregation_temporality: AggregationTemporality
is_monotonic: bool


@dataclass(frozen=True)
Expand All @@ -45,12 +45,14 @@ class Gauge:

@dataclass(frozen=True)
class Histogram:
start_time_unix_nano: int
time_unix_nano: int
aggregation_temporality: AggregationTemporality
bucket_counts: Sequence[int]
explicit_bounds: Sequence[float]
max: int
min: int
start_time_unix_nano: int
sum: Union[int, float]
aggregation_temporality: AggregationTemporality
time_unix_nano: int


PointT = Union[Sum, Gauge, Histogram]
Expand Down
104 changes: 65 additions & 39 deletions opentelemetry-sdk/tests/metrics/test_aggregation.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,12 +624,14 @@ class TestHistogramConvertAggregationTemporality(TestCase):
def test_previous_point_none(self):

current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=2,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=1,
)

self.assertEqual(
Expand All @@ -648,42 +650,50 @@ def test_previous_point_non_cumulative(self):

_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=2,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=1,
),
HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=9,
min=2,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
),
AggregationTemporality.DELTA,
),

def test_same_aggregation_temporality_cumulative(self):
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
)
self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=1,
),
current_point,
AggregationTemporality.CUMULATIVE,
Expand All @@ -693,23 +703,27 @@ def test_same_aggregation_temporality_cumulative(self):

def test_same_aggregation_temporality_delta(self):
current_point = HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=9,
min=1,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
)

self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
),
current_point,
AggregationTemporality.DELTA,
Expand All @@ -719,67 +733,79 @@ def test_same_aggregation_temporality_delta(self):

def test_aggregation_temporality_to_cumulative(self):
current_point = HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
)

self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=1,
),
current_point,
AggregationTemporality.CUMULATIVE,
),
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
),
)

def test_aggregation_temporality_to_delta(self):
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=22,
min=3,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
)

self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=1,
),
current_point,
AggregationTemporality.DELTA,
),
HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=22,
min=3,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
),
)

Expand Down
16 changes: 9 additions & 7 deletions opentelemetry-sdk/tests/metrics/test_point.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ class TestDatapointToJSON(TestCase):
def test_sum(self):
point = _create_metric(
Sum(
aggregation_temporality=2,
is_monotonic=True,
start_time_unix_nano=10,
time_unix_nano=20,
value=9,
aggregation_temporality=2,
is_monotonic=True,
)
)
self.assertEqual(
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"start_time_unix_nano": 10, "time_unix_nano": 20, "value": 9, "aggregation_temporality": 2, "is_monotonic": true}}',
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"aggregation_temporality": 2, "is_monotonic": true, "start_time_unix_nano": 10, "time_unix_nano": 20, "value": 9}}',
point.to_json(),
)

Expand All @@ -59,16 +59,18 @@ def test_gauge(self):
def test_histogram(self):
point = _create_metric(
Histogram(
start_time_unix_nano=50,
time_unix_nano=60,
aggregation_temporality=1,
bucket_counts=[0, 0, 1, 0],
explicit_bounds=[0.1, 0.5, 0.9, 1],
aggregation_temporality=1,
max=0.8,
min=0.8,
start_time_unix_nano=50,
sum=0.8,
time_unix_nano=60,
)
)
self.maxDiff = None
self.assertEqual(
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"start_time_unix_nano": 50, "time_unix_nano": 60, "bucket_counts": [0, 0, 1, 0], "explicit_bounds": [0.1, 0.5, 0.9, 1], "sum": 0.8, "aggregation_temporality": 1}}',
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"aggregation_temporality": 1, "bucket_counts": [0, 0, 1, 0], "explicit_bounds": [0.1, 0.5, 0.9, 1], "max": 0.8, "min": 0.8, "start_time_unix_nano": 50, "sum": 0.8, "time_unix_nano": 60}}',
point.to_json(),
)