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 2020/10/16 19:06:37 UTC

[GitHub] [apisix] tzssangglass opened a new pull request #2443: feature: prometheus plugin `apisix_http_status` metric `route` tag Im…

tzssangglass opened a new pull request #2443:
URL: https://github.com/apache/apisix/pull/2443


   …prove recognition (#1574)
   
   fix #1574
   
   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   Prometheus plugin `apisix_http_status` metric `route` tag Improve recognition.
   
   Originally, the `/apisix/prometheus/metrics` endpoint only showed the `id` of the `router`.
   However, if the route has `uris` or `hosts` parameters, the user does not know the number of times an element in `uris` or `hosts` has been matched, the collection of coarse granularity, and in Grafana to display the `id` of the `route` is not friendly enough.
   
   This PR adds two elements, `matched_uri` and `matched_host`, to the `apisix_http_status` property of the `/apisix/prometheus/metrics` endpoint to show in detail the elements that are matched in the `uris` or `hosts`.
   Note: I added the parameter `curr_req_matched_record` to the `api_ctx` to keep track of the `route` record for the current request match.
   
   The `apisix_http_status` will look like this
   
   ```
   apisix_http_status{code="200",route="",matched_uri="",matched_host="",service="localhost",node=""} 5
   apisix_http_status{code="200",route="1",matched_uri="/hello",matched_host="",service="",node=""} 6
   apisix_http_status{code="200",route="1",matched_uri="/hello",matched_host="",service="",node="127.0.0.1"} 20
   apisix_http_status{code="200",route="2",matched_uri="/hello1",matched_host="",service="1",node="127.0.0.1"} 6
   apisix_http_status{code="404",route="",matched_uri="",matched_host="",service="localhost",node=""} 6
   apisix_http_status{code="404",route="3",matched_uri="/hello3",matched_host="",service="",node="127.0.0.1"} 2
   ```
   
   [#1574](https://github.com/apache/apisix/issues/1574)
   
   <!--- If it fixes an open issue, please link to the issue here. -->
   
   ### Pre-submission checklist:
   
   * [x] 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?
   * [x] Is this PR backward compatible?
   


----------------------------------------------------------------
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] tzssangglass closed pull request #2443: feature: prometheus plugin `apisix_http_status` metric `route` tag Im…

Posted by GitBox <gi...@apache.org>.
tzssangglass closed pull request #2443:
URL: https://github.com/apache/apisix/pull/2443


   


----------------------------------------------------------------
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] tzssangglass commented on pull request #2443: feature: prometheus plugin `apisix_http_status` metric `route` tag Im…

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


   @membphis @moonming @nic-chen please help me, take a look at CI error:https://github.com/apache/apisix/pull/2443/checks?check_run_id=1266120426#step:6:417
   I don't think my changes have anything to do with the failure of these test cases.
   


----------------------------------------------------------------
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] tzssangglass commented on pull request #2443: feature: prometheus plugin `apisix_http_status` metric `route` tag Im…

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


   > @tzssangglass I think your new code is wrong. please check it first.
   > 
   > https://github.com/apache/apisix/pull/2443/checks?check_run_id=1266120426#step:6:373
   
   Yes, you're right, I found my error through debugging.


----------------------------------------------------------------
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] tzssangglass commented on pull request #2443: feature: prometheus plugin `apisix_http_status` metric `route` tag Im…

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


   @membphis please take a look at this CI error:https://github.com/apache/apisix/pull/2443/checks?check_run_id=1266120426#step:6:153 and so on  
   my commit did not change the code associated with this test case
   I don't know what this error has to do with `prove -Itest-nginx/lib -r t`.
   


----------------------------------------------------------------
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] membphis commented on pull request #2443: feature: prometheus plugin `apisix_http_status` metric `route` tag Im…

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


   @tzssangglass I think your new code is wrong. please check it first.
   
   https://github.com/apache/apisix/pull/2443/checks?check_run_id=1266120426#step:6:373


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