-
Notifications
You must be signed in to change notification settings - Fork 14
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
middleware(prometheus): use summaries instead of histograms #975
Conversation
It is much more efficient to use summaries than histograms. This would give information on p50, p90 and p99. Fixes #925
386b16c
to
33cdac3
Compare
Can you provide information on how much time |
Once we deploy we can do some new benchmarking, to compare apples to apples. |
# TYPE test_http_request_size_bytes summary | ||
test_http_request_size_bytes{code="200",method="GET",quantile="0.5"} 5171 | ||
test_http_request_size_bytes{code="200",method="GET",quantile="0.9"} 102451 | ||
test_http_request_size_bytes{code="200",method="GET",quantile="0.99"} 102451 |
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.
Request size does not look useful in a summary like this.
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.
Why?
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.
It's not about summary, but rather request size itself.
While duration may outline some slower requests, size does not matter that much.
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.
You suggest we remove it altogether?
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.
Counting the sizes requires some technical prep work (wrapping) we could get rid 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.
I'm hesitating, but it looks like we could get rid of it.
73b686e
to
2af155e
Compare
2af155e
to
0702551
Compare
@Choraden PTAL |
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.
LGTM
It is much more efficient to use summaries than histograms. This would give information on p50, p90 and p99.
Fixes #925