fix(prometheus): conflict between global rule and route configure#6579
Conversation
| 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.
tokers
left a comment
tokers
left a comment
There was a problem hiding this comment.
The way do work for fixing the problem but my concern is that are there other plugins have the same problem? Shall we have a generic mechanism to handle this situation?
| 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 |
| 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?
| --- 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...
5 participants
What this PR does / why we need it:
FIX #6555
Pre-submission checklist: