From 039e656c1aba2c6d0f5eba16db9f05a410a53a06 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 3 Apr 2024 22:21:40 +0545 Subject: [PATCH 01/11] fix: disable features when plugin is turned off --- apisix/plugin.lua | 2 ++ apisix/plugins/prometheus/exporter.lua | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index c6d1232ff6fa..03fb33326b99 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -376,6 +376,8 @@ function _M.exit_worker() for name in pairs(stream_local_plugins_hash) do unload_plugin(name, PLUGIN_TYPE_STREAM) end + + require("apisix.plugins.prometheus.exporter").destroy() end diff --git a/apisix/plugins/prometheus/exporter.lua b/apisix/plugins/prometheus/exporter.lua index 59da6c67018c..2022f16d8889 100644 --- a/apisix/plugins/prometheus/exporter.lua +++ b/apisix/plugins/prometheus/exporter.lua @@ -522,6 +522,9 @@ _M.get_api = get_api function _M.export_metrics(stream_only) + if not prometheus then + core.response.exit(200, "") + end local api = get_api(false) local uri = ngx.var.uri local method = ngx.req.get_method() @@ -545,4 +548,10 @@ function _M.get_prometheus() return prometheus end + +function _M.destroy() + prometheus = nil +end + + return _M From a4358040b2bd76faafc37a7998d320583c9b345f Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Fri, 5 Apr 2024 17:33:32 +0545 Subject: [PATCH 02/11] support reloading --- apisix/plugin.lua | 26 ++++- apisix/plugins/prometheus/exporter.lua | 126 ++++++++++++++----------- t/cli/test_prometheus_reload.sh | 93 ++++++++++++++++++ 3 files changed, 186 insertions(+), 59 deletions(-) create mode 100755 t/cli/test_prometheus_reload.sh diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 03fb33326b99..d10b877f184a 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -331,6 +331,8 @@ function _M.load(config) return local_plugins end + local exporter = require("apisix.plugins.prometheus.exporter") + if ngx.config.subsystem == "http" then if not http_plugin_names then core.log.error("failed to read plugin list from local file") @@ -344,6 +346,20 @@ function _M.load(config) if not ok then core.log.error("failed to load plugins: ", err) end + + if ngx.config.subsystem == "http" then + local enabled = core.table.array_find(http_plugin_names, "prometheus") ~= nil + local active = exporter.get_prometheus() ~= nil + core.log.warn("inside subsystem: ", enabled, ". ", active, core.json.encode(http_plugin_names)) + if not enabled and active then + core.log.warn("disabled and active") + exporter.destroy() + end + if enabled and not active then + core.log.warn("enabled and inactive") + exporter.http_init() + end + end end end @@ -354,6 +370,14 @@ function _M.load(config) if not ok then core.log.error("failed to load stream plugins: ", err) end + + if ngx.config.subsystem == "stream" then + if not core.table.array_find(stream_plugin_names, "prometheus") then + exporter.destroy() + else + exporter.stream_init() + end + end end -- for test @@ -376,8 +400,6 @@ function _M.exit_worker() for name in pairs(stream_local_plugins_hash) do unload_plugin(name, PLUGIN_TYPE_STREAM) end - - require("apisix.plugins.prometheus.exporter").destroy() end diff --git a/apisix/plugins/prometheus/exporter.lua b/apisix/plugins/prometheus/exporter.lua index 2022f16d8889..5c647343beb3 100644 --- a/apisix/plugins/prometheus/exporter.lua +++ b/apisix/plugins/prometheus/exporter.lua @@ -28,6 +28,7 @@ local pcall = pcall local select = select local type = type local prometheus +local prometheus_bkp local router = require("apisix.router") local get_routes = router.http_routes local get_ssls = router.ssls @@ -112,6 +113,9 @@ function _M.http_init(prometheus_enabled_in_stream) -- todo: support hot reload, we may need to update the lua-prometheus -- library if ngx.get_phase() ~= "init" and ngx.get_phase() ~= "init_worker" then + if prometheus_bkp ~= nil then + prometheus = prometheus_bkp + end return end @@ -227,78 +231,83 @@ end function _M.http_log(conf, ctx) - local vars = ctx.var - - local route_id = "" - local balancer_ip = ctx.balancer_ip or "" - local service_id = "" - local consumer_name = ctx.consumer_name or "" - - local matched_route = ctx.matched_route and ctx.matched_route.value - if matched_route then - route_id = matched_route.id - service_id = matched_route.service_id or "" - if conf.prefer_name == true then - route_id = matched_route.name or route_id - if service_id ~= "" then - local service = service_fetch(service_id) - service_id = service and service.value.name or service_id + -- only record metrics when prometheus is enabled + if prometheus then + local vars = ctx.var + + local route_id = "" + local balancer_ip = ctx.balancer_ip or "" + local service_id = "" + local consumer_name = ctx.consumer_name or "" + + local matched_route = ctx.matched_route and ctx.matched_route.value + if matched_route then + route_id = matched_route.id + service_id = matched_route.service_id or "" + if conf.prefer_name == true then + route_id = matched_route.name or route_id + if service_id ~= "" then + local service = service_fetch(service_id) + service_id = service and service.value.name or service_id + end end end - end - - local matched_uri = "" - local matched_host = "" - if ctx.curr_req_matched then - matched_uri = ctx.curr_req_matched._path or "" - matched_host = ctx.curr_req_matched._host or "" - end - metrics.status:inc(1, - gen_arr(vars.status, route_id, matched_uri, matched_host, - service_id, consumer_name, balancer_ip, - unpack(extra_labels("http_status", ctx)))) + local matched_uri = "" + local matched_host = "" + if ctx.curr_req_matched then + matched_uri = ctx.curr_req_matched._path or "" + matched_host = ctx.curr_req_matched._host or "" + end - local latency, upstream_latency, apisix_latency = latency_details(ctx) - local latency_extra_label_values = extra_labels("http_latency", ctx) + metrics.status:inc(1, + gen_arr(vars.status, route_id, matched_uri, matched_host, + service_id, consumer_name, balancer_ip, + unpack(extra_labels("http_status", ctx)))) - metrics.latency:observe(latency, - gen_arr("request", route_id, service_id, consumer_name, balancer_ip, - unpack(latency_extra_label_values))) + local latency, upstream_latency, apisix_latency = latency_details(ctx) + local latency_extra_label_values = extra_labels("http_latency", ctx) - if upstream_latency then - metrics.latency:observe(upstream_latency, - gen_arr("upstream", route_id, service_id, consumer_name, balancer_ip, + metrics.latency:observe(latency, + gen_arr("request", route_id, service_id, consumer_name, balancer_ip, unpack(latency_extra_label_values))) - end - metrics.latency:observe(apisix_latency, - gen_arr("apisix", route_id, service_id, consumer_name, balancer_ip, - unpack(latency_extra_label_values))) + if upstream_latency then + metrics.latency:observe(upstream_latency, + gen_arr("upstream", route_id, service_id, consumer_name, balancer_ip, + unpack(latency_extra_label_values))) + end - local bandwidth_extra_label_values = extra_labels("bandwidth", ctx) + metrics.latency:observe(apisix_latency, + gen_arr("apisix", route_id, service_id, consumer_name, balancer_ip, + unpack(latency_extra_label_values))) + + local bandwidth_extra_label_values = extra_labels("bandwidth", ctx) - metrics.bandwidth:inc(vars.request_length, - gen_arr("ingress", route_id, service_id, consumer_name, balancer_ip, - unpack(bandwidth_extra_label_values))) + metrics.bandwidth:inc(vars.request_length, + gen_arr("ingress", route_id, service_id, consumer_name, balancer_ip, + unpack(bandwidth_extra_label_values))) - metrics.bandwidth:inc(vars.bytes_sent, - gen_arr("egress", route_id, service_id, consumer_name, balancer_ip, - unpack(bandwidth_extra_label_values))) + metrics.bandwidth:inc(vars.bytes_sent, + gen_arr("egress", route_id, service_id, consumer_name, balancer_ip, + unpack(bandwidth_extra_label_values))) + end end function _M.stream_log(conf, ctx) - local route_id = "" - local matched_route = ctx.matched_route and ctx.matched_route.value - if matched_route then - route_id = matched_route.id - if conf.prefer_name == true then - route_id = matched_route.name or route_id + if prometheus then + local route_id = "" + local matched_route = ctx.matched_route and ctx.matched_route.value + if matched_route then + route_id = matched_route.id + if conf.prefer_name == true then + route_id = matched_route.name or route_id + end end - end - metrics.stream_connection_total:inc(1, gen_arr(route_id)) + metrics.stream_connection_total:inc(1, gen_arr(route_id)) + end end @@ -523,7 +532,7 @@ _M.get_api = get_api function _M.export_metrics(stream_only) if not prometheus then - core.response.exit(200, "") + core.response.exit(200, "{}") end local api = get_api(false) local uri = ngx.var.uri @@ -550,7 +559,10 @@ end function _M.destroy() - prometheus = nil + if prometheus ~= nil then + prometheus_bkp = core.table.deepcopy(prometheus) + prometheus = nil + end end diff --git a/t/cli/test_prometheus_reload.sh b/t/cli/test_prometheus_reload.sh new file mode 100755 index 000000000000..a6ce4ada2031 --- /dev/null +++ b/t/cli/test_prometheus_reload.sh @@ -0,0 +1,93 @@ +#!/usr/bin/env bash + +# +# 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. +# + +. ./t/cli/common.sh + +git checkout conf/config.yaml + +sleep 1 + +make run + +sleep 2 + +echo "removing prometheus from the plugins list" +echo ' +deployment: + role: traditional + role_traditional: + config_provider: etcd + admin: + admin_key: null +apisix: + node_listen: 1984 +plugins: + - ip-restriction' > conf/config.yaml + +echo "fetch metrics, should not contain {}" + +if curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/null; then + echo "failed: metrics should not contain '{}' when prometheus is enabled" + exit 1 +fi + +echo "calling reload API to actually disable prometheus" + +curl -i http://127.0.0.1:9090/v1/plugins/reload -XPUT + +sleep 2 + +echo "fetch metrics after reload should contain {}" + +if ! curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/null; then + echo "failed: metrics should contain '{}' when prometheus is disabled" + exit 1 +fi + +echo "re-enable prometheus" + +echo ' +deployment: + role: traditional + role_traditional: + config_provider: etcd + admin: + admin_key: null +apisix: + node_listen: 1984 +plugins: + - prometheus' > conf/config.yaml + +echo "fetching metrics without reloading should give same result as before" + +if ! curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/null; then + echo "failed: metrics should contain '{}' when prometheus is disabled" + exit 1 +fi + +echo "calling reload API to actually enable prometheus" + +curl -i http://127.0.0.1:9090/v1/plugins/reload -XPUT + +sleep 2 + +if curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/null; then + echo "failed: metrics should not contain '{}' when prometheus is enabled" + exit 1 +fi From 7b9cd26fef7ca902b7396dbf3f3e30c3279e29c7 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Fri, 5 Apr 2024 21:41:10 +0545 Subject: [PATCH 03/11] tests for batch processor --- t/plugin/prometheus4.t | 137 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/t/plugin/prometheus4.t b/t/plugin/prometheus4.t index 93c6f0d2d545..870ced5d3726 100644 --- a/t/plugin/prometheus4.t +++ b/t/plugin/prometheus4.t @@ -277,3 +277,140 @@ apisix_http_latency_bucket\{type="upstream",route="1",service="",consumer="",nod apisix_http_latency_bucket\{type="upstream",route="1",service="",consumer="",node="127.0.0.1",le="105"\} \d+ apisix_http_latency_bucket\{type="upstream",route="1",service="",consumer="",node="127.0.0.1",le="205"\} \d+ apisix_http_latency_bucket\{type="upstream",route="1",service="",consumer="",node="127.0.0.1",le="505"\} \d+/ + + + +=== TEST 10: set sys plugins +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/9', + ngx.HTTP_PUT, + [[{ + "methods": ["GET"], + "plugins": { + "prometheus": {}, + "syslog": { + "host": "127.0.0.1", + "include_req_body": false, + "max_retry_times": 1, + "tls": false, + "retry_interval": 1, + "batch_max_size": 1000, + "buffer_duration": 60, + "port": 1000, + "name": "sys-logger", + "flush_limit": 4096, + "sock_type": "tcp", + "timeout": 3, + "drop_limit": 1048576, + "pool_size": 5 + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/batch-process-metrics" + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 11: remove prometheus -> reload -> send batch request -> add prometheus for next tests +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local httpc = http.new() + + local t = require("lib.test_admin").test + ngx.sleep(0.1) + local data = [[ +deployment: + role: traditional + role_traditional: + config_provider: etcd + admin: + admin_key: null +apisix: + node_listen: 1984 +plugins: + - example-plugin +plugin_attr: + example-plugin: + val: 1 + ]] + require("lib.test_admin").set_config_yaml(data) + local code, _, org_body = t('/v1/plugins/reload', ngx.HTTP_PUT) + local code, body = t('/batch-process-metrics', + ngx.HTTP_GET + ) + + ngx.status = code + ngx.say(body) + + + local data = [[ +deployment: + role: traditional + role_traditional: + config_provider: etcd + admin: + admin_key: null +apisix: + node_listen: 1984 +plugins: + - prometheus +plugin_attr: + example-plugin: + val: 1 + ]] + require("lib.test_admin").set_config_yaml(data) + } + } +--- request +GET /t +--- error_code: 404 +--- response_body_like eval +qr/404 Not Found/ + + + +=== TEST 12: fetch prometheus metrics -> batch_process_entries metrics should not be present +--- request +GET /apisix/prometheus/metrics +--- error_code: 200 +--- response_body_unlike eval +qr/apisix_batch_process_entries\{name="sys-logger",route_id="9",server_addr="127.0.0.1"\} \d+/ + + + +=== TEST 13: hit batch-process-metrics with prometheus enabled from TEST 11 +--- request +GET /batch-process-metrics +--- error_code: 404 + + + +=== TEST 14: batch_process_entries metrics should be present now +--- request +GET /apisix/prometheus/metrics +--- error_code: 200 +--- response_body_like eval +qr/apisix_batch_process_entries\{name="sys-logger",route_id="9",server_addr="127.0.0.1"\} \d+/ From f30eaa91b23225a8914083e96e4333b96d43ac41 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sat, 6 Apr 2024 08:11:32 +0545 Subject: [PATCH 04/11] remove logs --- apisix/plugin.lua | 3 --- 1 file changed, 3 deletions(-) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index d10b877f184a..a5ce85ca405e 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -350,13 +350,10 @@ function _M.load(config) if ngx.config.subsystem == "http" then local enabled = core.table.array_find(http_plugin_names, "prometheus") ~= nil local active = exporter.get_prometheus() ~= nil - core.log.warn("inside subsystem: ", enabled, ". ", active, core.json.encode(http_plugin_names)) if not enabled and active then - core.log.warn("disabled and active") exporter.destroy() end if enabled and not active then - core.log.warn("enabled and inactive") exporter.http_init() end end From eab64d3562b952151bc3f9b2dad41d2c28b7c493 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sat, 6 Apr 2024 22:30:47 +0545 Subject: [PATCH 05/11] test stream hot reload --- t/cli/test_prometheus_reload.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/t/cli/test_prometheus_reload.sh b/t/cli/test_prometheus_reload.sh index a6ce4ada2031..3d74d3513612 100755 --- a/t/cli/test_prometheus_reload.sh +++ b/t/cli/test_prometheus_reload.sh @@ -91,3 +91,31 @@ if curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/nu echo "failed: metrics should not contain '{}' when prometheus is enabled" exit 1 fi + +echo "disable http prometheus and enable stream prometheus and call reload" + +exit_if_not_customed_nginx + +echo " +apisix: + proxy_mode: http&stream + enable_admin: true + stream_proxy: + tcp: + - addr: 9100 +plugins: + - example-plugin +stream_plugins: + - prometheus +" > conf/config.yaml + +curl -i http://127.0.0.1:9090/v1/plugins/reload -XPUT + +sleep 2 + +echo "fetching metrics should actually work demonstrating hot reload" + +if ! curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/null; then + echo "failed: metrics should not contain '{}' when prometheus is enabled" + exit 1 +fi From ad3229928e6d12512256faf767ba225aa3826266 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sat, 6 Apr 2024 22:36:40 +0545 Subject: [PATCH 06/11] remove check for log phase as they are called only when prometheus is enabled by default/design lol --- apisix/plugins/prometheus/exporter.lua | 115 ++++++++++++------------- 1 file changed, 55 insertions(+), 60 deletions(-) diff --git a/apisix/plugins/prometheus/exporter.lua b/apisix/plugins/prometheus/exporter.lua index 5c647343beb3..a21acfcab816 100644 --- a/apisix/plugins/prometheus/exporter.lua +++ b/apisix/plugins/prometheus/exporter.lua @@ -231,83 +231,78 @@ end function _M.http_log(conf, ctx) - -- only record metrics when prometheus is enabled - if prometheus then - local vars = ctx.var - - local route_id = "" - local balancer_ip = ctx.balancer_ip or "" - local service_id = "" - local consumer_name = ctx.consumer_name or "" - - local matched_route = ctx.matched_route and ctx.matched_route.value - if matched_route then - route_id = matched_route.id - service_id = matched_route.service_id or "" - if conf.prefer_name == true then - route_id = matched_route.name or route_id - if service_id ~= "" then - local service = service_fetch(service_id) - service_id = service and service.value.name or service_id - end + local vars = ctx.var + + local route_id = "" + local balancer_ip = ctx.balancer_ip or "" + local service_id = "" + local consumer_name = ctx.consumer_name or "" + + local matched_route = ctx.matched_route and ctx.matched_route.value + if matched_route then + route_id = matched_route.id + service_id = matched_route.service_id or "" + if conf.prefer_name == true then + route_id = matched_route.name or route_id + if service_id ~= "" then + local service = service_fetch(service_id) + service_id = service and service.value.name or service_id end end + end - local matched_uri = "" - local matched_host = "" - if ctx.curr_req_matched then - matched_uri = ctx.curr_req_matched._path or "" - matched_host = ctx.curr_req_matched._host or "" - end - - metrics.status:inc(1, - gen_arr(vars.status, route_id, matched_uri, matched_host, - service_id, consumer_name, balancer_ip, - unpack(extra_labels("http_status", ctx)))) + local matched_uri = "" + local matched_host = "" + if ctx.curr_req_matched then + matched_uri = ctx.curr_req_matched._path or "" + matched_host = ctx.curr_req_matched._host or "" + end - local latency, upstream_latency, apisix_latency = latency_details(ctx) - local latency_extra_label_values = extra_labels("http_latency", ctx) + metrics.status:inc(1, + gen_arr(vars.status, route_id, matched_uri, matched_host, + service_id, consumer_name, balancer_ip, + unpack(extra_labels("http_status", ctx)))) - metrics.latency:observe(latency, - gen_arr("request", route_id, service_id, consumer_name, balancer_ip, - unpack(latency_extra_label_values))) + local latency, upstream_latency, apisix_latency = latency_details(ctx) + local latency_extra_label_values = extra_labels("http_latency", ctx) - if upstream_latency then - metrics.latency:observe(upstream_latency, - gen_arr("upstream", route_id, service_id, consumer_name, balancer_ip, - unpack(latency_extra_label_values))) - end + metrics.latency:observe(latency, + gen_arr("request", route_id, service_id, consumer_name, balancer_ip, + unpack(latency_extra_label_values))) - metrics.latency:observe(apisix_latency, - gen_arr("apisix", route_id, service_id, consumer_name, balancer_ip, + if upstream_latency then + metrics.latency:observe(upstream_latency, + gen_arr("upstream", route_id, service_id, consumer_name, balancer_ip, unpack(latency_extra_label_values))) + end - local bandwidth_extra_label_values = extra_labels("bandwidth", ctx) + metrics.latency:observe(apisix_latency, + gen_arr("apisix", route_id, service_id, consumer_name, balancer_ip, + unpack(latency_extra_label_values))) - metrics.bandwidth:inc(vars.request_length, - gen_arr("ingress", route_id, service_id, consumer_name, balancer_ip, - unpack(bandwidth_extra_label_values))) + local bandwidth_extra_label_values = extra_labels("bandwidth", ctx) - metrics.bandwidth:inc(vars.bytes_sent, - gen_arr("egress", route_id, service_id, consumer_name, balancer_ip, - unpack(bandwidth_extra_label_values))) - end + metrics.bandwidth:inc(vars.request_length, + gen_arr("ingress", route_id, service_id, consumer_name, balancer_ip, + unpack(bandwidth_extra_label_values))) + + metrics.bandwidth:inc(vars.bytes_sent, + gen_arr("egress", route_id, service_id, consumer_name, balancer_ip, + unpack(bandwidth_extra_label_values))) end function _M.stream_log(conf, ctx) - if prometheus then - local route_id = "" - local matched_route = ctx.matched_route and ctx.matched_route.value - if matched_route then - route_id = matched_route.id - if conf.prefer_name == true then - route_id = matched_route.name or route_id - end + local route_id = "" + local matched_route = ctx.matched_route and ctx.matched_route.value + if matched_route then + route_id = matched_route.id + if conf.prefer_name == true then + route_id = matched_route.name or route_id end - - metrics.stream_connection_total:inc(1, gen_arr(route_id)) end + + metrics.stream_connection_total:inc(1, gen_arr(route_id)) end From 7ab3625771e85e660ef4b77f6dec7501fc57bc82 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 7 Apr 2024 21:27:01 +0545 Subject: [PATCH 07/11] need not check stream --- apisix/plugin.lua | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index a5ce85ca405e..933dbe94e0c1 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -347,15 +347,13 @@ function _M.load(config) core.log.error("failed to load plugins: ", err) end - if ngx.config.subsystem == "http" then - local enabled = core.table.array_find(http_plugin_names, "prometheus") ~= nil - local active = exporter.get_prometheus() ~= nil - if not enabled and active then - exporter.destroy() - end - if enabled and not active then - exporter.http_init() - end + local enabled = core.table.array_find(http_plugin_names, "prometheus") ~= nil + local active = exporter.get_prometheus() ~= nil + if not enabled and active then + exporter.destroy() + end + if enabled and not active then + exporter.http_init() end end end @@ -367,14 +365,6 @@ function _M.load(config) if not ok then core.log.error("failed to load stream plugins: ", err) end - - if ngx.config.subsystem == "stream" then - if not core.table.array_find(stream_plugin_names, "prometheus") then - exporter.destroy() - else - exporter.stream_init() - end - end end -- for test From 2e65e5cc7be4ddbf0d3cb3c3adee5206a5d79824 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 7 Apr 2024 21:27:17 +0545 Subject: [PATCH 08/11] remove stream tests --- t/cli/test_prometheus_reload.sh | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/t/cli/test_prometheus_reload.sh b/t/cli/test_prometheus_reload.sh index 3d74d3513612..a6ce4ada2031 100755 --- a/t/cli/test_prometheus_reload.sh +++ b/t/cli/test_prometheus_reload.sh @@ -91,31 +91,3 @@ if curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/nu echo "failed: metrics should not contain '{}' when prometheus is enabled" exit 1 fi - -echo "disable http prometheus and enable stream prometheus and call reload" - -exit_if_not_customed_nginx - -echo " -apisix: - proxy_mode: http&stream - enable_admin: true - stream_proxy: - tcp: - - addr: 9100 -plugins: - - example-plugin -stream_plugins: - - prometheus -" > conf/config.yaml - -curl -i http://127.0.0.1:9090/v1/plugins/reload -XPUT - -sleep 2 - -echo "fetching metrics should actually work demonstrating hot reload" - -if ! curl -i http://127.0.0.1:9091/apisix/prometheus/metrics | grep "{}" > /dev/null; then - echo "failed: metrics should not contain '{}' when prometheus is enabled" - exit 1 -fi From 9d5689c5ad05f43b2eea4e1d93a08bedc240fdd5 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 8 Apr 2024 16:42:43 +0545 Subject: [PATCH 09/11] fix lint --- t/plugin/prometheus4.t | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/t/plugin/prometheus4.t b/t/plugin/prometheus4.t index 870ced5d3726..79c65f640ba4 100644 --- a/t/plugin/prometheus4.t +++ b/t/plugin/prometheus4.t @@ -292,21 +292,21 @@ apisix_http_latency_bucket\{type="upstream",route="1",service="",consumer="",nod "plugins": { "prometheus": {}, "syslog": { - "host": "127.0.0.1", - "include_req_body": false, - "max_retry_times": 1, - "tls": false, - "retry_interval": 1, - "batch_max_size": 1000, - "buffer_duration": 60, - "port": 1000, - "name": "sys-logger", - "flush_limit": 4096, - "sock_type": "tcp", - "timeout": 3, - "drop_limit": 1048576, - "pool_size": 5 - } + "host": "127.0.0.1", + "include_req_body": false, + "max_retry_times": 1, + "tls": false, + "retry_interval": 1, + "batch_max_size": 1000, + "buffer_duration": 60, + "port": 1000, + "name": "sys-logger", + "flush_limit": 4096, + "sock_type": "tcp", + "timeout": 3, + "drop_limit": 1048576, + "pool_size": 5 + } }, "upstream": { "nodes": { @@ -361,7 +361,7 @@ plugin_attr: local code, body = t('/batch-process-metrics', ngx.HTTP_GET ) - + ngx.status = code ngx.say(body) From 669dc182fe77717685e7dd4a56a39a4cf2122baa Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 8 Apr 2024 19:34:08 +0545 Subject: [PATCH 10/11] code review --- apisix/plugin.lua | 2 +- apisix/plugins/prometheus/exporter.lua | 2 +- t/plugin/prometheus4.t | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 933dbe94e0c1..cabcda176535 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -349,7 +349,7 @@ function _M.load(config) local enabled = core.table.array_find(http_plugin_names, "prometheus") ~= nil local active = exporter.get_prometheus() ~= nil - if not enabled and active then + if not enabled then exporter.destroy() end if enabled and not active then diff --git a/apisix/plugins/prometheus/exporter.lua b/apisix/plugins/prometheus/exporter.lua index a21acfcab816..fc282031ee70 100644 --- a/apisix/plugins/prometheus/exporter.lua +++ b/apisix/plugins/prometheus/exporter.lua @@ -113,7 +113,7 @@ function _M.http_init(prometheus_enabled_in_stream) -- todo: support hot reload, we may need to update the lua-prometheus -- library if ngx.get_phase() ~= "init" and ngx.get_phase() ~= "init_worker" then - if prometheus_bkp ~= nil then + if prometheus_bkp then prometheus = prometheus_bkp end return diff --git a/t/plugin/prometheus4.t b/t/plugin/prometheus4.t index 79c65f640ba4..89190448e731 100644 --- a/t/plugin/prometheus4.t +++ b/t/plugin/prometheus4.t @@ -340,7 +340,7 @@ passed local httpc = http.new() local t = require("lib.test_admin").test - ngx.sleep(0.1) + ngx.sleep(0.1) local data = [[ deployment: role: traditional @@ -365,7 +365,6 @@ plugin_attr: ngx.status = code ngx.say(body) - local data = [[ deployment: role: traditional From ee5972fb5e5babf5f001ab83c356e0df540714dc Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 8 Apr 2024 19:44:11 +0545 Subject: [PATCH 11/11] remove sleep --- t/cli/test_prometheus_reload.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/t/cli/test_prometheus_reload.sh b/t/cli/test_prometheus_reload.sh index a6ce4ada2031..7a8b1a1f810f 100755 --- a/t/cli/test_prometheus_reload.sh +++ b/t/cli/test_prometheus_reload.sh @@ -21,8 +21,6 @@ git checkout conf/config.yaml -sleep 1 - make run sleep 2