nic-6443 commented on code in PR #14034: URL: https://github.com/apache/apisix/pull/14034#discussion_r4278875170
########## apisix/plugins/prometheus/process_metrics.lua: ########## @@ -0,0 +1,385 @@ +-- +-- 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 + + +local function timers_max() + local nginx_config = core.config.local_conf().nginx_config or {} + return nginx_config.max_running_timers or DEFAULT_MAX_RUNNING_TIMERS, + nginx_config.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 = 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 Review Comment: Fixed in 2f0c948. `timers_max()` and the self check now share one `nginx_config()` helper that falls back to `{}`. This can really happen: the CLI refuses to start without `nginx_config`, but a plugin reload re-reads config.yaml through `local_conf(true)`, and `nginx_config: ~` drops the default table during the merge. As for `expected` being nil, treating it as a failed check is deliberate. Without a configured `worker_connections` there is nothing to validate the FFI read against, so the connection gauges stay out and the warning says why ("worker_connections is nil"). Exporting a number we could not check is the one outcome this guard exists to prevent. ########## t/cli/test_prometheus_stream.sh: ########## @@ -124,3 +124,26 @@ if ! echo "$out" | grep "apisix_node_info{hostname=" > /dev/null; then fi echo "passed: prometheus works when only stream is enabled" + +# the http subsystem exists only to export metrics here, yet every process +# still reports its own Lua VMs and connection slots +ok=0 +deadline=$(( $(date +%s) + 20 )) +{ 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]' \ Review Comment: Agreed, fixed in 8c89cc8. The stream-only check now accepts any count, 0 included. The series showing up with a number is what proves the stream VM reports; how many timers happen to be running at that moment is incidental. -- 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]
