You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "kingluo (via GitHub)" <gi...@apache.org> on 2023/03/06 10:05:05 UTC
[GitHub] [apisix] kingluo opened a new pull request, #9012: feat: move server-info plugin into core
kingluo opened a new pull request, #9012:
URL: https://github.com/apache/apisix/pull/9012
### Description
Add `GET /apisix/admin/data_planes` admin API, so that we could know how many data planes.
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1136438460
##########
docs/en/latest/get-data-planes-from-etcd.md:
##########
@@ -0,0 +1,45 @@
+---
+id: get-data-planes-from-etcd
+title: Get data planes from etcd
+keywords:
+ - API gateway
+ - Apache APISIX
+description: Get data planes from etcd
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+To query all data planes information from etcd, we move `server-info` plugin into the APISIX core, and use it to report data plane information.
+
+The data plane information keeps the same format of `server-info` plugin output:
+
+| Name | Type | Description |
+|--------------|---------|------------------------------------------------------------------------------------------------------------------------|
+| boot_time | integer | Bootstrap time (UNIX timestamp) of the APISIX instance. Resets when hot updating but not when APISIX is just reloaded. |
+| id | string | APISIX instance ID. |
+| etcd_version | string | Version of the etcd cluster used by APISIX. Will be `unknown` if the network to etcd is partitioned. |
+| version | string | Version of APISIX instance. |
+| hostname | string | Hostname of the machine/pod APISIX is deployed to. |
+
+## Admin API to query data planes
+
+Refer to this link for detail:
+
+https://apisix.apache.org/en/docs/apisix/admin-api/#query-data-planes
Review Comment:
In most other cases, we use relative links like
https://github.com/apache/apisix/blob/7e292eadd36e738f84647634b0c351bc268ea529/docs/en/latest/terminology/plugin.md?plain=1#L35
https://github.com/apache/apisix/blob/7e292eadd36e738f84647634b0c351bc268ea529/docs/en/latest/terminology/route.md?plain=1#L82
@navendu-pottekkat
Are the relative links preferred nowadays?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1133508893
##########
apisix/plugin.lua:
##########
@@ -746,6 +746,10 @@ function _M.init_worker()
-- see https://github.com/apache/apisix/issues/3286
_M.load()
+ if is_http then
+ require("apisix.plugins.server-info").init_worker()
Review Comment:
@spacewander Split server-info.lua into server-info/common.lua and server-info/init_worker.lua, have a check please.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1131890899
##########
docs/en/latest/get-data-planes-from-etcd.md:
##########
@@ -0,0 +1,97 @@
+---
+id: get-data-planes-from-etcd
+title: Get data planes from etcd
+keywords:
+ - API gateway
+ - Apache APISIX
+description: Get data planes from etcd
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+To query all data planes information from etcd, we move `server-info` plugin into the APISIX core, and use it to report data plane information.
+
+The data plane information keeps the same format of `server-info` plugin output:
+
+https://apisix.apache.org/en/docs/apisix/plugins/server-info/
+
+## Admin API to query data planes
Review Comment:
We should move this into Admin API doc: https://github.com/apache/apisix/blob/master/docs/en/latest/admin-api.md
##########
apisix/plugin.lua:
##########
@@ -746,6 +746,10 @@ function _M.init_worker()
-- see https://github.com/apache/apisix/issues/3286
_M.load()
+ if is_http then
+ require("apisix.plugins.server-info").init_worker()
Review Comment:
`require("apisix.plugins.xxx")` will make the module import complex. We should use a separate file to hold the functons.
##########
docs/en/latest/get-data-planes-from-etcd.md:
##########
@@ -0,0 +1,97 @@
+---
+id: get-data-planes-from-etcd
+title: Get data planes from etcd
+keywords:
+ - API gateway
+ - Apache APISIX
+description: Get data planes from etcd
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+To query all data planes information from etcd, we move `server-info` plugin into the APISIX core, and use it to report data plane information.
+
+The data plane information keeps the same format of `server-info` plugin output:
+
+https://apisix.apache.org/en/docs/apisix/plugins/server-info/
Review Comment:
Better to move the relative content in server-info out to here. As that plugin is not so useful now.
##########
apisix/admin/init.lua:
##########
@@ -237,6 +237,40 @@ local function run()
end
+local function get_data_planes()
+ local key = "/data_plane/server_info"
+ local is_dir = true
+ local uri_segs = core.utils.split_uri(ngx.var.uri)
+ local id = uri_segs[5]
+ if id then
+ key = key .. "/" .. id
+ is_dir = false
+ end
+
+ local etcd_cli, prefix = core.etcd.get_etcd_cli()
+ key = prefix .. key
+ local res, err = etcd_cli:readdir(key)
+ if err then
+ core.log.warn("failed to do etcd readdir: ", err)
+ core.response.exit(502, {error_msg =
Review Comment:
Like https://github.com/apache/apisix/blob/09cc4946bc051452960975b4dd7436a2497df346/apisix/admin/resource.lua#L122, we usually return 503 for this case
##########
apisix/plugins/server-info.lua:
##########
@@ -307,8 +307,8 @@ function _M.init()
end
-function _M.destroy()
- timers.unregister_timer("plugin#server-info", true)
+function _M.init()
+ core.log.warn("server-info plugin was moved into APISIX core, no need to enable it explicitly")
Review Comment:
We should update server-info plugin's doc too.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo closed pull request #9012: change: move server-info plugin into core
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo closed pull request #9012: change: move server-info plugin into core
URL: https://github.com/apache/apisix/pull/9012
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1132056079
##########
apisix/plugin.lua:
##########
@@ -746,6 +746,10 @@ function _M.init_worker()
-- see https://github.com/apache/apisix/issues/3286
_M.load()
+ if is_http then
+ require("apisix.plugins.server-info").init_worker()
Review Comment:
Do you mean moving `server-info.lua` to elsewhere? But we should keep compatibility so that users could still use this plugin like before but with warning logs.
If only for `init_worker()` function, then because it's tightly coupled with symbols in `server-info.lua`, so it seems no clean way to separate it.
And I think it's not so necessary. `apisix/plugin.lua` is exactly used to load a specific plugin. In fact, in the same function, it also requires Prometheus plugin with a similar path:
https://github.com/apache/apisix/blob/09cc4946bc051452960975b4dd7436a2497df346/apisix/plugin.lua#L740
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1133356846
##########
apisix/plugin.lua:
##########
@@ -746,6 +746,10 @@ function _M.init_worker()
-- see https://github.com/apache/apisix/issues/3286
_M.load()
+ if is_http then
+ require("apisix.plugins.server-info").init_worker()
Review Comment:
Yes, but we don't require Prometheus plugin. The required module is "apisix.plugins.prometheus.exporter", not the Prometheus plugin itself. We need to do the same with server-info.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1134792279
##########
docs/en/latest/get-data-planes-from-etcd.md:
##########
@@ -0,0 +1,103 @@
+---
+id: get-data-planes-from-etcd
+title: Get data planes from etcd
+keywords:
+ - API gateway
+ - Apache APISIX
+description: Get data planes from etcd
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+To query all data planes information from etcd, we move `server-info` plugin into the APISIX core, and use it to report data plane information.
+
+The data plane information keeps the same format of `server-info` plugin output:
+
+| Name | Type | Description |
+|--------------|---------|------------------------------------------------------------------------------------------------------------------------|
+| boot_time | integer | Bootstrap time (UNIX timestamp) of the APISIX instance. Resets when hot updating but not when APISIX is just reloaded. |
+| id | string | APISIX instance ID. |
+| etcd_version | string | Version of the etcd cluster used by APISIX. Will be `unknown` if the network to etcd is partitioned. |
+| version | string | Version of APISIX instance. |
+| hostname | string | Hostname of the machine/pod APISIX is deployed to. |
+
+## Admin API to query data planes
+
+* `GET /apisix/admin/data_planes`
+
+It returns a JSON array which contains all alive data planes.
+
+```bash
+curl http://127.0.0.1:9180/apisix/admin/data_planes \
Review Comment:
We can simply drop a link to the admin-api.md here?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] navendu-pottekkat commented on a diff in pull request #9012: feat: move server-info plugin into core
Posted by "navendu-pottekkat (via GitHub)" <gi...@apache.org>.
navendu-pottekkat commented on code in PR #9012:
URL: https://github.com/apache/apisix/pull/9012#discussion_r1136455496
##########
docs/en/latest/get-data-planes-from-etcd.md:
##########
@@ -0,0 +1,45 @@
+---
+id: get-data-planes-from-etcd
+title: Get data planes from etcd
+keywords:
+ - API gateway
+ - Apache APISIX
+description: Get data planes from etcd
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+To query all data planes information from etcd, we move `server-info` plugin into the APISIX core, and use it to report data plane information.
+
+The data plane information keeps the same format of `server-info` plugin output:
+
+| Name | Type | Description |
+|--------------|---------|------------------------------------------------------------------------------------------------------------------------|
+| boot_time | integer | Bootstrap time (UNIX timestamp) of the APISIX instance. Resets when hot updating but not when APISIX is just reloaded. |
+| id | string | APISIX instance ID. |
+| etcd_version | string | Version of the etcd cluster used by APISIX. Will be `unknown` if the network to etcd is partitioned. |
+| version | string | Version of APISIX instance. |
+| hostname | string | Hostname of the machine/pod APISIX is deployed to. |
+
+## Admin API to query data planes
+
+Refer to this link for detail:
+
+https://apisix.apache.org/en/docs/apisix/admin-api/#query-data-planes
Review Comment:
@spacewander Yes. It would be okay as long as the relative path mentioned is in the same repo. This would mean that links work on GitHub as well as the built website.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org