-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add support for Percentile Stats lines #11
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Antoine Leroyer <[email protected]>
Signed-off-by: Antoine Leroyer <[email protected]>
4e17db7
to
e579ec0
Compare
Signed-off-by: Antoine Leroyer <[email protected]>
The data provided reasonably resembles the shape of a Prometheus |
Also as a general terminology note, "bucket" is strongly associated with histograms, not quantiles. I would like to avoid mixing these to avoid confusion |
Thanks for the feedback @matthiasr.
I agree with For example, I can rename this
into this
However, As stated in Prometheus documentation:
But rsyslog here is exposing the
Will this be an issue? If yes, I will need to do the sum/count part in the exporter but I don't think I can obtain something accurate. |
Yes they need to be cumulative. You can create a complete summary with NewConstSummary, but you will need the aggregated sum and count for that. The way I am used to doing that is to hold the values in a separate (map of) struct(s), then implement the custom collector interface to only construct the actual metric objects at scrape time How does the window relate to the reporting interval? If we get the count for the window every time at the end of the window, summing them all up would be "good enough". If somehow an increment gets lost, nothing we can do about it. OTOH if the exporter restarts and the count starts from zero again, that's OK as long as it doesn't happen too frequently. |
This is a first attempt in supporting Percentile Stats.
Not entirely satisfied for now as the exporter doesn't support multiple labels per time series (this will need bigger changes) but this should be sufficient to fix #10.
Samples: