-
Notifications
You must be signed in to change notification settings - Fork 1k
Document histograms and histogram_quantile. #43
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
Conversation
for latency, for example). Note that the number of observations (showing up in | ||
Prometheus as a time series with a `_count` suffix) is inherently a counter (as | ||
described above, it only goes up), while the sum of observations (showing up as | ||
a time series with a `_sum` suffix) is inherently a gauge (if a negative value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit messy, the cases where it won't be a counter are very small - and in that case the _sum is of questionable use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the next commit, I'll try to make the usual case clearer without denying the possibility of negative observations.
I'll comment and rework the text ASAP. Just very busy with other stuff right now. Will get back to this tomorrow or so. |
vs. histogram](/docs/practices/instrumentation/#summary-vs.-histogram) for | ||
details of histogram usage and differences to [summaries](#summary). | ||
|
||
Client library usage documentation for summaries: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/for summaries/for histograms/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
Still polishing.... commit might only happen tonight. Stay tuned. :) |
c7496e6
to
6efe3d3
Compare
Sorry for the longer-then-expected delay... Here is the heavily reworked version. Most importantly, I have created a completely new section for the delicate subject of histograms and |
# Histograms and summaries | ||
|
||
Histograms and summaries are more complex metric types. Not only | ||
creates a single histogram or summary a multitude of time series, it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar is off here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which way? Need language assistance...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Not only creates a single histogram" is missing a subject, or at least it seems that way, because I think you mean "histogram or summary" to be the subject, but the location they appear in makes them seem like the object.
Probably you want to say "Not only do they create a multitude of time series, it is also ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, the "do" is missing. http://dictionary.cambridge.org/grammar/british-grammar/not-only-but-also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, also the subject is in the wrong position, and the verb is not conjugated correctly? I would just copy my suggestion :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"a single histogram or summary" is clearly singular. A "does" was missing, and then "creates" becomes infinite and goes to a different place. The subject was correctly placed from the beginning.
Should be good now.
cannot apply `rate()` to it anymore. | ||
|
||
To calculate the average request duration during the last 5 minutes | ||
from a histogram or summary called `http_request_duration_second`, use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/http_request_duration_second/http_request_duration_seconds/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
The essential difference between summaries and histograms is that summaries | ||
calculate streaming φ-quantiles on the client side and expose them directly, | ||
while histograms expose bucketed observations counts and the calculation of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"observations counts" -> "observation counts"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DOne.
715b75b
to
924cad3
Compare
PHAL |
👍 |
Most importantly, created a completely new section for histograms and summaries and updated all the references. Also add other minor improvements.
924cad3
to
c532c7f
Compare
Document histograms and histogram_quantile.
OM1.0 required that the Sum of Histograms is not represented when there are negative observations in a histogram. This PR is removing this requirement in OM2.0. Due to: The requirement was never implemented by the Go and Java instrumentation libraries. Enforcing it now would be breaking. The requirement makes it impossible to implement the use case where the user wants to measure the Sum anyway. We already warned users in the documentation about the possibility of Sum decreasing and not being usable for rate() 10 years ago: #43. And native histograms will not take Sum into account when calculating counter resets during rate() , thus this problem won't come up. Note: this PR does not make Sum mandatory, that is a different question. Signed-off-by: György Krajcsovits <[email protected]>
@juliusv @brian-brazil