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:
The phase guards and the `init_prometheus` gate are real, but they are a
pre-existing limitation that this PR does not touch, and fixing the timer gate
alone would change nothing.
For an enable-after-startup 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; `exporter_timer` then returns at its own `if
not prometheus` (exporter.lua:1169) on every tick. Starting the timer
unconditionally would just spin a no-op. Making that path work means dropping
the phase guard, which is the `-- todo: support hot reload, we may need to
update the lua-prometheus library` right above it — re-registering metrics
against a live shdict is its own change.
On the test: leaving `plugins` at the default is the scenario, not a mask. A
data plane whose plugin list is owned by a control plane still ships the stock
local config, so prometheus is in the local `plugins` and absent from
`stream_plugins` — which is exactly why the L4 half broke and the HTTP half did
not. Writing etcd before `make run` is likewise how it happens: the plugin list
is in etcd before the data plane starts.
What that scenario needed, and what this fixes, is `_M.init()`.
`plugin.load()` runs in init_worker off the local list, so the exporter is
built there; before this it was built without the L4 gauges, and
`collect_stream_zone_metrics()` stopped at its first guard for the rest of the
process. The later etcd-driven reload keeps them: `destroy()` only nils
`prometheus` and backs it up (exporter.lua:1370), leaving `metrics` intact, and
the phase-guarded re-entry restores from `prometheus_bkp`. The added case fails
on master and passes here.
Enable-after-startup is worth a follow-up, but it needs the lua-prometheus
work and is not what this PR is about.
--
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]
