Repository navigation
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
6 open findings
core.config.local_conf().nginx_configcan benil(or missingevent) depending on… · Newrunningtimer count, butngx.timer.running_count()can be… · Newstate="running"timer count is non-zero ([1-9]\d*), but… · Newrunningtimer count can be0, so requiring[1-9]risks… · Newprocess_metrics.refresh()runs on every collection/scrape in the rendering process, which can… · Newstop()does not allow an immediate restart if a timer is already pending: callingstart()after… · New
What changed in this PR
Adds Prometheus process-scoped metrics for OpenResty internals (timers, connection slots, Lua VM memory) so per-process resource exhaustion (notably privileged-agent) becomes visible before failures.
Changes:
- Introduces a new
process_metricsmodule to collect/report per-process gauges via shared metric dict. - Hooks process-metric collection into the Prometheus exporter lifecycle (init/start/stop/refresh).
- Adds/extends tests and documentation (EN/ZH) to cover/explain the new gauges (including stream-only mode).
| File | Description |
|---|---|
| t/plugin/prometheus-process-metrics.t | Adds integration tests for the new per-process timers/connections/memory gauges (http + stream, worker + privileged-agent). |
| t/cli/test_prometheus_stream.sh | Extends stream-only CI test to assert new process metrics are present. |
| docs/zh/latest/plugins/prometheus.md | Documents the new metrics and label semantics in Chinese. |
| docs/en/latest/plugins/prometheus.md | Documents the new metrics and label semantics in English. |
| apisix/plugins/prometheus/process_metrics.lua | New collector module that periodically writes per-process gauges (timers, connection slots, Lua memory) into the metrics dict. |
| apisix/plugins/prometheus/exporter.lua | Integrates process_metrics init/start/stop and refreshes local process values during collection. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+198
to
+203
| local event = core.config.local_conf().nginx_config.event | ||
| local expected = event and tonumber(event.worker_connections) | ||
| if connection_n ~= expected then | ||
| return false, "worker connection slots read as " .. tostring(connection_n) | ||
| .. ", but worker_connections is " .. tostring(expected) | ||
| end |
| { set +x; } 2>/dev/null | ||
| while [ "$(date +%s)" -lt "$deadline" ]; do | ||
| out="$(curl -s --connect-timeout 1 --max-time 2 http://127.0.0.1:9091/apisix/prometheus/metrics || true)" | ||
| if echo "$out" | grep -qE 'apisix_nginx_timers\{process="worker-0",subsystem="stream",state="running"\} [1-9]' \ |
| local res = assert(httpc:request_uri( | ||
| "http://127.0.0.1:1984/apisix/prometheus/metrics")) | ||
| local patterns = { | ||
| [[apisix_nginx_timers\{process="worker-0",subsystem="http",state="running"\} [1-9]\d*]], |
| local res = assert(httpc:request_uri( | ||
| "http://127.0.0.1:1984/apisix/prometheus/metrics")) | ||
| local patterns = { | ||
| [[apisix_nginx_timers\{process="worker-0",subsystem="stream",state="running"\} [1-9]\d*]], |
Comment on lines
1205
to
+1208
| local function collect(yieldable) | ||
| -- the other processes report their own values on a timer | ||
| process_metrics.refresh() | ||
|
|
Comment on lines
+359
to
+382
| function _M.start(report_interval) | ||
| if not gauges then | ||
| return | ||
| end | ||
|
|
||
| interval = report_interval or DEFAULT_INTERVAL | ||
| stopped = false | ||
| if timer_running then | ||
| return | ||
| end | ||
|
|
||
| timer_running = true | ||
| local ok, err = timer_at(0, tick) | ||
| if not ok then | ||
| timer_running = false | ||
| core.log.error("failed to start process metrics: ", err) | ||
| end | ||
| end | ||
|
|
||
|
|
||
| -- The pending timer sees this when it fires and does not schedule another. | ||
| function _M.stop() | ||
| stopped = true | ||
| end |
Copilot stopped reviewing on behalf of
nic-6443 due to an error
October 14, 2026 10:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This adds Prometheus gauges for a few OpenResty internals that can run out per process, so exhaustion becomes visible before it turns into errors:
apisix_nginx_timers{process, subsystem, state}:ngx.timer.running_count()/pending_count()of each Lua VMapisix_nginx_timers_max{state}: the configuredmax_running_timers/max_pending_timersapisix_nginx_process_connections{process}: connection slots in use in each process (connection_n - free_connection_n)apisix_nginx_process_connections_max{process}: connection slots of each process (worker_connectionsfor workers, 512 for the privileged agent)apisix_nginx_lua_memory_bytes{process, subsystem}:collectgarbage("count")of each Lua VMprocessisworker-<id>orprivileged-agent. The motivating case is a privileged agent that slowly leaks cosockets until its 512 connection slots are gone: the workers look perfectly healthy, andnginx_http_current_connections(stub_status) only counts accepted client connections, so nothing in the current exposition moves until the agent starts failing.None of these values can be read from another process, so each process writes its own gauges into the metric dict on a timer (every
refresh_interval), and the privileged agent refreshes its own values right before it renders the cache. The collection lives in its own module,apisix/plugins/prometheus/process_metrics.lua; the exporter only registers it and starts/stops it together with the plugin, so nothing new runs whenprometheusis disabled. The http and the stream VM report separately (they have their own timers and memory), and the per-process values (connections, timer limits) come from the http VM only.The connection numbers are read through FFI from the exported
ngx_cycle. The struct prefix used is identical in every nginx core shipped in recent runtimes, but a layout change would silently produce garbage, so there are two guards: every process checks that the first connection slot is wired to the first read/write events, and a worker checks that the number of slots it reads equalsworker_connectionsfrom the config. If that check fails, it logs one warning and the two connection gauges are dropped for every process, including the privileged agent, instead of exporting a wrong number.Tests cover all five gauges for the worker and the privileged agent in both subsystems, a worker's connection gauge going up by the number of cosockets held open and back down when they are closed, the mismatch case, and the stream-only mode in
t/cli/test_prometheus_stream.sh. Docs are updated in both languages.Checklist