You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "janiussyafiq (via GitHub)" <gi...@apache.org> on 2026/04/13 04:40:37 UTC
[PR] feat(prometheus): support disabling labels and metrics to reduce cardinality [apisix]
janiussyafiq opened a new pull request, #13202:
URL: https://github.com/apache/apisix/pull/13202
### Description
Add two new per-metric configuration options to the Prometheus plugin's `plugin_attr`:
- **`disable_labels`**: A list of built-in label names to drop from a metric, reducing cardinality without changing the metric schema for other labels.
- **`disable`**: A boolean flag to skip registering an entire metric, removing it from `/metrics` output entirely.
Both options are configured under `plugin_attr.prometheus.metrics.<metric_name>` alongside the existing `expire` and `extra_labels` fields.
Example config:
```yaml
plugin_attr:
prometheus:
metrics:
http_status:
disable_labels:
- node
- consumer
bandwidth:
disable: true
```
This addresses high-cardinality issues in dynamic environments (e.g. Kubernetes autoscaling where pod IPs churn rapidly), which can cause prometheus shared dict overflow and excessive memory consumption.
#### Which issue(s) this PR fixes:
Fixes #12679
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "janiussyafiq (via GitHub)" <gi...@apache.org>.
janiussyafiq commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3382844653
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,61 @@ local function extra_labels(name, ctx)
end
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+}
+
+
+local function get_disabled_label_metric_map()
+ local attr = plugin.plugin_attr("prometheus")
+ local metrics_conf = attr and attr.metrics or {}
+ local disabled_label_metric_map = {}
+ for metric_name, metric_conf in pairs(metrics_conf) do
+ if metric_conf.disable_labels then
+ disabled_label_metric_map[metric_name] = {}
+ for _, label in ipairs(metric_conf.disable_labels) do
+ disabled_label_metric_map[metric_name][label] = true
+ end
+ end
+ end
Review Comment:
Addressed in the latest commit. Configuration moved from `plugin_attr` to plugin **metadata** (`disabled_labels`), which is now validated by a `metadata_schema` (`type: object`/`array`, per-metric `enum`, `additionalProperties: false`). Invalid input — wrong types, unknown metric keys, or non-disable-able labels — is now rejected at config time by the Admin API instead of erroring per request in the datapath.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,61 @@ local function extra_labels(name, ctx)
end
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
Review Comment:
Addressed. `metric_label_map` is now the single source of truth: every metric is registered in `http_init` via `append_tables(metric_label_map.<name>, extra_labels(...))` and the duplicated inline label lists were removed. Registration and value-collapsing now read the same array, so their ordering can no longer drift.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -292,6 +347,7 @@ end
function _M.http_log(conf, ctx)
local vars = ctx.var
+ local disabled_label_metric_map = get_disabled_label_metric_map()
Review Comment:
Addressed. `get_disabled_label_metric_map()` now reads the plugin metadata and caches the built lookup with `core.lrucache` keyed by `metadata.modifiedIndex`, so it is rebuilt only when the configuration changes rather than on every request — the same pattern used by `error-log-logger`/`cors`.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -771,12 +838,15 @@ local function inc_llm_active_connections(ctx, value)
matched_host = ctx.curr_req_matched._host or ""
end
+ local disabled_label_metric_map = get_disabled_label_metric_map()
+
Review Comment:
Addressed. `inc_llm_active_connections` calls the same cached `get_disabled_label_metric_map()`, so it reuses the lrucache-cached map and no longer allocates a fresh table on each increment/decrement.
##########
conf/config.yaml.example:
##########
@@ -635,17 +635,24 @@ plugin_attr: # Plugin attributes
refresh_interval: 15 # Set the interval for refreshing cached metric data. unit: second.
# metrics: # Create extra labels from nginx variables: https://nginx.org/en/docs/varindex.html
# http_status:
+ # disable_labels: # List of built-in label names to drop (reduces cardinality).
+ # - node # e.g. drop the upstream node IP label
Review Comment:
Addressed. `disable_labels` was removed from `plugin_attr` in `config.yaml.example` — the feature now lives in plugin metadata as `disabled_labels` — and the documentation describes it as collapsing the label *value* to an empty string while keeping the label registered. No "drop"/removal wording remains.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3385916204
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
+ type = "object",
+ properties = {
+ disabled_labels = {
+ type = "object",
+ properties = {
+ http_status = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "matched_uri", "matched_host", "service",
+ "consumer", "node", "request_type",
+ "request_llm_model", "llm_model", "response_source"},
+ },
+ },
+ http_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ bandwidth = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_prompt_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_completion_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_active_connections = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "route_id", "matched_uri", "matched_host",
Review Comment:
The `enum` lists in `metadata_schema` (`prometheus.lua`) are a manual copy of
`metric_label_map` (`exporter.lua`) minus the structural labels. They live in
different files and must be kept in sync by hand — easy to drift.
Suggestion: make `metric_label_map` the single source
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "janiussyafiq (via GitHub)" <gi...@apache.org>.
janiussyafiq commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3389984182
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
+ type = "object",
+ properties = {
+ disabled_labels = {
+ type = "object",
+ properties = {
+ http_status = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "matched_uri", "matched_host", "service",
+ "consumer", "node", "request_type",
+ "request_llm_model", "llm_model", "response_source"},
+ },
+ },
+ http_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ bandwidth = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_prompt_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_completion_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_active_connections = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "route_id", "matched_uri", "matched_host",
Review Comment:
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "janiussyafiq (via GitHub)" <gi...@apache.org>.
janiussyafiq commented on PR #13202:
URL: https://github.com/apache/apisix/pull/13202#issuecomment-5173704805
> Hello, We are evaluating this enhancement for our APISIX deployment and had a couple of questions:
>
> 1. Could you please confirm whether this feature is available in APISIX 3.17.0, or if it is planned to be included starting with APISIX 3.18.0 (or a later release)?
> 2. Has this change also been integrated into the official APISIX Helm Chart? If so, is there any additional Helm configuration required to enable and configure disabled_labels, or is it managed entirely through Prometheus plugin metadata?
>
> We are currently facing high Prometheus metric cardinality in production and are evaluating this feature as a potential solution. Any guidance would be appreciated. we have a very short time.
>
> @shreemaan-abhishek @janiussyafiq
@satngp
1. This feature is not yet available in APISIX 3.17.0, and expected to be available in the upcoming release (3.18.0).
2. Once this has been released, our maintainers will bump the APISIX version in apisix-helm-charts hence this feature would also be available.
cc @shreemaan-abhishek
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "liangbeirong-byron (via GitHub)" <gi...@apache.org>.
liangbeirong-byron commented on PR #13202:
URL: https://github.com/apache/apisix/pull/13202#issuecomment-4850139032
Hello,I'd like to ask if this change has been built into the apisix Helm chart?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "janiussyafiq (via GitHub)" <gi...@apache.org>.
janiussyafiq commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3398761361
##########
apisix/plugins/prometheus/exporter.lua:
##########
Review Comment:
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3385916204
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
+ type = "object",
+ properties = {
+ disabled_labels = {
+ type = "object",
+ properties = {
+ http_status = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "matched_uri", "matched_host", "service",
+ "consumer", "node", "request_type",
+ "request_llm_model", "llm_model", "response_source"},
+ },
+ },
+ http_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ bandwidth = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_prompt_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_completion_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_active_connections = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "route_id", "matched_uri", "matched_host",
Review Comment:
The `enum` lists in `metadata_schema` (`prometheus.lua`) are a manual copy of
`metric_label_map` (`exporter.lua`) minus the structural labels. They live in
different files and must be kept in sync by hand — easy to drift.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek merged PR #13202:
URL: https://github.com/apache/apisix/pull/13202
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels and metrics to reduce cardinality [apisix]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3114688329
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -771,12 +838,15 @@ local function inc_llm_active_connections(ctx, value)
matched_host = ctx.curr_req_matched._host or ""
end
+ local disabled_label_metric_map = get_disabled_label_metric_map()
+
Review Comment:
Same hot-path allocation issue as in `http_log`: `inc_llm_active_connections` recomputes `disabled_label_metric_map` on each increment/decrement. If you implement caching for `get_disabled_label_metric_map`, it should be reused here as well to avoid extra per-request allocations.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,61 @@ local function extra_labels(name, ctx)
end
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+}
+
+
+local function get_disabled_label_metric_map()
+ local attr = plugin.plugin_attr("prometheus")
+ local metrics_conf = attr and attr.metrics or {}
+ local disabled_label_metric_map = {}
+ for metric_name, metric_conf in pairs(metrics_conf) do
+ if metric_conf.disable_labels then
+ disabled_label_metric_map[metric_name] = {}
+ for _, label in ipairs(metric_conf.disable_labels) do
+ disabled_label_metric_map[metric_name][label] = true
+ end
+ end
+ end
Review Comment:
`disable_labels` is iterated with `ipairs(metric_conf.disable_labels)` without any type checks. Because `plugin_attr` does not appear to be schema-validated, a misconfiguration (e.g. `disable_labels: node` as a string) will raise a runtime error on every request. Consider validating `metrics_conf`/`metric_conf`/`disable_labels` types before iterating (and logging + ignoring invalid config) to keep the datapath robust.
```suggestion
if type(metrics_conf) ~= "table" then
core.log.warn("invalid prometheus plugin_attr.metrics type: ",
type(metrics_conf), ", expected table; ignoring")
return disabled_label_metric_map
end
for metric_name, metric_conf in pairs(metrics_conf) do
if type(metric_conf) ~= "table" then
core.log.warn("invalid prometheus metric config for metric '", metric_name,
"': expected table, got ", type(metric_conf), "; ignoring")
elseif metric_conf.disable_labels ~= nil then
if type(metric_conf.disable_labels) ~= "table" then
core.log.warn("invalid prometheus disable_labels for metric '", metric_name,
"': expected table, got ", type(metric_conf.disable_labels), "; ignoring")
else
disabled_label_metric_map[metric_name] = {}
for _, label in ipairs(metric_conf.disable_labels) do
disabled_label_metric_map[metric_name][label] = true
end
end
end
end
```
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,61 @@ local function extra_labels(name, ctx)
end
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
Review Comment:
`metric_label_map` duplicates the label-name lists already defined in `http_init` for each metric. This introduces a drift risk: if labels are added/reordered in the metric registration, `disable_labels` will silently stop applying to the intended label or apply to the wrong one. Consider defining the label-name arrays once (shared by both registration and filtering), or deriving the name list from the metric definition so ordering can’t diverge.
```suggestion
local HTTP_STATUS_LABELS = {"code", "route", "matched_uri", "matched_host", "service", "consumer",
"node", "request_type", "request_llm_model", "llm_model", "response_source"}
local HTTP_LATENCY_LABELS = {"type", "route", "service", "consumer", "node",
"request_type", "request_llm_model", "llm_model"}
local BANDWIDTH_LABELS = {"type", "route", "service", "consumer", "node",
"request_type", "request_llm_model", "llm_model"}
local LLM_LATENCY_LABELS = {"route_id", "service_id", "consumer", "node",
"request_type", "request_llm_model", "llm_model"}
local LLM_PROMPT_TOKENS_LABELS = {"route_id", "service_id", "consumer", "node",
"request_type", "request_llm_model", "llm_model"}
local LLM_COMPLETION_TOKENS_LABELS = {"route_id", "service_id", "consumer", "node",
"request_type", "request_llm_model", "llm_model"}
local LLM_ACTIVE_CONNECTIONS_LABELS = {"route", "route_id", "matched_uri", "matched_host",
"service", "service_id", "consumer", "node",
"request_type", "request_llm_model", "llm_model"}
local metric_label_map = {
http_status = HTTP_STATUS_LABELS,
http_latency = HTTP_LATENCY_LABELS,
bandwidth = BANDWIDTH_LABELS,
llm_latency = LLM_LATENCY_LABELS,
llm_prompt_tokens = LLM_PROMPT_TOKENS_LABELS,
llm_completion_tokens = LLM_COMPLETION_TOKENS_LABELS,
llm_active_connections = LLM_ACTIVE_CONNECTIONS_LABELS,
```
##########
conf/config.yaml.example:
##########
@@ -635,17 +635,24 @@ plugin_attr: # Plugin attributes
refresh_interval: 15 # Set the interval for refreshing cached metric data. unit: second.
# metrics: # Create extra labels from nginx variables: https://nginx.org/en/docs/varindex.html
# http_status:
+ # disable_labels: # List of built-in label names to drop (reduces cardinality).
+ # - node # e.g. drop the upstream node IP label
Review Comment:
The new `disable_labels` comment says "drop" labels, but the implementation/docs describe collapsing the label values to an empty string while keeping the label present (non-breaking). Wording here is misleading and could cause users to assume the label will be removed from the metric schema.
```suggestion
# disable_labels: # List of built-in label names whose values are set to empty strings (reduces cardinality while keeping labels).
# - node # e.g. keep the upstream node IP label but set its value to an empty string
```
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -292,6 +347,7 @@ end
function _M.http_log(conf, ctx)
local vars = ctx.var
+ local disabled_label_metric_map = get_disabled_label_metric_map()
Review Comment:
`get_disabled_label_metric_map()` builds a fresh nested table on every request (`http_log` runs in the hot path). This adds avoidable allocations/GC pressure at high QPS (the exact scenario this feature targets). Consider caching the computed disabled-label map per worker and only recomputing when `plugin_attr.prometheus.metrics` changes (e.g. by tracking a config version/modifiedIndex or comparing the attr table reference).
```suggestion
local cached_prometheus_metrics_attr
local cached_disabled_label_metric_map = {}
local function get_cached_disabled_label_metric_map()
local attr = plugin.plugin_attr("prometheus")
local metrics_attr = attr and attr.metrics
if metrics_attr ~= cached_prometheus_metrics_attr then
cached_prometheus_metrics_attr = metrics_attr
cached_disabled_label_metric_map = get_disabled_label_metric_map() or {}
end
return cached_disabled_label_metric_map
end
function _M.http_log(conf, ctx)
local vars = ctx.var
local disabled_label_metric_map = get_cached_disabled_label_metric_map()
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3382884090
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
Review Comment:
The disabled-label allowlist is duplicated across `metadata_schema` (here) and `metric_label_map` (exporter.lua), and the comment notes they must stay in sync. To avoid drift, consider centralizing these label lists in a shared module/constant, or generating the schema enums from a single source-of-truth table and/or adding a test that asserts the schema enums match the exporter’s built-in label lists (minus structural labels).
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,103 @@ local function extra_labels(name, ctx)
end
+local lrucache = core.lrucache.new({
+ type = "plugin",
+})
+
+
+-- Single source of truth for the ordered built-in label list of every metric
+-- that supports label disabling. It is used both to register the metric (see
+-- `append_tables` in `http_init`) and to map positional label values back to
+-- their names when collapsing disabled labels (see
+-- `get_enabled_label_values_for_metric`). Keeping a single declaration avoids
+-- the two lists drifting out of sync.
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+}
+
+
+-- Concatenate the given label-name tables into a new ordered table. Used at
+-- metric registration time to combine the built-in labels from
+-- `metric_label_map` with the operator-defined `extra_labels`.
+local function append_tables(...)
+ local res = {}
+ for _, tab in ipairs({...}) do
+ for _, v in ipairs(tab) do
+ core.table.insert(res, v)
+ end
+ end
+ return res
+end
+
+
+-- Shared empty table returned when no `disabled_labels` metadata is configured,
+-- to avoid allocating in the log hot path. It is only ever read, never mutated.
+local empty_disabled_map = {}
+
+
+local function build_disabled_label_metric_map(disabled_labels)
+ local disabled_label_metric_map = {}
+ for metric_name, labels in pairs(disabled_labels) do
+ disabled_label_metric_map[metric_name] = {}
+ for _, label in ipairs(labels) do
+ disabled_label_metric_map[metric_name][label] = true
+ end
+ end
+ return disabled_label_metric_map
+end
+
+
+-- Read the per-metric `disabled_labels` from the plugin metadata and return a
+-- lookup of metric_name -> {label_name = true}. The built map is cached via
+-- lrucache keyed by the metadata's modifiedIndex, so it is only rebuilt when the
+-- metadata changes rather than on every request in the log hot path.
+local function get_disabled_label_metric_map()
+ local metadata = plugin.plugin_metadata(plugin_name)
+ if not (metadata and metadata.value and metadata.value.disabled_labels
+ and metadata.modifiedIndex) then
+ return empty_disabled_map
+ end
+
+ return lrucache(plugin_name, metadata.modifiedIndex,
+ build_disabled_label_metric_map, metadata.value.disabled_labels)
+end
+
+
+local function get_enabled_label_values_for_metric(metric_name, disabled_label_metric_map, ...)
+ local label_values = gen_arr(...)
+ local metric_labels = metric_label_map[metric_name]
+
+ if not metric_labels then
+ return label_values
+ end
+
+ local disabled_labels = disabled_label_metric_map[metric_name] or {}
+ for i in ipairs(label_values) do
+ local label_name = metric_labels[i]
+ if label_name and disabled_labels[label_name] then
+ label_values[i] = ""
+ end
+ end
Review Comment:
Iterating with `ipairs(label_values)` can stop early if any positional label value is `nil`, which would skip collapsing for subsequent labels and can leave holes in the positional label array. A more robust approach is to iterate over the known ordered `metric_labels` list (its indices are the authoritative positions) and apply collapsing by position, optionally normalizing missing values to `\"\"` for built-in label positions to preserve correct label/value alignment.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -213,51 +306,42 @@ function _M.http_init(prometheus_enabled_in_stream)
end
metrics.latency = prometheus:histogram("http_latency",
- "HTTP request latency in milliseconds per service in APISIX",
- {"type", "route", "service", "consumer", "node",
- "request_type", "request_llm_model", "llm_model",
- unpack(extra_labels("http_latency"))},
- buckets, latency_metrics_exptime)
+ "HTTP request latency in milliseconds per service in APISIX",
+ append_tables(metric_label_map.http_latency, extra_labels("http_latency")),
+ buckets, latency_metrics_exptime)
metrics.bandwidth = prometheus:counter("bandwidth",
"Total bandwidth in bytes consumed per service in APISIX",
- {"type", "route", "service", "consumer", "node",
- "request_type", "request_llm_model", "llm_model",
- unpack(extra_labels("bandwidth"))},
+ append_tables(metric_label_map.bandwidth, extra_labels("bandwidth")),
bandwidth_metrics_exptime)
Review Comment:
The new `disabled_labels` behavior applies to multiple metrics (e.g., `bandwidth` and the LLM metrics), but the added test only verifies `http_status` collapsing and `http_latency` non-collapsing. Consider adding at least one assertion for another metric (e.g., `bandwidth`) to cover that the per-metric scoping works beyond the two HTTP metrics, and/or a minimal LLM metric case if those labels are expected to be present in this test environment.
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
+ type = "object",
+ properties = {
+ disabled_labels = {
+ type = "object",
+ properties = {
+ http_status = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "matched_uri", "matched_host", "service",
+ "consumer", "node", "request_type",
+ "request_llm_model", "llm_model", "response_source"},
+ },
+ },
+ http_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ bandwidth = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_prompt_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_completion_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_active_connections = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ },
+ additionalProperties = false,
+ },
+ },
+}
Review Comment:
At the metadata root level, `additionalProperties` is not set, so unknown top-level keys in plugin metadata will be accepted silently. If the intent is to keep metadata strict (similar to `disabled_labels.additionalProperties = false`), consider adding `additionalProperties = false` at the root of `metadata_schema` as well to avoid typos/misconfigurations being ignored.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,103 @@ local function extra_labels(name, ctx)
end
+local lrucache = core.lrucache.new({
+ type = "plugin",
+})
+
+
+-- Single source of truth for the ordered built-in label list of every metric
+-- that supports label disabling. It is used both to register the metric (see
+-- `append_tables` in `http_init`) and to map positional label values back to
+-- their names when collapsing disabled labels (see
+-- `get_enabled_label_values_for_metric`). Keeping a single declaration avoids
+-- the two lists drifting out of sync.
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+}
+
+
+-- Concatenate the given label-name tables into a new ordered table. Used at
+-- metric registration time to combine the built-in labels from
+-- `metric_label_map` with the operator-defined `extra_labels`.
+local function append_tables(...)
+ local res = {}
+ for _, tab in ipairs({...}) do
+ for _, v in ipairs(tab) do
+ core.table.insert(res, v)
+ end
+ end
+ return res
+end
+
+
+-- Shared empty table returned when no `disabled_labels` metadata is configured,
+-- to avoid allocating in the log hot path. It is only ever read, never mutated.
+local empty_disabled_map = {}
+
+
+local function build_disabled_label_metric_map(disabled_labels)
+ local disabled_label_metric_map = {}
+ for metric_name, labels in pairs(disabled_labels) do
+ disabled_label_metric_map[metric_name] = {}
+ for _, label in ipairs(labels) do
+ disabled_label_metric_map[metric_name][label] = true
+ end
+ end
+ return disabled_label_metric_map
+end
+
+
+-- Read the per-metric `disabled_labels` from the plugin metadata and return a
+-- lookup of metric_name -> {label_name = true}. The built map is cached via
+-- lrucache keyed by the metadata's modifiedIndex, so it is only rebuilt when the
+-- metadata changes rather than on every request in the log hot path.
+local function get_disabled_label_metric_map()
+ local metadata = plugin.plugin_metadata(plugin_name)
+ if not (metadata and metadata.value and metadata.value.disabled_labels
+ and metadata.modifiedIndex) then
+ return empty_disabled_map
+ end
+
+ return lrucache(plugin_name, metadata.modifiedIndex,
+ build_disabled_label_metric_map, metadata.value.disabled_labels)
+end
+
+
+local function get_enabled_label_values_for_metric(metric_name, disabled_label_metric_map, ...)
+ local label_values = gen_arr(...)
+ local metric_labels = metric_label_map[metric_name]
+
+ if not metric_labels then
+ return label_values
+ end
+
+ local disabled_labels = disabled_label_metric_map[metric_name] or {}
+ for i in ipairs(label_values) do
+ local label_name = metric_labels[i]
+ if label_name and disabled_labels[label_name] then
+ label_values[i] = ""
+ end
+ end
+
+ return label_values
+end
Review Comment:
`get_enabled_label_values_for_metric` always iterates over `label_values` even when no labels are disabled, which adds overhead in the log hot path. Consider an early return when `disabled_label_metric_map` is the shared empty map or when `disabled_label_metric_map[metric_name]` is nil/empty (e.g., `next(disabled_labels) == nil`) so the common case stays as close to the old `gen_arr(...)` path as possible.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "satngp (via GitHub)" <gi...@apache.org>.
satngp commented on PR #13202:
URL: https://github.com/apache/apisix/pull/13202#issuecomment-5169431677
Hello, We are evaluating this enhancement for our APISIX deployment and had a couple of questions:
1) Could you please confirm whether this feature is available in APISIX 3.17.0, or if it is planned to be included starting with APISIX 3.18.0 (or a later release)?
2) Has this change also been integrated into the official APISIX Helm Chart? If so, is there any additional Helm configuration required to enable and configure disabled_labels, or is it managed entirely through Prometheus plugin metadata?
We are currently facing high Prometheus metric cardinality in production and are evaluating this feature as a potential solution. Any guidance would be appreciated. we have a very short time.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "janiussyafiq (via GitHub)" <gi...@apache.org>.
janiussyafiq commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3382953435
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,103 @@ local function extra_labels(name, ctx)
end
+local lrucache = core.lrucache.new({
+ type = "plugin",
+})
+
+
+-- Single source of truth for the ordered built-in label list of every metric
+-- that supports label disabling. It is used both to register the metric (see
+-- `append_tables` in `http_init`) and to map positional label values back to
+-- their names when collapsing disabled labels (see
+-- `get_enabled_label_values_for_metric`). Keeping a single declaration avoids
+-- the two lists drifting out of sync.
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+}
+
+
+-- Concatenate the given label-name tables into a new ordered table. Used at
+-- metric registration time to combine the built-in labels from
+-- `metric_label_map` with the operator-defined `extra_labels`.
+local function append_tables(...)
+ local res = {}
+ for _, tab in ipairs({...}) do
+ for _, v in ipairs(tab) do
+ core.table.insert(res, v)
+ end
+ end
+ return res
+end
+
+
+-- Shared empty table returned when no `disabled_labels` metadata is configured,
+-- to avoid allocating in the log hot path. It is only ever read, never mutated.
+local empty_disabled_map = {}
+
+
+local function build_disabled_label_metric_map(disabled_labels)
+ local disabled_label_metric_map = {}
+ for metric_name, labels in pairs(disabled_labels) do
+ disabled_label_metric_map[metric_name] = {}
+ for _, label in ipairs(labels) do
+ disabled_label_metric_map[metric_name][label] = true
+ end
+ end
+ return disabled_label_metric_map
+end
+
+
+-- Read the per-metric `disabled_labels` from the plugin metadata and return a
+-- lookup of metric_name -> {label_name = true}. The built map is cached via
+-- lrucache keyed by the metadata's modifiedIndex, so it is only rebuilt when the
+-- metadata changes rather than on every request in the log hot path.
+local function get_disabled_label_metric_map()
+ local metadata = plugin.plugin_metadata(plugin_name)
+ if not (metadata and metadata.value and metadata.value.disabled_labels
+ and metadata.modifiedIndex) then
+ return empty_disabled_map
+ end
+
+ return lrucache(plugin_name, metadata.modifiedIndex,
+ build_disabled_label_metric_map, metadata.value.disabled_labels)
+end
+
+
+local function get_enabled_label_values_for_metric(metric_name, disabled_label_metric_map, ...)
+ local label_values = gen_arr(...)
+ local metric_labels = metric_label_map[metric_name]
+
+ if not metric_labels then
+ return label_values
+ end
+
+ local disabled_labels = disabled_label_metric_map[metric_name] or {}
+ for i in ipairs(label_values) do
+ local label_name = metric_labels[i]
+ if label_name and disabled_labels[label_name] then
+ label_values[i] = ""
+ end
+ end
+
+ return label_values
+end
Review Comment:
Fixed in 2a59ae4d. `get_enabled_label_values_for_metric` now returns early when `disabled_label_metric_map[metric_name]` is nil — i.e. whenever nothing is disabled for the metric, which is always the case when no metadata is configured. This keeps the no-config path as close as possible to the original `gen_arr(...)` and also removes a per-call empty-table allocation (the previous `... or {}`).
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -107,6 +106,103 @@ local function extra_labels(name, ctx)
end
+local lrucache = core.lrucache.new({
+ type = "plugin",
+})
+
+
+-- Single source of truth for the ordered built-in label list of every metric
+-- that supports label disabling. It is used both to register the metric (see
+-- `append_tables` in `http_init`) and to map positional label values back to
+-- their names when collapsing disabled labels (see
+-- `get_enabled_label_values_for_metric`). Keeping a single declaration avoids
+-- the two lists drifting out of sync.
+local metric_label_map = {
+ http_status = {"code", "route", "matched_uri", "matched_host", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model", "response_source"},
+ http_latency = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ bandwidth = {"type", "route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_latency = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_prompt_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_completion_tokens = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ llm_active_connections = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+}
+
+
+-- Concatenate the given label-name tables into a new ordered table. Used at
+-- metric registration time to combine the built-in labels from
+-- `metric_label_map` with the operator-defined `extra_labels`.
+local function append_tables(...)
+ local res = {}
+ for _, tab in ipairs({...}) do
+ for _, v in ipairs(tab) do
+ core.table.insert(res, v)
+ end
+ end
+ return res
+end
+
+
+-- Shared empty table returned when no `disabled_labels` metadata is configured,
+-- to avoid allocating in the log hot path. It is only ever read, never mutated.
+local empty_disabled_map = {}
+
+
+local function build_disabled_label_metric_map(disabled_labels)
+ local disabled_label_metric_map = {}
+ for metric_name, labels in pairs(disabled_labels) do
+ disabled_label_metric_map[metric_name] = {}
+ for _, label in ipairs(labels) do
+ disabled_label_metric_map[metric_name][label] = true
+ end
+ end
+ return disabled_label_metric_map
+end
+
+
+-- Read the per-metric `disabled_labels` from the plugin metadata and return a
+-- lookup of metric_name -> {label_name = true}. The built map is cached via
+-- lrucache keyed by the metadata's modifiedIndex, so it is only rebuilt when the
+-- metadata changes rather than on every request in the log hot path.
+local function get_disabled_label_metric_map()
+ local metadata = plugin.plugin_metadata(plugin_name)
+ if not (metadata and metadata.value and metadata.value.disabled_labels
+ and metadata.modifiedIndex) then
+ return empty_disabled_map
+ end
+
+ return lrucache(plugin_name, metadata.modifiedIndex,
+ build_disabled_label_metric_map, metadata.value.disabled_labels)
+end
+
+
+local function get_enabled_label_values_for_metric(metric_name, disabled_label_metric_map, ...)
+ local label_values = gen_arr(...)
+ local metric_labels = metric_label_map[metric_name]
+
+ if not metric_labels then
+ return label_values
+ end
+
+ local disabled_labels = disabled_label_metric_map[metric_name] or {}
+ for i in ipairs(label_values) do
+ local label_name = metric_labels[i]
+ if label_name and disabled_labels[label_name] then
+ label_values[i] = ""
+ end
+ end
Review Comment:
Fixed in 2a59ae4d. The collapse loop now iterates the authoritative ordered list via `for i = 1, #metric_labels` instead of `ipairs(label_values)`, so a `nil` positional value can no longer terminate the scan early. `#metric_labels` covers only the built-in label positions, so operator-defined `extra_labels` appended after them are intentionally left untouched.
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
Review Comment:
Intentional, leaving as-is. The `enum` here is deliberately a *subset* of `metric_label_map`: it omits the structural labels (`code`, `type`) that must not be disable-able. So the two are not the same list and cannot be shared verbatim — deriving the enums from the exporter table would require encoding the structural-label exclusion separately and building the schema dynamically, which is less readable than the explicit literal and diverges from how other plugins declare their metadata schemas.
The drift consequence is also mild: a label added to the registration list but not to the enum simply isn't disable-able yet (rejected at config time), not a runtime error. I've kept the explicit schema with the in-code sync note; this mirrors how the same feature is implemented in api7's gateway. Happy to add a consistency assertion if maintainers prefer it.
##########
apisix/plugins/prometheus.lua:
##########
@@ -29,18 +29,99 @@ local schema = {
}
+-- Per-metric list of built-in labels whose values can be collapsed to an empty
+-- string to reduce cardinality. The `enum` of each metric intentionally omits
+-- structural labels that define the metric's identity (`code` on `http_status`,
+-- `type` on `http_latency`/`bandwidth`), so they cannot be disabled. The enum
+-- must stay in sync with `metric_label_map` in exporter.lua.
+local metadata_schema = {
+ type = "object",
+ properties = {
+ disabled_labels = {
+ type = "object",
+ properties = {
+ http_status = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "matched_uri", "matched_host", "service",
+ "consumer", "node", "request_type",
+ "request_llm_model", "llm_model", "response_source"},
+ },
+ },
+ http_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ bandwidth = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "service", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_latency = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_prompt_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_completion_tokens = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route_id", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ llm_active_connections = {
+ type = "array",
+ items = {
+ type = "string",
+ enum = {"route", "route_id", "matched_uri", "matched_host",
+ "service", "service_id", "consumer", "node",
+ "request_type", "request_llm_model", "llm_model"},
+ },
+ },
+ },
+ additionalProperties = false,
+ },
+ },
+}
Review Comment:
Left open intentionally for consistency with the codebase. None of the existing metadata schemas (`http-logger`, `syslog`, `kafka-logger`, `error-log-logger`, …) set `additionalProperties = false` at the root, so adding it only here would diverge from the established pattern. The inner `disabled_labels.additionalProperties = false` already rejects the realistic misconfiguration — an unknown metric key. Happy to tighten the root if maintainers would rather standardize on strict metadata.
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -213,51 +306,42 @@ function _M.http_init(prometheus_enabled_in_stream)
end
metrics.latency = prometheus:histogram("http_latency",
- "HTTP request latency in milliseconds per service in APISIX",
- {"type", "route", "service", "consumer", "node",
- "request_type", "request_llm_model", "llm_model",
- unpack(extra_labels("http_latency"))},
- buckets, latency_metrics_exptime)
+ "HTTP request latency in milliseconds per service in APISIX",
+ append_tables(metric_label_map.http_latency, extra_labels("http_latency")),
+ buckets, latency_metrics_exptime)
metrics.bandwidth = prometheus:counter("bandwidth",
"Total bandwidth in bytes consumed per service in APISIX",
- {"type", "route", "service", "consumer", "node",
- "request_type", "request_llm_model", "llm_model",
- unpack(extra_labels("bandwidth"))},
+ append_tables(metric_label_map.bandwidth, extra_labels("bandwidth")),
bandwidth_metrics_exptime)
Review Comment:
Added in 2a59ae4d. The test now also disables `node` on `bandwidth` and asserts `apisix_bandwidth` exposes `node=""` while keeping `route="1"` — covering per-metric scoping on a third metric, and a second metric *type* (counter), beyond `http_status`/`http_latency`. The LLM metrics are intentionally left out: they go through the identical `get_enabled_label_values_for_metric` path, so exercising them would require standing up an ai-proxy fixture for no additional code coverage.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #13202:
URL: https://github.com/apache/apisix/pull/13202#discussion_r3393049559
##########
apisix/plugins/prometheus/exporter.lua:
##########
Review Comment:
It seems that there are still some metrocis that have not been modified.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(prometheus): support disabling labels via plugin metadata to reduce cardinality [apisix]
Posted by "satngp (via GitHub)" <gi...@apache.org>.
satngp commented on PR #13202:
URL: https://github.com/apache/apisix/pull/13202#issuecomment-5177444280
@janiussyafiq - Thanks for your response.
@shreemaan-abhishek Is there any tentative release time for next version
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org