AlinsRan commented on code in PR #13878:
URL: https://github.com/apache/apisix/pull/13878#discussion_r3877396884
##########
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:
Correcting my earlier reply: this is right, and it is fixed in f46c3267.
I said the phase guard made it impractical. It does not — the API7 gateway,
whose data plane always learns its plugin list from a control plane over etcd,
has been solving it without touching the guard, and I have adopted its wiring
here:
- `plugin.init_worker()` builds the exporter whatever `plugins` in
config.yaml lists (`http_init` in the HTTP subsystem, `stream_init` in the
stream one). The guard can then only ever be asked to *restore* what
init_worker built, which is what it can do.
- `plugin.load()` drops it again with `exporter.destroy()` when prometheus
is not in the effective list, and calls `http_init` when it is in the list but
not active. So a deployment that never enables prometheus is left with a
destroyed exporter and `exporter_timer` returns at its own `if not prometheus`
— no cost for keeping the door open.
- `init_prometheus()` no longer consults the local lists, for the same
reason.
- The plugin `init` hooks that used to drive this are gone. That also
removes a side effect: `load_stream()` runs in the HTTP subsystem too, so the
stream plugin's `destroy = exporter.destroy` could nil the HTTP exporter, and
its `init = exporter.stream_init` could rebuild `metrics` with only the L4
gauges.
Verified locally against `prometheus` in neither local list, Admin API off:
`/apisix/prometheus/metrics` answers `{}` on the previous commit, exports 91
series after `etcdctl put /apisix/plugins '[{"name":"prometheus"}]'` with this
one, and goes back to `{}` when etcd disables it again.
`t/cli/test_prometheus.sh` now carries that case.
One deliberate difference from the gateway: it passes `http_init(true)`
unconditionally, since it only ever runs on APISIX-Runtime with the stream
subsystem. Here the argument follows `proxy_mode`, so a pure-HTTP deployment
does not register L4 gauges it can never fill.
--
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]
