fix(prometheus): conflict between global rule and route configure #6579
Conversation
| @@ -39,6 +39,7 @@ local _M = { | |||
| name = plugin_name, | |||
| log = exporter.log, | |||
| schema = schema, | |||
| run_policy = "prefer_route", | |||
There was a problem hiding this comment.
Does the nginx-lua-prometheus need update ?
There was a problem hiding this comment.
Does the
nginx-lua-prometheusneed update ?
No, APISIX from 2.7 to master, nginx-lua-prometheus has only one version upgrade, and this upgrade of nginx-lua-prometheus is not related to the current issue.
https://github.com/knyar/nginx-lua-prometheus/blob/master/CHANGELOG.md#020220127
| @@ -171,6 +171,8 @@ function _M.log(conf, ctx) | |||
|
|
|||
| metrics.bandwidth:inc(vars.bytes_sent, | |||
| gen_arr("egress", route_id, service_id, consumer_name, balancer_ip)) | |||
|
|
|||
| core.log.info("prometheus run report") | |||
There was a problem hiding this comment.
Can we check the result of prometheus instead of adding a new log just for testing?
There was a problem hiding this comment.
this is not just for testing purposes. Currently, there is no log output when prometheus reports data. I think adding this log will also help developers watch it while developing and debugging.
what do you think?
There was a problem hiding this comment.
A hit will increase the metric:
apisix/apisix/plugins/prometheus/exporter.lua
Line 153 in 1ac7166
There is no need to add another way.
This has been discussed before. The |
t/plugin/prometheus3.t
Outdated
| qr/prometheus run \w+/ | ||
| --- grep_error_log_out | ||
| prometheus run report | ||
| --- error_code: 200 |
There was a problem hiding this comment.
The error_code check here is redundant?
t/plugin/prometheus3.t
Outdated
| --- request | ||
| GET /apisix/prometheus/metrics | ||
| --- response_body eval | ||
| qr/apisix_http_status\{code="200",route="1",matched_uri="\/opentracing",matched_host="",service="",consumer="",node="127.0.0.1\"} \d+/ |
There was a problem hiding this comment.
It doesn't check the number this request is hit...
What this PR does / why we need it:
FIX #6555
Pre-submission checklist: