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.

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

Jump to bottom

remove unnecessary buckets on exponential histograms #3771

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

remove unnecessary buckets on exponential histograms #3771

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6296645
remove unnecessary buckets on exponential histograms
loomis-relativity Mar 11, 2024
11efdaa
update changelog
loomis-relativity Mar 11, 2024
9da195e
fix merge conflict
loomis-relativity May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Fix python 3.12 deprecation warning
([#3751](https://github.com/open-telemetry/opentelemetry-python/pull/3751))
- Remove unnecessary buckets from exponential histogram serialization
([#3767](https://github.com/open-telemetry/opentelemetry-python/pull/3767))

## Version 1.23.0/0.44b0 (2024-02-23)

Expand All @@ -32,7 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#3564](https://github.com/open-telemetry/opentelemetry-python/pull/3564))
- Fix explicit bucket histogram aggregation
([#3429](https://github.com/open-telemetry/opentelemetry-python/pull/3429))
- Add `code.lineno`, `code.function` and `code.filepath` to all logs
- Add `code.lineno`, `code.function` and `code.filepath` to all logs
([#3675](https://github.com/open-telemetry/opentelemetry-python/pull/3675))
- Add Synchronous Gauge instrument
([#3462](https://github.com/open-telemetry/opentelemetry-python/pull/3462))
Expand All @@ -57,7 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#3572](https://github.com/open-telemetry/opentelemetry-python/pull/3572))
- Remove Jaeger exporters
([#3554](https://github.com/open-telemetry/opentelemetry-python/pull/3554))
- Log stacktrace on `UNKNOWN` status OTLP export error
- Log stacktrace on `UNKNOWN` status OTLP export error
([#3536](https://github.com/open-telemetry/opentelemetry-python/pull/3536))
- Fix OTLPExporterMixin shutdown timeout period
([#3524](https://github.com/open-telemetry/opentelemetry-python/pull/3524))
Expand All @@ -82,7 +84,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#3251](https://github.com/open-telemetry/opentelemetry-python/pull/3251))
- Add missing schema_url in global api for logging and metrics
([#3251](https://github.com/open-telemetry/opentelemetry-python/pull/3251))
- Prometheus exporter support for auto instrumentation
- Prometheus exporter support for auto instrumentation
([#3413](https://github.com/open-telemetry/opentelemetry-python/pull/3413))
- Implement Process Resource detector
([#3472](https://github.com/open-telemetry/opentelemetry-python/pull/3472))
Expand Down Expand Up @@ -1465,7 +1467,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#3564](https://github.com/open-telemetry/opentelemetry-python/pull/3564))
- Fix explicit bucket histogram aggregation
([#3429](https://github.com/open-telemetry/opentelemetry-python/pull/3429))
- Add `code.lineno`, `code.function` and `code.filepath` to all logs
- Add `code.lineno`, `code.function` and `code.filepath` to all logs
([#3645](https://github.com/open-telemetry/opentelemetry-python/pull/3645))
- Add Synchronous Gauge instrument
([#3462](https://github.com/open-telemetry/opentelemetry-python/pull/3462))
Expand Down
16 changes: 11 additions & 5 deletions ...common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
Sum,
ExponentialHistogram as ExponentialHistogramType,
)
from typing import Dict
from typing import Dict, Sequence
from opentelemetry.proto.resource.v1.resource_pb2 import (
Resource as PB2Resource,
)
Expand Down Expand Up @@ -172,6 +172,12 @@ def _get_aggregation(

return instrument_class_aggregation

def truncate_trailing_zeros(lst: Sequence[int]) -> Sequence[int]:
if lst:
for i, value in enumerate(reversed(lst)):
if value != 0:
return lst[0:len(lst)-i]
return []

def encode_metrics(data: MetricsData) -> ExportMetricsServiceRequest:
resource_metrics_dict = {}
Expand Down Expand Up @@ -272,18 +278,18 @@ def encode_metrics(data: MetricsData) -> ExportMetricsServiceRequest:
elif isinstance(metric.data, ExponentialHistogramType):
for data_point in metric.data.data_points:

if data_point.positive.bucket_counts:
if positive_buckets := truncate_trailing_zeros(data_point.positive.bucket_counts):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe move truncation logic into def bucket_counts implementation to avoid duplication?

positive = pb2.ExponentialHistogramDataPoint.Buckets(
offset=data_point.positive.offset,
bucket_counts=data_point.positive.bucket_counts,
bucket_counts=positive_buckets,
)
else:
positive = None

if data_point.negative.bucket_counts:
if negative_buckets := truncate_trailing_zeros(data_point.negative.bucket_counts):
negative = pb2.ExponentialHistogramDataPoint.Buckets(
offset=data_point.negative.offset,
bucket_counts=data_point.negative.bucket_counts,
bucket_counts=negative_buckets,
)
else:
negative = None
Expand Down
14 changes: 12 additions & 2 deletions exporter/opentelemetry-exporter-otlp-proto-common/tests/test_metrics_encoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
# pylint: disable=protected-access
import unittest

from opentelemetry.exporter.otlp.proto.common._internal.metrics_encoder import (
truncate_trailing_zeros,
)
from opentelemetry.exporter.otlp.proto.common.metrics_encoder import (
encode_metrics,
)
Expand Down Expand Up @@ -694,6 +697,13 @@ def test_encode_multiple_scope_histogram(self):
actual = encode_metrics(metrics_data)
self.assertEqual(expected, actual)

def test_truncate_trailing_zeros(self):
self.assertEqual([], truncate_trailing_zeros([]))
self.assertEqual([], truncate_trailing_zeros([0]))
self.assertEqual([0, 1], truncate_trailing_zeros([0, 1]))
self.assertEqual([0, 1], truncate_trailing_zeros([0, 1, 0]))
self.assertEqual([1, -1], truncate_trailing_zeros([1, -1, 0, 0]))

def test_encode_exponential_histogram(self):
exponential_histogram = Metric(
name="exponential_histogram",
Expand All @@ -709,8 +719,8 @@ def test_encode_exponential_histogram(self):
sum=3,
scale=4,
zero_count=5,
positive=Buckets(offset=6, bucket_counts=[7, 8]),
negative=Buckets(offset=9, bucket_counts=[10, 11]),
positive=Buckets(offset=6, bucket_counts=[7, 8, 0]),
negative=Buckets(offset=9, bucket_counts=[10, 11, 0, 0]),
flags=12,
min=13.0,
max=14.0,
Expand Down