Copilot commented on code in PR #13878:
URL: https://github.com/apache/apisix/pull/13878#discussion_r3869354149
##########
apisix/plugins/prometheus.lua:
##########
@@ -109,8 +109,16 @@ end
function _M.init()
local local_conf = core.config.local_conf()
- local enabled_in_stream = core.table.array_find(local_conf.stream_plugins,
"prometheus")
- exporter.http_init(enabled_in_stream)
+ -- Not `stream_plugins` from config.yaml: that list is only the boot-time
+ -- default, and /apisix/plugins in etcd can turn the stream prometheus
+ -- plugin on later. This runs whenever the plugin is loaded, so building
+ -- `metrics` without the L4 gauges here would leave
+ -- collect_stream_zone_metrics() stopped at its first guard for the rest of
+ -- the process's life. Whether the stream subsystem runs at all is the
+ -- thing that does not change under APISIX.
+ local proxy_mode = local_conf.apisix.proxy_mode
+ local stream_enabled = proxy_mode == "stream" or proxy_mode ==
"http&stream"
+ exporter.http_init(stream_enabled)
Review Comment:
The etcd-only case is still not initialized reliably. A first load after
startup is ignored because both `http_init()` and `stream_init()` return
outside `init`/`init_worker` (`exporter.lua:451-459,652-655`); even a
pre-seeded list can miss the cache timer because `plugin.init_prometheus()`
still checks the local lists (`plugin.lua:914-924`). The test masks this by
omitting `plugins`, which restores the default HTTP list containing
`prometheus`, and by writing etcd before `make run`. Please initialize the
exporter and timer from the effective plugin list (including hot loads), and
test with Prometheus excluded from both local lists and enabled after startup.
##########
t/cli/test_stream_config.sh:
##########
@@ -30,34 +30,48 @@ apisix:
make init
+# Two, not one: the stream subsystem has no server of its own to export metrics
+# from, so an http{} block is rendered to host the prometheus export server.
count=$(grep -c "lua_package_path" conf/nginx.conf)
-if [ "$count" -ne 1 ]; then
+if [ "$count" -ne 2 ]; then
Review Comment:
This expectation fails on the supported non-APISIX-Runtime path. The extra
exporter `http{}` is nested under `{% if use_apisix_base %}` in
`ngx_tpl.lua:70-135`, so stock OpenResty still renders only one
`lua_package_path`; unlike the Prometheus integration test, this script does
not call `exit_if_not_customed_nginx`. Make the expected count conditional on
`apisix-nginx-module` (or skip only this assertion) so the CLI test remains
valid in both runtime modes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
