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.

Sign up for GitHub

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

Jump to bottom

Summary quantiles respond +Inf if rps decreases to zero #303

Closed
DifferentialOrange opened this issue Sep 16, 2021 · 3 comments · Fixed by #305
Closed

Summary quantiles respond +Inf if rps decreases to zero #303

DifferentialOrange opened this issue Sep 16, 2021 · 3 comments · Fixed by #305
Assignees
@DifferentialOrange
Labels
customer

Comments

@DifferentialOrange
Copy link
Member

If there are 0 new requests for some time, summary collector quantiles respond with +Inf values.

image

http_server_request_latency{path="/hello",method="GET",alias="tnt_router",status="200",quantile="0.5"} +Inf
http_server_request_latency{path="/hello",method="GET",alias="tnt_router",status="200",quantile="0.9"} +Inf
http_server_request_latency{path="/hello",method="GET",alias="tnt_router",status="200",quantile="0.99"} +Inf
http_server_request_latency{path="/goodbye",method="POST",alias="tnt_router",status="500",quantile="0.5"} +Inf
http_server_request_latency{path="/goodbye",method="POST",alias="tnt_router",status="500",quantile="0.9"} +Inf
http_server_request_latency{path="/goodbye",method="POST",alias="tnt_router",status="500",quantile="0.99"} +Inf
http_server_request_latency{path="/hell0",method="GET",alias="tnt_router",status="400",quantile="0.5"} +Inf
http_server_request_latency{path="/hell0",method="GET",alias="tnt_router",status="400",quantile="0.9"} +Inf
http_server_request_latency{path="/hell0",method="GET",alias="tnt_router",status="400",quantile="0.99"} +Inf

It breaks visualization with ugly vertical lines. I'm sure there are better approaches to handle this.

Reproducer

You may use https://github.com/tarantool/grafana-dashboard docker cluster and stop load container after several minutes.

docker-compose up
# let it generate some metrics
docker stop grafana-dashboard_load_generator_1
@yngvar-antonsson
Copy link
Collaborator

Now it's a part of the quantile algorithm and covers with tests:

metrics/test/quantile_test.lua

Line 144 in 7652824

g.test_query_on_empty_quantile = function()

We could convert values from empty quantile collector to null.

@DifferentialOrange
Copy link
Member Author

Now it's a part of the quantile algorithm and covers with tests:

metrics/test/quantile_test.lua

Line 144 in 7652824

g.test_query_on_empty_quantile = function()

We could convert values from empty quantile collector to null.

I think it is a good idea

@yngvar-antonsson
Copy link
Collaborator

yngvar-antonsson commented Sep 16, 2021 •
edited
Loading

metrics/metrics/quantile.lua

Line 158 in 7652824

function stream:query(q) 

if s.n == 0 then
  return nil
end

Since stream:query function is called after buffer flush, if stream is empty (== 0) then there was no insertion in collector.

@DifferentialOrange

@DifferentialOrange DifferentialOrange self-assigned this Sep 17, 2021
DifferentialOrange added a commit that referenced this issue Sep 17, 2021
@DifferentialOrange
Set quantile value to nil if there are no samples provided for an age
9883725 
Closes #303
@DifferentialOrange DifferentialOrange mentioned this issue Sep 17, 2021
Merged
4 tasks
DifferentialOrange added a commit that referenced this issue Sep 17, 2021
@DifferentialOrange
Set quantile value to nil if there are no samples provided for an age
4b04408 
Closes #303
DifferentialOrange added a commit that referenced this issue Sep 21, 2021
@DifferentialOrange
Set quantile value to NAN if there are no samples provided for an age
23d8391 
Closes #303
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 18, 2022
@DifferentialOrange
stats: separate quantile and average fields
ebc730a 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 18, 2022
@DifferentialOrange
stats: separate quantile and average fields
8fe6dbb 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 18, 2022
@DifferentialOrange
stats: separate quantile and average fields
4429a00 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 18, 2022
@DifferentialOrange
stats: separate quantile and average fields
2e5113d 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
@DifferentialOrange DifferentialOrange mentioned this issue May 18, 2022
Merged
3 tasks
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 18, 2022
@DifferentialOrange
stats: separate quantile and average fields
85e6ac8 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 19, 2022
@DifferentialOrange
stats: separate quantile and average fields
95e6c30 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 20, 2022
@DifferentialOrange
stats: separate quantile and average fields
35ac849 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 20, 2022
@DifferentialOrange
stats: separate quantile and average fields
4a76d92 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
DifferentialOrange added a commit to tarantool/crud that referenced this issue May 20, 2022
@DifferentialOrange
stats: separate quantile and average fields
c66d330 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

Closes #286
Totktonada pushed a commit to tarantool/crud that referenced this issue May 20, 2022
@DifferentialOrange @Totktonada
stats: separate quantile and average fields
e709a87 
Add separate quantile (`latency_quantile_recent`) and average (
`latency_average`) fields to `crud.stats()` output. `latency` field and
tarantool/metrics output remains unchanged.

Before this patch, `latency` displayed `latency_quantile_recent` or
`latency_average` and there wasn't any was to see pre-computed average
if quantiles are enabled. But it may be useful if quantile is `nan`.

Quantiles may display `-nan` if there were no observations for a several
ages. Such behavior is expected [1] and valid: for example, Grafana
should ignore such values and they will be displayed as `No data` for
a window when there wasn't any requests.

1. tarantool/metrics#303

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

@DifferentialOrange DifferentialOrange

Labels
customer
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set quantile value to NAN if there are no samples provided for an age tarantool/metrics
2 participants
@DifferentialOrange @yngvar-antonsson