You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/02/11 10:39:54 UTC
[GitHub] [apisix] nic-6443 opened a new issue #6301: bug: some mistakes in prometheus plugin
nic-6443 opened a new issue #6301:
URL: https://github.com/apache/apisix/issues/6301
### Issue description
The APISIX prometheus plugin uses the `/nginx_status` endpoint provided by nginx to get some internal metrics, which return the data like
```
Active connections: 291
server accepts handled requests
Reading: 6 Writing: 179 Waiting: 179
Reading: 6 Writing: 179 Waiting: 106
```
According to the [nginx documentation](https://nginx.org/libxslt/en/docs/http/ngx_http_stub_status_module.html):
> The following status information is provided:
>
> Active connections
> The current number of active client connections including Waiting connections.
> accepts
> The total number of accepted client connections.
> handled
> The total number of handled connections. Generally, the parameter value is the same as accepts unless some resource limits have been reached (for example, the worker_connections limit).
> requests
> The total number of client requests.
> Reading
> The current number of connections where nginx is reading the request header.
> Writing
> The current number of connections where nginx is writing the response back to the client.
> Waiting
> The current number of idle client connections waiting for a request.
The third number in the third line is `The total number of client requests`, but APISIX now puts this metric in `nginx_http_current_connections{label="total"}`, which is obviously wrong.
Also the three metrics in the third line are all cumulative, should we use the counter type instead of the guage for those metrics? Which is more in line with the prometheus definition of metric types.
So I want to introduce two new metric:
1. `nginx_http_requests_total`(counter type)
2. `nginx_http_connections_total`(counter type) with a `type` label, which has two values: `accepts`, `handled`
### Environment
- apisix version (cmd: `apisix version`): 2.10.1
### Steps to reproduce
Just enable prometheus plugin.
### Actual result
/
### Error log
/
### Expected result
_No response_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038558231
I think `nginx_http_requests_total` or `http_requests_total` is fine, because we will add an `apisix` prefix by default (the user can customize this prefix configuration in the plugin configuration), so there is no need to repeat it.
https://github.com/apache/apisix/blob/0920d1d21fba36947b4ce13438040874c5d37610/apisix/plugins/prometheus/exporter.lua#L79-L85
@spacewander What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038073587
Yes, it is good.
> nginx_http_requests_total
Do you mean `apisix_http_requests_total`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1040072532
> I found that I couldn't use the `counter` type when coding, because the metrics are directly obtain from nginx, there is no accumulation process, and the Prometheus counter type has no `set` method, it can only accumulate the count. For this reason, I just introduced a new metric `http_requests_total`, which is of type guage.
Can we just record the value and next time after we get the new value, let's accumulate the counter with the increments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038073587
Yes, it is good.
> nginx_http_requests_total
Do you mean `apisix_http_requests_total`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038686037
I will vote for `http_requests_total`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038542214
> Yes, it is good.
>
> > nginx_http_requests_total
>
> Do you mean `apisix_http_requests_total`?
I think it's OK to use `apisix` as a prefix, but the previous metric used `nginx` as a prefix, do you think it needs to be changed together?
https://github.com/apache/apisix/blob/0920d1d21fba36947b4ce13438040874c5d37610/apisix/plugins/prometheus/exporter.lua#L86-L88
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038542214
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1040030416
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1040030416
I found that I couldn't use the `counter` type when coding, because the metrics are directly obtain from nginx, there is no accumulation process, and the Prometheus counter type has no `set` method, it can only accumulate the count.
For this reason, I just introduced a new metric `http_requests_total`, which is of type guage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1040072532
> I found that I couldn't use the `counter` type when coding, because the metrics are directly obtain from nginx, there is no accumulation process, and the Prometheus counter type has no `set` method, it can only accumulate the count. For this reason, I just introduced a new metric `http_requests_total`, which is of type guage.
Can we just record the value and next time after we get the new value, let's accumulate the counter with the increments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1038693977
> I will vote for `http_requests_total`.
Got it. As discussed above, I will be introducing two new metrics:
1. `http_requests_total`(counter type)
2. `nginx_http_connections_total`(counter type) with a `type` label, which has two values: `accepts`, `handled`
Also, the `state` label in the old `nginx_http_current_connections` metric will only have four values: `active`/ `reading`/ `writing`/ `waiting` and the `accepted`/ `handled`/ `total` will be removed.
I will submit this PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-6443 commented on issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on issue #6301:
URL: https://github.com/apache/apisix/issues/6301#issuecomment-1040086133
> > I found that I couldn't use the `counter` type when coding, because the metrics are directly obtain from nginx, there is no accumulation process, and the Prometheus counter type has no `set` method, it can only accumulate the count. For this reason, I just introduced a new metric `http_requests_total`, which is of type guage.
>
> Can we just record the value and next time after we get the new value, let's accumulate the counter with the increments.
I feel that this is complicated and unnecessary, because metric type is only as a hint, and does not affect the subsequent data storage and processing, even if we use the `guage` type, can still use functions such as `rate` to handle, like the metric of the counter type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander closed issue #6301: bug: some mistakes in prometheus plugin
Posted by GitBox <gi...@apache.org>.
spacewander closed issue #6301:
URL: https://github.com/apache/apisix/issues/6301
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org