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

Exemplars for Exponential LongHistogram always have value 0. #6010

Closed
fstab opened this issue Nov 23, 2023 · 1 comment · Fixed by #6024
Closed

Exemplars for Exponential LongHistogram always have value 0. #6010

fstab opened this issue Nov 23, 2023 · 1 comment · Fixed by #6024
Labels
Bug Something isn't working

Comments

@fstab
Copy link
Member

fstab commented Nov 23, 2023

I realized this while working on unit tests for #5940:

In order to switch to exponential histograms, you register a View with aggregation

Aggregation.base2ExponentialBucketHistogram()

However, under the hood there's only one implementation: DoubleBase2ExponentialHistogramAggregator. This is used for both DoubleHistogram and LongHistogram.

As a result, if you have a LongHistogram the Exemplars always have value 0, because the aggregation always uses ReservoirCell.doubleValue and never ReservoirCell.longValue.

It looks like a LongBase2ExponentialHistogramAggregator is missing.

@fstab fstab added the Bug Something isn't working label Nov 23, 2023
@jack-berg
Copy link
Member

Good catch! I just looked at the source code and it looks like the explicit bucket histogram aggregation doesn't suffer from this problem because it takes advantage of the exemplar reservoir being HistogramExemplarReservoir, a purpose built reservoir which knows that only double histograms are exported and casts the long measurement value to a double before recording in the reservoir.

Something similar is needed for exponential histograms, but even the explicit bucket histogram solution is brittle because it would be subject to the same failure mode once we make exemplar reservoirs configurable via views. We really need both explicit and exponential bucket histograms to always cast measurements to doubles before trying to record in the exemplar reservoir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
2 participants