Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(prometheus): disable features when prometheus plugin is turned off #11117

Merged
11 changes: 11 additions & 0 deletions apisix/plugin.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -344,6 +346,15 @@ function _M.load(config)
if not ok then
core.log.error("failed to load plugins: ", err)
end

local enabled = core.table.array_find(http_plugin_names, "prometheus") ~= nil
local active = exporter.get_prometheus() ~= nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need to get the active status.
Since the exporter.destroy is harmless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And so do line355 ~ line357

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no I think the second if check needs the active status. Consider this case:

  • apisix is running with prometheus active and enabled
  • hit plugin reload

In this case enabled == true and active == true. But we need not call http_init() in prometheus because it is already running during this operation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

if not enabled then
exporter.destroy()
end
if enabled and not active then
exporter.http_init()
end
end
end

Expand Down
16 changes: 16 additions & 0 deletions apisix/plugins/prometheus/exporter.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 then
prometheus = prometheus_bkp
end
return
end

Expand Down Expand Up @@ -522,6 +526,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()
Expand All @@ -545,4 +552,13 @@ function _M.get_prometheus()
return prometheus
end


function _M.destroy()
if prometheus ~= nil then
prometheus_bkp = core.table.deepcopy(prometheus)
prometheus = nil
end
end


return _M
91 changes: 91 additions & 0 deletions t/cli/test_prometheus_reload.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#!/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

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
136 changes: 136 additions & 0 deletions t/plugin/prometheus4.t
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,139 @@ 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+/
Loading