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 2021/04/06 08:59:20 UTC

[GitHub] [apisix] Yiyiyimu opened a new pull request #3993: fix: update prometheus to also expose upstream latency & update grafana

Yiyiyimu opened a new pull request #3993:
URL: https://github.com/apache/apisix/pull/3993


   Signed-off-by: yiyiyimu <wo...@gmail.com>
   
   ### What this PR does / why we need it:
   
   Update point:
   1. in Prometheus metric add upstream latency, rename latency -> request latency, overhead -> apisix latency
   2. move legend to the right with sort on total
   3. split status code
   4. delete batch process metric since it's optional
   
   ### Pre-submission checklist:
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3993: change(prometheus): redesign the latency metrics & update grafana

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3993:
URL: https://github.com/apache/apisix/pull/3993#discussion_r608384672



##########
File path: docs/en/latest/plugins/prometheus.md
##########
@@ -151,27 +153,17 @@ Or you can goto [Grafana official](https://grafana.com/grafana/dashboards/11719)
 * `etcd reachability`: A gauge type with a value of 0 or 1, representing if etcd can be reached by a APISIX or not, where `1` is available, and `0` is unavailable.
 * `Connections`: Various Nginx connection metrics like active, reading, writing, and number of accepted connections.
 * `Batch process entries`: A gauge type, when we use plugins and the plugin used batch process to send data, such as: sys logger, http logger, sls logger, tcp logger, udp logger and zipkin, then the entries which hasn't been sent in batch process will be counted in the metrics.
-* `Latency`: The per service histogram of request time and the overhead added by APISIX (request time - upstream response time).
+* `Latency`: The per service histogram of request time in different dimensions.
 
     Attributes:
 
     | Name      | Description |
     | ----------| ------------- |
-    | type      | Its value is fixed as `request`, which means HTTP request. |
+    | type      | The optional values are `apisix`, `upstream` and `request`, which means http latency caused by apisix, upstream, and in total. |

Review comment:
       "The value can be `apisix`, `upstream` or `request`, which means http latency caused by apisix, upstream, or their sum." would be better?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander merged pull request #3993: change(prometheus): redesign the latency metrics & update grafana

Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #3993:
URL: https://github.com/apache/apisix/pull/3993


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3993: change(prometheus): redesign the latency metrics & update grafana

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3993:
URL: https://github.com/apache/apisix/pull/3993#discussion_r608391271



##########
File path: docs/en/latest/plugins/prometheus.md
##########
@@ -151,27 +153,17 @@ Or you can goto [Grafana official](https://grafana.com/grafana/dashboards/11719)
 * `etcd reachability`: A gauge type with a value of 0 or 1, representing if etcd can be reached by a APISIX or not, where `1` is available, and `0` is unavailable.
 * `Connections`: Various Nginx connection metrics like active, reading, writing, and number of accepted connections.
 * `Batch process entries`: A gauge type, when we use plugins and the plugin used batch process to send data, such as: sys logger, http logger, sls logger, tcp logger, udp logger and zipkin, then the entries which hasn't been sent in batch process will be counted in the metrics.
-* `Latency`: The per service histogram of request time and the overhead added by APISIX (request time - upstream response time).
+* `Latency`: The per service histogram of request time in different dimensions.
 
     Attributes:
 
     | Name      | Description |
     | ----------| ------------- |
-    | type      | Its value is fixed as `request`, which means HTTP request. |
+    | type      | The optional values are `apisix`, `upstream` and `request`, which means http latency caused by apisix, upstream, and in total. |

Review comment:
       Thanks for your suggestions and it is indeed more fluent! Fixed




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3993: fix: update prometheus to also expose upstream latency & update grafana

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3993:
URL: https://github.com/apache/apisix/pull/3993#issuecomment-813956029


   Waiting to update new screenshots


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu edited a comment on pull request #3993: fix: update prometheus to also expose upstream latency & update grafana

Posted by GitBox <gi...@apache.org>.
Yiyiyimu edited a comment on pull request #3993:
URL: https://github.com/apache/apisix/pull/3993#issuecomment-813956029


   ~Waiting to update new screenshots~
   
   Updated


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3993: fix: update prometheus to also expose upstream latency & update grafana

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3993:
URL: https://github.com/apache/apisix/pull/3993#issuecomment-814133875


   To make it clear, put the updated images here
   ![1-nginx](https://user-images.githubusercontent.com/34589752/113720997-bb7a4700-9721-11eb-9709-6ddb839aaebc.png)
   ![2-bandwidth](https://user-images.githubusercontent.com/34589752/113721004-bcab7400-9721-11eb-9955-24f3b98828c4.png)
   ![3-http](https://user-images.githubusercontent.com/34589752/113721008-bd440a80-9721-11eb-9347-25013bbdb6a8.png)
   ![4-Misc](https://user-images.githubusercontent.com/34589752/113721013-bddca100-9721-11eb-9747-5a8db2844205.png)
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org