-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix(prometheus): disable features when prometheus plugin is turned off #11117
Conversation
86d0e0e
to
eab64d3
Compare
… enabled by default/design lol
@@ -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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
Description
This PR allows disabling the export server when prometheus is removed from plugins list in config.yaml and then the plugin reload api is called.
Fixes #11046
Checklist