nic-6443 commented on code in PR #14034:
URL: https://github.com/apache/apisix/pull/14034#discussion_r4280005211


##########
apisix/plugins/prometheus/process_metrics.lua:
##########
@@ -0,0 +1,393 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+-- Per-process OpenResty internals: Lua timers, connection slots and Lua VM
+-- memory. These values only exist inside the process they describe, while the
+-- exposition is rendered by a single process, so every process writes its own
+-- gauges into the shared metric dict from a timer, and the process rendering
+-- the exposition refreshes its own values right before it does.
+local core          = require("apisix.core")
+local ffi           = require("ffi")
+local process       = require("ngx.process")
+local C             = ffi.C
+local ngx           = ngx
+local timer_at      = ngx.timer.at
+local collectgarbage = collectgarbage
+local tonumber      = tonumber
+local tostring      = tostring
+local pcall         = pcall
+local ipairs        = ipairs
+local select        = select
+
+
+-- The prefix of struct ngx_cycle_s up to write_events. Its layout is the same
+-- in every nginx core shipped by the supported runtimes (1.21.4 to 1.29.x);
+-- the self check in check_cycle_layout() is what keeps a future change from
+-- turning into a wrong number.
+ffi.cdef[[
+typedef struct {
+    uintptr_t       log_level;
+    void           *file;
+    uintptr_t       connection;
+    long            disk_full_time;
+    void           *handler;
+    void           *data;
+    void           *writer;
+    void           *wdata;
+    char           *action;
+    void           *next;
+} apisix_prom_ngx_log_t;
+
+typedef struct {
+    void           *elts;
+    uintptr_t       nelts;
+    size_t          size;
+    uintptr_t       nalloc;
+    void           *pool;
+} apisix_prom_ngx_array_t;
+
+typedef struct {
+    void           *elts;
+    uintptr_t       nelts;
+    void           *next;
+} apisix_prom_ngx_list_part_t;
+
+typedef struct {
+    void                        *last;
+    apisix_prom_ngx_list_part_t  part;
+    size_t                       size;
+    uintptr_t                    nalloc;
+    void                        *pool;
+} apisix_prom_ngx_list_t;
+
+typedef struct {
+    uintptr_t       key;
+    void           *left;
+    void           *right;
+    void           *parent;
+    unsigned char   color;
+    unsigned char   data;
+} apisix_prom_ngx_rbtree_node_t;
+
+typedef struct {
+    void           *root;
+    void           *sentinel;
+    void           *insert;
+} apisix_prom_ngx_rbtree_t;
+
+typedef struct {
+    void           *data;
+    void           *read;
+    void           *write;
+} apisix_prom_ngx_connection_head_t;
+
+typedef struct {
+    void                           *conf_ctx;
+    void                           *pool;
+    void                           *log;
+    apisix_prom_ngx_log_t           new_log;
+    uintptr_t                       log_use_stderr;
+    void                           *files;
+    void                           *free_connections;
+    uintptr_t                       free_connection_n;
+    void                           *modules;
+    uintptr_t                       modules_n;
+    uintptr_t                       modules_used;
+    void                           *reusable_connections_queue[2];
+    uintptr_t                       reusable_connections_n;
+    long                            connections_reuse_time;
+    apisix_prom_ngx_array_t         listening;
+    apisix_prom_ngx_array_t         paths;
+    apisix_prom_ngx_array_t         config_dump;
+    apisix_prom_ngx_rbtree_t        config_dump_rbtree;
+    apisix_prom_ngx_rbtree_node_t   config_dump_sentinel;
+    apisix_prom_ngx_list_t          open_files;
+    apisix_prom_ngx_list_t          shared_memory;
+    uintptr_t                       connection_n;
+    uintptr_t                       files_n;
+    apisix_prom_ngx_connection_head_t *connections;
+    void                           *read_events;
+    void                           *write_events;
+} apisix_prom_ngx_cycle_t;
+
+extern apisix_prom_ngx_cycle_t *ngx_cycle;
+]]
+
+
+-- nginx's own defaults for lua_max_running_timers / lua_max_pending_timers,
+-- in effect when the configuration leaves them out
+local DEFAULT_MAX_RUNNING_TIMERS = 256
+local DEFAULT_MAX_PENDING_TIMERS = 1024
+local DEFAULT_INTERVAL = 15
+
+-- Shared by every process, so a layout mismatch found by one worker keeps the
+-- connection gauges out of the exposition everywhere, including the
+-- privileged agent, whose connection count cannot be checked against config.
+local CONNECTIONS_VERDICT_KEY = "process_metrics:connections_readable"
+
+local _M = {}
+
+local gauges
+local dict
+local process_label
+local no_extra_labels = {}
+local extra_label_names = no_extra_labels
+local extra_label_values
+local subsystem = ngx.config.subsystem
+local verdict_logged = false
+
+local timer_running = false
+local stopped = true
+local interval = DEFAULT_INTERVAL
+
+
+local function get_process_label()
+    if process.type() == "privileged agent" then
+        return "privileged-agent"
+    end
+
+    return "worker-" .. tostring(ngx.worker.id() or 0)
+end
+
+
+-- The CLI refuses to start without nginx_config, but a forced re-read of
+-- config.yaml at runtime (plugin reload) can still drop it: `nginx_config: ~`
+-- deletes the default table while merging.
+local function nginx_config()
+    return core.config.local_conf().nginx_config or {}
+end
+
+
+local function timers_max()
+    local conf = nginx_config()
+    return conf.max_running_timers or DEFAULT_MAX_RUNNING_TIMERS,
+           conf.max_pending_timers or DEFAULT_MAX_PENDING_TIMERS
+end
+
+
+local function read_cycle()
+    local cycle = C.ngx_cycle
+    local connection_n = tonumber(cycle.connection_n)
+    local free_n = tonumber(cycle.free_connection_n)
+
+    -- every connection slot is wired to its own pair of events at startup, so
+    -- this pins down the offsets right after connection_n as well
+    local conns = cycle.connections
+    if connection_n == 0 or free_n > connection_n or conns == nil
+       or conns[0].read ~= cycle.read_events or conns[0].write ~= 
cycle.write_events
+    then
+        return nil
+    end
+
+    return connection_n, free_n
+end
+
+
+local function check_cycle_layout()
+    local ok, connection_n = pcall(read_cycle)
+    if not ok or not connection_n then
+        return false, "unexpected nginx cycle layout"
+    end
+
+    local event = 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
+
+    return true
+end
+
+
+-- Only a worker can tell whether the numbers read through FFI are right: its
+-- connection slots must match worker_connections from the configuration.
+local function connections_readable()
+    local readable = dict:get(CONNECTIONS_VERDICT_KEY)
+    if readable ~= nil then
+        return readable
+    end
+
+    if process.type() == "privileged agent" then
+        -- no worker has checked yet
+        return false
+    end
+
+    local ok, err = check_cycle_layout()
+    dict:set(CONNECTIONS_VERDICT_KEY, ok)
+    if not ok then
+        gauges.connections:reset()
+        gauges.connections_max:reset()
+        if not verdict_logged then
+            verdict_logged = true
+            core.log.warn("process connection metrics are disabled: ", err)
+        end
+    end
+
+    return ok
+end
+
+
+local function with_extra_labels(labels)
+    for _, name in ipairs(extra_label_names) do
+        labels[#labels + 1] = name
+    end
+    return labels
+end
+
+
+local function label_values(extra, ...)
+    local values = {...}
+    local n = select("#", ...)
+    for i, value in ipairs(extra) do
+        values[n + i] = value
+    end
+    return values
+end
+
+
+-- extra_labels, when given, appends the same labels to every gauge here:
+-- {names = {...}, values = function() return {...} end}
+function _M.init(registry, extra_labels)
+    dict = registry.dict
+    if extra_labels then
+        extra_label_names = extra_labels.names
+        extra_label_values = extra_labels.values
+    end
+
+    gauges = {
+        timers = registry:gauge("nginx_timers",
+            "Number of Lua timers per process and subsystem, by state",
+            with_extra_labels({"process", "subsystem", "state"})),
+        timers_max = registry:gauge("nginx_timers_max",
+            "Configured maximum number of Lua timers per process, by state",
+            with_extra_labels({"state"})),
+        connections = registry:gauge("nginx_process_connections",
+            "Number of connection slots in use per nginx process",
+            with_extra_labels({"process"})),
+        connections_max = registry:gauge("nginx_process_connections_max",
+            "Number of connection slots per nginx process",
+            with_extra_labels({"process"})),
+        lua_memory = registry:gauge("nginx_lua_memory_bytes",
+            "Memory allocated by the Lua VM per process and subsystem",
+            with_extra_labels({"process", "subsystem"})),

Review Comment:
   Good catch, fixed in 6d33fdb. I went with cleanup instead of an expiry. 
Expiring a gauge only works when every `set()` also renews its key-index entry. 
The library version bundled here does that, but older nginx-lua-prometheus 
versions renew only on the first set of a series, so a live series would 
quietly expire. The module should not depend on that.
   
   When collection starts, the privileged agent (http VM) now removes every 
`worker-<id>` series of the process gauges whose id is `>= ngx.worker.count()`. 
It runs a second pass one interval later, to catch a sample an exiting worker 
might still have been writing. The PA is the natural owner here: it always 
exists, and it restarts on every reload.
   
   On disable: the process series are not deleted. They stop updating, because 
every process stops its reporting timer, and the exporter returns `{}` while 
the plugin is off, so nothing stale is exposed. Re-enabling restarts the 
timers, which overwrite the values right away.
   
   Regression coverage is in `t/cli/test_prometheus_reload.sh`: it starts with 
2 workers, reloads with 1, and polls for up to 20s until `worker-0` is reported 
and no `worker-1` series is left. With the cleanup disabled, that step fails.



-- 
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]

Reply via email to