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:
Three separate things here.
**The `init_prometheus` gate.** Real, but pre-existing and a no-op to fix on
its own. With prometheus in neither local list, `http_init()` returns at the
phase guard (`exporter.lua:454`) and `prometheus_bkp` is nil, so `prometheus`
stays nil and `exporter_timer` returns at its own `if not prometheus`
(`exporter.lua:1169`) on every tick. Starting the timer unconditionally would
just spin. Making that path work means dropping the phase guard — 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 is its own change.
**The test isn't masking anything; the default list *is* the scenario.** A
data plane whose plugin list is owned by a control plane still ships the stock
local `config.yaml`: prometheus is in `plugins` and absent from
`stream_plugins`. That asymmetry is exactly why the L4 half broke and the HTTP
half did not. The list reaching etcd before the data plane starts is the normal
order too, not a shortcut.
**What this PR fixes is `_M.init()`.** `plugin.load()` runs in init_worker
off the local list, so the exporter is built there — previously without the L4
gauges, and `collect_stream_zone_metrics()` then stopped at its first guard for
the life of the process. The later etcd-driven reload keeps them: `destroy()`
only nils `prometheus` and backs it up (`exporter.lua:1370`), so `metrics`
survives. The added case fails on master and passes here.
Enable-after-startup needs the lua-prometheus work; I'd rather do that
separately.
--
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]
