AlinsRan commented on code in PR #13878:
URL: https://github.com/apache/apisix/pull/13878#discussion_r3870053902
##########
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:
Dropping that gate on its own wouldn't help. With prometheus in neither
local list, `http_init()` returns at the phase guard (`exporter.lua:454`) and
`prometheus` stays nil, so the timer would return at `if not prometheus`
(`:1169`) on every tick. Making the path work means removing the phase guard,
which is the `-- todo: support hot reload, we may need to update the
lua-prometheus library` sitting right above it: re-registering metrics against
a live shdict. Worth a separate issue, not this PR.
The config it needs is also narrower than "etcd-only": Admin API off, or the
`/plugins` watcher is never created (`plugin.lua:932`), plus prometheus removed
from the local `plugins` by hand. Startup timing isn't part of it. The first
`/plugins` sync runs in a timer (`config_etcd.lua:1112`), so it hits the same
guard even when etcd was seeded before APISIX started.
The default list isn't a mask, it's the case this PR is about. Stock
`config.yaml` has prometheus in `plugins` and not in `stream_plugins`, which is
why the L4 half broke and the HTTP half didn't. `plugin.load()` builds the
exporter in init_worker off that list, previously without the L4 gauges, and
`collect_stream_zone_metrics()` then stopped at its first guard for the rest of
the process. The etcd reload keeps them: `destroy()` only nils `prometheus` and
backs it up (`:1370`), so `metrics` survives. The added case fails on master.
--
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]