You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "qihaiyan (via GitHub)" <gi...@apache.org> on 2025/10/28 04:48:15 UTC
[PR] feat: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
qihaiyan opened a new pull request, #12706:
URL: https://github.com/apache/apisix/pull/12706
### Description
{% if enabled_plugins["prometheus"] and not enabled_stream_plugins["prometheus"] then %}
lua_shared_dict prometheus-metrics {* http.lua_shared_dict["prometheus-metrics"] *};
-- add prometheus-cache here
lua_shared_dict prometheus-cache {* http.lua_shared_dict["prometheus-cache"] *};
{% end #%}
Maybe it's better to set prometheus-cache together with prometheus-metrics in ngx_tpl.lua
Only when use_apisix_base is set to true, will the prometheus-cache take effect.
But in order to integrate with QAT, i used a custom built openresty instead of apisix-runtime, so use_apisix_base is not true, and prometheus-cache is not setted in nginx.conf even if prometheus plugin is enabled.
#### Which issue(s) this PR fixes:
Fixes #12703
### Checklist
- [v] I have explained the need for this PR and the problem it solves
- [v] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] 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)
--
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: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
Posted by "qihaiyan (via GitHub)" <gi...@apache.org>.
qihaiyan commented on code in PR #12706:
URL: https://github.com/apache/apisix/pull/12706#discussion_r2548659218
##########
apisix/cli/ngx_tpl.lua:
##########
@@ -334,6 +334,9 @@ http {
{% if enabled_plugins["prometheus"] and not enabled_stream_plugins["prometheus"] then %}
lua_shared_dict prometheus-metrics {* http.lua_shared_dict["prometheus-metrics"] *};
+ {% if not use_apisix_base then %}
+ lua_shared_dict prometheus-cache {* meta.lua_shared_dict["prometheus-cache"] *};
Review Comment:
yes, it's expected.
--
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: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on code in PR #12706:
URL: https://github.com/apache/apisix/pull/12706#discussion_r2562747303
##########
t/cli/test_prometheus_stream.sh:
##########
@@ -60,7 +60,7 @@ sleep 1 # wait for sync
out="$(curl http://127.0.0.1:9091/apisix/prometheus/metrics)"
if ! echo "$out" | grep "apisix_stream_connection_total{route=\"1\"} 1" > /dev/null; then
- echo "failed: prometheus can't work in stream subsystem"
+ echo "failed: prometheus can't work in both http & stream subsystem"
Review Comment:
I checked the existing tests and didn't find anything related to `use_apisix_base=false`, so I think this is acceptable. Let's see what other maintainers think.
--
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: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop closed pull request #12706: feat: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703)
URL: https://github.com/apache/apisix/pull/12706
--
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: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
Posted by "qihaiyan (via GitHub)" <gi...@apache.org>.
qihaiyan commented on code in PR #12706:
URL: https://github.com/apache/apisix/pull/12706#discussion_r2548675438
##########
t/cli/test_prometheus_stream.sh:
##########
@@ -60,7 +60,7 @@ sleep 1 # wait for sync
out="$(curl http://127.0.0.1:9091/apisix/prometheus/metrics)"
if ! echo "$out" | grep "apisix_stream_connection_total{route=\"1\"} 1" > /dev/null; then
- echo "failed: prometheus can't work in stream subsystem"
+ echo "failed: prometheus can't work in both http & stream subsystem"
Review Comment:
I didn't find out how to set use_apisix_base=false in ut environment, it's setted in env.lua:
```
local use_apisix_base = true
if not or_info:find("apisix-nginx-module", 1, true) then
use_apisix_base = false
end
```
--
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: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on code in PR #12706:
URL: https://github.com/apache/apisix/pull/12706#discussion_r2548606284
##########
t/cli/test_prometheus_stream.sh:
##########
@@ -60,7 +60,7 @@ sleep 1 # wait for sync
out="$(curl http://127.0.0.1:9091/apisix/prometheus/metrics)"
if ! echo "$out" | grep "apisix_stream_connection_total{route=\"1\"} 1" > /dev/null; then
- echo "failed: prometheus can't work in stream subsystem"
+ echo "failed: prometheus can't work in both http & stream subsystem"
Review Comment:
Additional testing is needed; the existing tests cannot verify whether prometheus-cache is correctly generated when use_apisix_base=false.
##########
apisix/cli/ngx_tpl.lua:
##########
@@ -334,6 +334,9 @@ http {
{% if enabled_plugins["prometheus"] and not enabled_stream_plugins["prometheus"] then %}
lua_shared_dict prometheus-metrics {* http.lua_shared_dict["prometheus-metrics"] *};
+ {% if not use_apisix_base then %}
+ lua_shared_dict prometheus-cache {* meta.lua_shared_dict["prometheus-cache"] *};
Review Comment:
If the user has also enabled the Prometheus plugin in the stream subsystem (i.e., `enabled_stream_plugins["prometheus"]` is true), this if block will be skipped. If `use_apisix_base` is also false at this time (skipping the top-level global definition), then neither `prometheus-metrics` nor `prometheus-cache` will be defined. Is this the expected behavior?
--
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: add additional handling of prometheus-cache in ngx_tpl.lua (#1212703) [apisix]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #12706:
URL: https://github.com/apache/apisix/pull/12706#issuecomment-3680652743
The issue it referenced is closed, so I'll close this PR for now. Please provide feedback and I'll reopen it if you have any questions. Thank you for your contribution @qihaiyan.
--
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