feat: server info #2884
feat: server info #2884
Conversation
tokers
requested review from
membphis,
spacewander and
juzhiyuan
and removed request for
membphis
|
CI failed, you need to fix it first |
Fixed, except the markdown link errors. |
apisix/admin/init.lua
Outdated
| @@ -316,6 +330,11 @@ local uri_route = { | |||
| methods = {"PUT"}, | |||
| handler = post_reload_plugins, | |||
| }, | |||
| { | |||
| paths = [[/apisix/admin/server_info]], | |||
There was a problem hiding this comment.
I think we need do this in DP, not admin API
There was a problem hiding this comment.
And we should implement this feature by plugin way.
Take a look at: https://github.com/apache/apisix/blob/master/apisix/plugins/node-status.lua
There was a problem hiding this comment.
What about the control API's milestone?
There was a problem hiding this comment.
There is not milestone. So far, we can hide the entry first.
| ## 测试插件 | ||
|
|
||
| ```bash | ||
| curl http://127.0.0.1:9080/apisix/admin/server_info -s | jq |
|
|
||
| ## 注意事项 | ||
|
|
||
| 当使用 etcd 作为 APISIX 的数据中心的说话，服务信息将被周期性地上报到 etcd（目前的上报间隔是 5 |
There was a problem hiding this comment.
"当使用 etcd 作为 APISIX 的数据中心的说话"
I can not understand it
| --- timeout: 6 | ||
| --- no_error_log | ||
| [error] | ||
|
|
There was a problem hiding this comment.
one blank line is enough at the end of file
| end | ||
|
|
||
| local opts = { | ||
| check_interval = 5, -- in seconds |
There was a problem hiding this comment.
5 seconds is short, I think 10 mins should be good
There was a problem hiding this comment.
and we should allow the user to specify this field
There was a problem hiding this comment.
I still think 10 mins is too long, also for now, all the basic server info are static (except the up_time and last_report_time), a relative short period is more suitable so we can easily judge whether a node is healthy or unstable when we are viewing the server info on Dashboard.
What about 1 min or 2 mins?
| check_interval = 5, -- in seconds | ||
| } | ||
|
|
||
| if core.config ~= require("apisix.core.config_etcd") then |
There was a problem hiding this comment.
need some comment for why call return here
| return nil, err | ||
| end | ||
|
|
||
| local key = "/data_plane/server_info/" .. server_info.id |
There was a problem hiding this comment.
we can not use data_plane here. CP will write the server info too.
how about /nodes/server_info/{node_id}?
There was a problem hiding this comment.
The key /nodes/server_info/{node_id} is good, but why CP also writes the server info?
| @@ -18,6 +18,7 @@ local require = require | |||
| local core = require("apisix.core") | |||
| local route = require("resty.radixtree") | |||
| local plugin = require("apisix.plugin") | |||
|
|
|||
| @@ -29,6 +30,7 @@ local reload_event = "/apisix/admin/plugins/reload" | |||
| local ipairs = ipairs | |||
| local error = error | |||
| local events | |||
|
|
|||
There was a problem hiding this comment.
ditto
not related to this title of PR
|
|
||
| By removing `server-info` in the plugin list of configure file `apisix/conf/config.yaml` and restart APISIX, you can diable `server-info` plugin easily. | ||
|
|
||
| ``` |
|
@agile6v We can add some limit to the report interval, such as the minimal report interval is 10s. |
agree. I think the minimal report interval can be |
If only |
|
This PR was pushed directly at apache/apisix, i will close it and please see the new one #2926. |
What this PR does / why we need it:
Close #2821.
Pre-submission checklist: