feat(prometheus): extra labels from nginx var for http request metrics #7549
Conversation
conf/config-default.yaml
Outdated
| @@ -522,6 +522,14 @@ plugin_attr: | |||
| export_addr: | |||
| ip: 127.0.0.1 | |||
| port: 9091 | |||
| # ngx_var_labels: | |||
There was a problem hiding this comment.
I think we don't need to highlight the var. Instead, we can just use the name custom_labels.
conf/config-default.yaml
Outdated
| @@ -522,6 +522,14 @@ plugin_attr: | |||
| export_addr: | |||
| ip: 127.0.0.1 | |||
| port: 9091 | |||
| # custom_labels: | |||
There was a problem hiding this comment.
Would be better to use:
metrics:
xxx:
extra_labels:
label_name_xxx: $label_var
yyy:
So we can add more options to a metric in the future. And we don't need to require the label name should be the variable name.
There was a problem hiding this comment.
And please add a comment to show that the commented configurations are the example but not the default value.
There was a problem hiding this comment.
The labels of extra_labels should be a list, because fields of dictionary in lua table are out of order, which would cause the metric full name may change each time it reloads the conf.
There was a problem hiding this comment.
We can provide order on the implementation side, for example sorting them before using them.
| for _, name in ipairs(labels) do | ||
| local val = ctx.var[name] | ||
| if val == nil then | ||
| val = "nil" |
There was a problem hiding this comment.
I would suggest using "", which is language-independent.
t/plugin/prometheus4.t
Outdated
| end | ||
| } | ||
| } | ||
| --- error_code: 200 |
There was a problem hiding this comment.
Actually we don't need to specify --- error_code: 200 as it is checked by default
t/plugin/prometheus4.t
Outdated
| custom_labels: | ||
| http_status: | ||
| - dummy | ||
| bandwidth: |
There was a problem hiding this comment.
We don't need the bandwidth?
Co-authored-by: tzssangglass <[email protected]>
t/plugin/prometheus4.t
Outdated
| - upstream_status: $upstream_status | ||
| --- request | ||
| GET /hello | ||
| --- error_code: 200 |
There was a problem hiding this comment.
Actually we don't need to specify --- error_code: 200 as it is checked by default
t/plugin/prometheus4.t
Outdated
|
|
||
|
|
||
|
|
||
| === TEST 6: fetch the prometheus metric data, with nil label |
There was a problem hiding this comment.
It is no longer a nil label?
docs/en/latest/plugins/prometheus.md
Outdated
| @@ -53,6 +53,29 @@ plugin_attr: | |||
| export_uri: /apisix/metrics | |||
| ``` | |||
|
|
|||
| ### Specifying `metrics` | |||
|
|
|||
| For http request related metrics, you could specify extra labels, which match the nginx variables. | |||
There was a problem hiding this comment.
Let's use APISIX variable like elsewhere in the doc.
apache#7549) close apache#4273 Co-authored-by: tzssangglass <[email protected]>
close #4273
Description
Add extra labels from nginx variables for http request metrics.
Fixes #4273
Checklist