remove unnecessary buckets on exponential histograms #3771
Conversation
There was a problem hiding this comment.
AFAIU for reference the js sdk implements the trimming on the bucket and calls it when merging or diffing exponential histograms:
https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/sdk-metrics/src/aggregator/exponential-histogram/Buckets.ts#L116
|
@euroelessar any chance you can take a look at this please? |
|
unfortunately this PR has to be rebased due to #3798 being merged |
|
@loomis-relativity any chance you can rebase your changes please? |
|
@xrmx Merge conflict resolved. |
|
@euroelessar hello, do you have time to take a look at it? |
| @@ -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): | |||
There was a problem hiding this comment.
maybe move truncation logic into def bucket_counts implementation to avoid duplication?
Description
Updates the serialization of exponential histogram buckets to remove unnecessary zero buckets. If there are no non-zero buckets, then none are serialized. This matches better the payloads from the JavaScript SDK and avoids ingestion problems with a downstream vendor when an empty negative bucket appears.
Fixes #3767
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Added unit tests to ensure that the truncation of the zero buckets works as expected. Updated the unit test for metric encoding to exercise this functionality.
Does This PR Require a Contrib Repo Change?
Answer the following question based on these examples of changes that would require a Contrib Repo Change:
The OTel specification has changed which prompted this PR to update the method interfaces of
opentelemetry-api/oropentelemetry-sdk/The method interfaces of
test/utilhave changedScripts in
scripts/that were copied over to the Contrib repo have changedConfiguration files that were copied over to the Contrib repo have changed (when consistency between repositories is applicable) such as in
pyproject.tomlisort.cfg.flake8When a new
.github/CODEOWNERis addedMajor changes to project information, such as in:
README.mdCONTRIBUTING.mdYes. - Link to PR:
No.
Checklist:
[ ] Documentation has been updated