Skip to content

Conversation

@DmitriyH
Copy link
Contributor

@DmitriyH DmitriyH commented Oct 4, 2024

Implementation of values total sum calculation in histograms.

The total sum of values is required to comply with Prometheus format standard:
https://prometheus.io/docs/concepts/metric_types/#histogram

Also, sum allows to calculate the average value, which could be useful for some metrics.

The corresponding issue:
#707

@github-actions
Copy link

github-actions bot commented Oct 4, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@DmitriyH
Copy link
Contributor Author

DmitriyH commented Oct 4, 2024

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/

@Anton3
Copy link
Member

Anton3 commented Jan 22, 2025

An additional atomic counter per bucket is redundant, as we only need the total sum.

But a single additional counter will cause additional contention for formats where the sum is not written, or if exact sums are not important.

Instead of a single counter, we could add a StripedCounter, but it will take extra space in case it's not needed.

As a compromise, I suggest the following:

  1. Add the extra std::atomic<double> sum counter to the layout of Histogram, 1 counter per histogram.
  2. Add a global std::atomic<bool> flag with a function for setting its value. If true, then sum is mutated in addition to the normal buckets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants