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/23 06:12:06 UTC
[GitHub] [apisix] kingluo opened a new pull request, #9151: feat: Upstream status report
kingluo opened a new pull request, #9151:
URL: https://github.com/apache/apisix/pull/9151
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
https://lists.apache.org/thread/2j12c5fxvqpxvhlccsq6gnnpk399kf4d
### 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
- [x] I have added tests corresponding to this change
- [x] 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] monkeyDluffy6017 merged pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #9151:
URL: https://github.com/apache/apisix/pull/9151
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150362623
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
The `_M` object is never exported, why define a `_M` 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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1151745584
##########
apisix/control/v1.lua:
##########
@@ -119,11 +201,19 @@ local function iter_and_find_healthcheck_info(values, src_type, src_id)
for _, value in core.config_util.iterate_values(values) do
if value.value.id == src_id then
- if not value.checker then
+ local checks = value.value.checks or
+ (value.value.upstream and value.value.upstream.checks)
+ if not checks then
return nil, str_format("no checker for %s[%s]", src_type, src_id)
end
- return extra_checker_info(value, src_type)
+ local info = extra_checker_info(value)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
Review Comment:
Can this duplicate code be extracted into a function?
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150367494
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
Review Comment:
Is `get_healthcheck_stats` more appropriate?
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150958154
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
But the return of the function is not the same, one is an object, the other is an html, the function of the same name is very disturbing
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150345562
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
Review Comment:
The `src_type` shoud be removed
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150371297
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
Review Comment:
The `src_type` is never used in this function
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1152696214
##########
docs/en/latest/control-api.md:
##########
@@ -92,118 +92,104 @@ Returns the JSON schema used by the APISIX instance:
### GET /v1/healthcheck
-Introduced in [v2.3](https://github.com/apache/apisix/releases/tag/2.3).
-
Returns a [health check](./tutorials/health-check.md) of the APISIX instance.
```json
[
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
- },
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/routes/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.1",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "routes"
- }
+ {
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1",
+ "type": "http"
+ }
]
+
```
Each of the returned objects contain the following fields:
-* src_type: where the health checker is reporting from. Value is one of `["routes", "services", "upstreams"]`.
-* src_id: id of the object creating the health checker. For example, if an Upstream
-object with id `1` creates a health checker, the `src_type` is `upstreams` and the `src_id` is `1`.
-* name: name of the health checker.
+* name: resource id, where the health checker is reporting from.
+* type: health check type.
* nodes: target nodes of the health checker.
-* healthy_nodes: healthy nodes discovered by the health checker.
+* nodes[i].ip: ip address.
+* nodes[i].port: port number.
+* nodes[i].status: health check result: `["healthy", "unhealthy", "mostly_healthy", "mostly_unhealthy"]`.
+* nodes[i].counter.success: success health check count.
+* nodes[i].counter.http_failure: http failures count.
+* nodes[i].counter.tcp_failure: tcp connect/read/write failures count.
+* nodes[i].counter.timeout_failure: timeout count.
You can also use `/v1/healthcheck/$src_type/$src_id` to get the health status of specific nodes.
For example, `GET /v1/healthcheck/upstreams/1` returns:
```json
{
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 2,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
Review Comment:
Done.
--
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] leslie-tsang commented on a diff in pull request #9151: feat: Upstream status report
Posted by "leslie-tsang (via GitHub)" <gi...@apache.org>.
leslie-tsang commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1149950563
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
Review Comment:
```suggestion
local healthcheck = require("resty.healthcheck")
```
Would be better ? Because it would have been initialized anyway. Seems no need to check it in `extra_checker_info`
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -458,6 +463,15 @@ local function collect(ctx, stream_only)
metrics.node_info:set(1, gen_arr(hostname))
+ -- update upstream_status metrics
+ local stats = control.get_health_checkers()
+ for _, stat in ipairs(stats) do
+ for _, node in ipairs(stat.nodes) do
+ metrics.upstream_status:set((node.status == "healthy") and 1 or 0,
+ gen_arr(stat.name, node.ip, node.port))
Review Comment:
```suggestion
metrics.upstream_status:set((node.status == "healthy") and 1 or 0,
gen_arr(stat.name, node.ip, node.port))
```
code style.
--
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 pull request #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on PR #9151:
URL: https://github.com/apache/apisix/pull/9151#issuecomment-1487368944
it's new implementation and definition of the api, so introduced from 2.3 will confuse the users, after all, we have no plan to backport the changes. so all in all, it's better to omit this note.
| |
***@***.***
|
|
***@***.***
|
---- Replied Message ----
| From | Liu ***@***.***> |
| Date | 03/28/2023 23:33 |
| To | ***@***.***> |
| Cc | jinhua ***@***.***>***@***.***> |
| Subject | Re: [apache/apisix] feat: Upstream status report (PR #9151) |
@monkeyDluffy6017 commented on this pull request.
In docs/en/latest/control-api.md:
> @@ -92,118 +92,104 @@ Returns the JSON schema used by the APISIX instance:
### GET /v1/healthcheck
-Introduced in [v2.3](https://github.com/apache/apisix/releases/tag/2.3).
Why do you delete this?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150936021
##########
apisix/control/v1.lua:
##########
@@ -155,6 +234,13 @@ function _M.get_health_checker()
if not info then
return 404, {error_msg = err}
end
+
+ local out = try_render_html({stats={info}})
Review Comment:
The return of `iter_and_find_healthcheck_info` is not correct.
The following code should be added
```
if checks.active and checks.active.type then
info.type = checks.active.type
elseif checks.passive and checks.passive.type then
info.type = checks.passive.type
end
```
--
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] monkeyDluffy6017 commented on pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9151:
URL: https://github.com/apache/apisix/pull/9151#issuecomment-1487320618
What about the chinese doc?
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150788319
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error("try_render_html failed: ", err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
+ local infos = get_health_checkers()
+ local out = try_render_html({stats=infos})
+ if out then
Review Comment:
The judement is not correct, the `out` returned by `try_render_html` will never be `nil` or `false`
--
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] monkeyDluffy6017 commented on pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9151:
URL: https://github.com/apache/apisix/pull/9151#issuecomment-1488368050
@moonming @membphis Do you have any further comments on this PR?
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150139656
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
Review Comment:
@leslie-tsang No, it's necessary. Because Prometheus exporter requires this file also in the stream subsystem in init phase, and require on the top level will cause some errors. This function is used only when it needs to get health check 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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150332056
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
- local checker = value.checker
- local upstream = value.checker_upstream
- local host = upstream.checks and upstream.checks.active and upstream.checks.active.host
- local port = upstream.checks and upstream.checks.active and upstream.checks.active.port
- local nodes = upstream.nodes
- local healthy_nodes = core.table.new(#nodes, 0)
- for _, node in ipairs(nodes) do
- local ok = checker:get_target_status(node.host, port or node.port, host)
- if ok then
- core.table.insert(healthy_nodes, node)
- end
+ if not healthcheck then
+ healthcheck = require("resty.healthcheck")
end
- local conf = value.value
+ local name = upstream_mod.get_healthchecker_name(value)
+ local nodes, err = healthcheck.get_target_list(name, "upstream-healthcheck")
+ if err then
+ core.log.error(err)
+ end
return {
- name = upstream_mod.get_healthchecker_name(value),
- src_id = conf.id,
- src_type = src_type,
Review Comment:
why do you delete the src_type and conf.id?
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150385173
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
It's used both for Prometheus and control API. So it's better to unify the name.
```lua
-- /v1/plugin_metadata/*
{
methods = {"GET"},
uris = {"/plugin_metadata/*"},
handler = _M.dump_plugin_metadata,
},
get_health_checkers = get_health_checkers,
```
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150958154
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
But the return of the function is not the same, one is an object, the other is an html, the function of the same name is confusing
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1151533094
##########
docs/en/latest/control-api.md:
##########
@@ -92,118 +92,104 @@ Returns the JSON schema used by the APISIX instance:
### GET /v1/healthcheck
-Introduced in [v2.3](https://github.com/apache/apisix/releases/tag/2.3).
-
Returns a [health check](./tutorials/health-check.md) of the APISIX instance.
```json
[
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
- },
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/routes/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.1",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "routes"
- }
+ {
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1",
+ "type": "http"
+ }
]
+
```
Each of the returned objects contain the following fields:
-* src_type: where the health checker is reporting from. Value is one of `["routes", "services", "upstreams"]`.
-* src_id: id of the object creating the health checker. For example, if an Upstream
-object with id `1` creates a health checker, the `src_type` is `upstreams` and the `src_id` is `1`.
-* name: name of the health checker.
+* name: resource id, where the health checker is reporting from.
+* type: health check type.
* nodes: target nodes of the health checker.
-* healthy_nodes: healthy nodes discovered by the health checker.
+* nodes[i].ip: ip address.
+* nodes[i].port: port number.
+* nodes[i].status: health check result: `["healthy", "unhealthy", "mostly_healthy", "mostly_unhealthy"]`.
+* nodes[i].counter.success: success health check count.
+* nodes[i].counter.http_failure: http failures count.
+* nodes[i].counter.tcp_failure: tcp connect/read/write failures count.
+* nodes[i].counter.timeout_failure: timeout count.
You can also use `/v1/healthcheck/$src_type/$src_id` to get the health status of specific nodes.
For example, `GET /v1/healthcheck/upstreams/1` returns:
```json
{
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 2,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
Review Comment:
> the name and the description implies the types, as well as the given example. for example, counter is always integer, ip is always string. so you can see that the original doc also ignores the type definition.
`type` is like this: "type": "http", it is never used before, but you import the property in `GET /v1/healthcheck`,
Don't we import this `type` to `GET /v1/healthcheck/upstreams/1`
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150139656
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
Review Comment:
@leslie-tsang No, it's necessary. Because Prometheus exporter requires this file also in the stream subsystem, and require on the top level will cause some errors. This function is used only when it needs to get health check 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] kingluo commented on a diff in pull request #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150139656
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
Review Comment:
@leslie-tsang No, it's necessary. Because Prometheus exporter requires this file also in the init phase of the stream subsystem, and require on the top level will cause some errors. This function is used only when it needs to get health check 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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150332056
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
- local checker = value.checker
- local upstream = value.checker_upstream
- local host = upstream.checks and upstream.checks.active and upstream.checks.active.host
- local port = upstream.checks and upstream.checks.active and upstream.checks.active.port
- local nodes = upstream.nodes
- local healthy_nodes = core.table.new(#nodes, 0)
- for _, node in ipairs(nodes) do
- local ok = checker:get_target_status(node.host, port or node.port, host)
- if ok then
- core.table.insert(healthy_nodes, node)
- end
+ if not healthcheck then
+ healthcheck = require("resty.healthcheck")
end
- local conf = value.value
+ local name = upstream_mod.get_healthchecker_name(value)
+ local nodes, err = healthcheck.get_target_list(name, "upstream-healthcheck")
+ if err then
+ core.log.error(err)
+ end
return {
- name = upstream_mod.get_healthchecker_name(value),
- src_id = conf.id,
- src_type = src_type,
Review Comment:
why do you delete the src_type and src_id?
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150791537
##########
apisix/control/v1.lua:
##########
@@ -155,6 +234,13 @@ function _M.get_health_checker()
if not info then
return 404, {error_msg = err}
end
+
+ local out = try_render_html({stats={info}})
+ if out then
Review Comment:
The judement is incorrect 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 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150142218
##########
apisix/plugins/prometheus/exporter.lua:
##########
@@ -458,6 +463,15 @@ local function collect(ctx, stream_only)
metrics.node_info:set(1, gen_arr(hostname))
+ -- update upstream_status metrics
+ local stats = control.get_health_checkers()
+ for _, stat in ipairs(stats) do
+ for _, node in ipairs(stat.nodes) do
+ metrics.upstream_status:set((node.status == "healthy") and 1 or 0,
+ gen_arr(stat.name, node.ip, node.port))
Review Comment:
@leslie-tsang No, it's not a code style issue, on the contrary, it needs to keep the style compatible with other metrics update indent in the same file, e.g.
```lua
metrics.latency:observe(latency,
gen_arr("request", route_id, service_id, consumer_name, balancer_ip,
unpack(latency_extra_label_values)))
```
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150318874
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
- local checker = value.checker
- local upstream = value.checker_upstream
- local host = upstream.checks and upstream.checks.active and upstream.checks.active.host
- local port = upstream.checks and upstream.checks.active and upstream.checks.active.port
- local nodes = upstream.nodes
- local healthy_nodes = core.table.new(#nodes, 0)
- for _, node in ipairs(nodes) do
- local ok = checker:get_target_status(node.host, port or node.port, host)
- if ok then
- core.table.insert(healthy_nodes, node)
- end
+ if not healthcheck then
+ healthcheck = require("resty.healthcheck")
end
- local conf = value.value
+ local name = upstream_mod.get_healthchecker_name(value)
+ local nodes, err = healthcheck.get_target_list(name, "upstream-healthcheck")
+ if err then
+ core.log.error(err)
Review Comment:
Can the error log be more detailed?
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150369253
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
The prototype comes from the original code, which is irrelevant to this PR. So no need to change it:
```lua
-- /v1/schema
{
methods = {"GET"},
uris = {"/schema"},
handler = _M.schema,
},
-- /v1/healthcheck
{
methods = {"GET"},
uris = {"/healthcheck"},
handler = _M.get_health_checkers,
},
-- /v1/healthcheck/{src_type}/{src_id}
{
methods = {"GET"},
uris = {"/healthcheck/*"},
handler = _M.get_health_checker,
},
```
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150359899
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
Review Comment:
No, it's used to retrieve info by resource type. The format of the results for each resource type from the health check is unified and contains the resource type and resource id. But here, the `src_type` argument is necessary because the resource types are independent and you need to retrieve them separately.
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150810135
##########
docs/en/latest/control-api.md:
##########
@@ -92,118 +92,104 @@ Returns the JSON schema used by the APISIX instance:
### GET /v1/healthcheck
-Introduced in [v2.3](https://github.com/apache/apisix/releases/tag/2.3).
Review Comment:
Why do you delete this?
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1152696122
##########
docs/zh/latest/control-api.md:
##########
@@ -96,111 +96,99 @@ APISIX 中一些插件添加了自己的 control API。如果你对他们感兴
```json
[
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
- },
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/routes/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.1",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "routes"
- }
+ {
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1",
+ "type": "http"
+ }
]
+
```
每个 entry 包含以下字段：
-* src_type：表示 health checker 的来源。值是 `[routes,services,upstreams]` 其中之一
-* src_id：表示创建 health checker 的对象的 id。例如，假设 id 为 1 的 Upstream 对象创建了一个 health checker，那么 `src_type` 就是 `upstreams`，`src_id` 就是 1
-* name：表示 health checker 的名称
-* nodes：health checker 的目标节点
-* healthy_nodes：表示 health checker 检测到的健康节点
+* name: 资源 ID，健康检查的报告对象。
+* type: 健康检查类型，取值为 `["http", "https", "tcp"]`。
+* nodes: 检查节点列表。
+* nodes[i].ip: IP 地址。
+* nodes[i].port: 端口。
+* nodes[i].status: 状态：`["healthy", "unhealthy", "mostly_healthy", "mostly_unhealthy"]`。
+* nodes[i].counter.success: 成功计数器。
+* nodes[i].counter.http_failure: HTTP 访问失败计数器。
+* nodes[i].counter.tcp_failure: TCP 连接或读写的失败计数器。
+* nodes[i].counter.timeout_failure: 超时计数器。
用户也可以通过 `/v1/healthcheck/$src_type/$src_id` 来获取指定 health checker 的状态。
例如，`GET /v1/healthcheck/upstreams/1` 返回：
```json
{
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 2,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1"
Review Comment:
Done.
##########
apisix/control/v1.lua:
##########
@@ -119,11 +201,19 @@ local function iter_and_find_healthcheck_info(values, src_type, src_id)
for _, value in core.config_util.iterate_values(values) do
if value.value.id == src_id then
- if not value.checker then
+ local checks = value.value.checks or
+ (value.value.upstream and value.value.upstream.checks)
+ if not checks then
return nil, str_format("no checker for %s[%s]", src_type, src_id)
end
- return extra_checker_info(value, src_type)
+ local info = extra_checker_info(value)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
Review Comment:
Done.
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150362623
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
The `_M` object is never used, why define a `_M` 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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150367494
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
Review Comment:
The function name `get_health_checkers` is ambiguous, is `get_healthcheck_stats` more appropriate?
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150938363
##########
docs/en/latest/control-api.md:
##########
@@ -92,118 +92,104 @@ Returns the JSON schema used by the APISIX instance:
### GET /v1/healthcheck
-Introduced in [v2.3](https://github.com/apache/apisix/releases/tag/2.3).
-
Returns a [health check](./tutorials/health-check.md) of the APISIX instance.
```json
[
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
- },
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/routes/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.1",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "routes"
- }
+ {
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1",
+ "type": "http"
+ }
]
+
```
Each of the returned objects contain the following fields:
-* src_type: where the health checker is reporting from. Value is one of `["routes", "services", "upstreams"]`.
-* src_id: id of the object creating the health checker. For example, if an Upstream
-object with id `1` creates a health checker, the `src_type` is `upstreams` and the `src_id` is `1`.
-* name: name of the health checker.
+* name: resource id, where the health checker is reporting from.
+* type: health check type.
* nodes: target nodes of the health checker.
-* healthy_nodes: healthy nodes discovered by the health checker.
+* nodes[i].ip: ip address.
+* nodes[i].port: port number.
+* nodes[i].status: health check result: `["healthy", "unhealthy", "mostly_healthy", "mostly_unhealthy"]`.
+* nodes[i].counter.success: success health check count.
+* nodes[i].counter.http_failure: http failures count.
+* nodes[i].counter.tcp_failure: tcp connect/read/write failures count.
+* nodes[i].counter.timeout_failure: timeout count.
You can also use `/v1/healthcheck/$src_type/$src_id` to get the health status of specific nodes.
For example, `GET /v1/healthcheck/upstreams/1` returns:
```json
{
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 2,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
Review Comment:
`type` is needed
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150954000
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
Review Comment:
Do we have a test case to test the rendered html
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150777624
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error("try_render_html failed: ", err)
Review Comment:
The error log here is: try_render_html failed: HTML template rendering: xxxxx.
The prefix is duplicated.
`core.log.error("try_render_html failed: ", out)`
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150378935
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
The function name `get_health_checkers` is the same with the one above, it's better to be different
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150385173
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
Review Comment:
It's used both for Prometheus and control API. So it's better to unify the name.
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150338093
##########
apisix/control/v1.lua:
##########
@@ -62,27 +64,20 @@ function _M.schema()
end
+local healthcheck
local function extra_checker_info(value, src_type)
- local checker = value.checker
- local upstream = value.checker_upstream
- local host = upstream.checks and upstream.checks.active and upstream.checks.active.host
- local port = upstream.checks and upstream.checks.active and upstream.checks.active.port
- local nodes = upstream.nodes
- local healthy_nodes = core.table.new(#nodes, 0)
- for _, node in ipairs(nodes) do
- local ok = checker:get_target_status(node.host, port or node.port, host)
- if ok then
- core.table.insert(healthy_nodes, node)
- end
+ if not healthcheck then
+ healthcheck = require("resty.healthcheck")
end
- local conf = value.value
+ local name = upstream_mod.get_healthchecker_name(value)
+ local nodes, err = healthcheck.get_target_list(name, "upstream-healthcheck")
+ if err then
+ core.log.error(err)
+ end
return {
- name = upstream_mod.get_healthchecker_name(value),
- src_id = conf.id,
- src_type = src_type,
Review Comment:
@monkeyDluffy6017 The name already denotes everything, so it's time to remove unnecessary fields by the way.
--
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 pull request #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on PR #9151:
URL: https://github.com/apache/apisix/pull/9151#issuecomment-1487361409
the name and the description implies the types, as well as the given example. for example, counter is always integer, ip is always string. so you can see that the original doc also ignores the type definition.
| |
***@***.***
|
|
***@***.***
|
---- Replied Message ----
| From | Liu ***@***.***> |
| Date | 03/29/2023 01:18 |
| To | ***@***.***> |
| Cc | jinhua ***@***.***>***@***.***> |
| Subject | Re: [apache/apisix] feat: Upstream status report (PR #9151) |
@monkeyDluffy6017 commented on this pull request.
In docs/en/latest/control-api.md:
> + },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
type is needed
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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 #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150369950
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
Review Comment:
I thnk it's better to reuse original name.
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
+ if not html_render then
+ local template = require("resty.template")
+ html_render = template.compile(HTML_TEMPLATE)
+ end
+ local accept = ngx_var.http_accept
+ if accept and accept:find("text/html") then
+ local ok, out = pcall(html_render, data)
+ if not ok then
+ local err = str_format("HTML template rendering: %s", out)
+ core.log.error(err)
+ return 503, err
+ end
+ return out
+ end
+end
+
+
+local function get_health_checkers()
Review Comment:
I think it's better to reuse the original name.
--
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 pull request #9151: feat: Upstream status report
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on PR #9151:
URL: https://github.com/apache/apisix/pull/9151#issuecomment-1487354786
it's not only for html. it has condition. they did the same things, just for different modules. anyway, naming is subjective, no perfect naming rule for all people. of course, if you insist, i will change it.
| |
***@***.***
|
|
***@***.***
|
---- Replied Message ----
| From | Liu ***@***.***> |
| Date | 03/29/2023 01:37 |
| To | ***@***.***> |
| Cc | jinhua ***@***.***>***@***.***> |
| Subject | Re: [apache/apisix] feat: Upstream status report (PR #9151) |
@monkeyDluffy6017 commented on this pull request.
In apisix/control/v1.lua:
> local infos = {}
local routes = get_routes()
iter_and_add_healthcheck_info(infos, routes, "routes")
local services = get_services()
iter_and_add_healthcheck_info(infos, services, "services")
local upstreams = get_upstreams()
iter_and_add_healthcheck_info(infos, upstreams, "upstreams")
+ return infos
+end
+
+
+function _M.get_health_checkers()
But the return of the function is not the same, one is an object, the other is an html, the function of the same name is very disturbing
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1150954000
##########
apisix/control/v1.lua:
##########
@@ -93,21 +88,105 @@ local function iter_and_add_healthcheck_info(infos, values, src_type)
end
for _, value in core.config_util.iterate_values(values) do
- if value.checker then
- core.table.insert(infos, extra_checker_info(value, src_type))
+ local checks = value.value.checks or (value.value.upstream and value.value.upstream.checks)
+ if checks then
+ local info = extra_checker_info(value, src_type)
+ if checks.active and checks.active.type then
+ info.type = checks.active.type
+ elseif checks.passive and checks.passive.type then
+ info.type = checks.passive.type
+ end
+ core.table.insert(infos, info)
end
end
end
-function _M.get_health_checkers()
+local HTML_TEMPLATE = [[
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+ <title>APISIX upstream check status</title>
+</head>
+<body>
+<h1>APISIX upstream check status</h1>
+<table style="background-color:white" cellspacing="0" cellpadding="3" border="1">
+ <tr bgcolor="#C0C0C0">
+ <th>Index</th>
+ <th>Upstream</th>
+ <th>Check type</th>
+ <th>Host</th>
+ <th>Status</th>
+ <th>Success counts</th>
+ <th>TCP Failures</th>
+ <th>HTTP Failures</th>
+ <th>TIMEOUT Failures</th>
+ </tr>
+{% local i = 0 %}
+{% for _, stat in ipairs(stats) do %}
+{% for _, node in ipairs(stat.nodes) do %}
+{% i = i + 1 %}
+ {% if node.status == "healthy" then %}
+ <tr>
+ {% else %}
+ <tr bgcolor="#FF0000">
+ {% end %}
+ <td>{* i *}</td>
+ <td>{* stat.name *}</td>
+ <td>{* stat.type *}</td>
+ <td>{* node.ip .. ":" .. node.port *}</td>
+ <td>{* node.status *}</td>
+ <td>{* node.counter.success *}</td>
+ <td>{* node.counter.tcp_failure *}</td>
+ <td>{* node.counter.http_failure *}</td>
+ <td>{* node.counter.timeout_failure *}</td>
+ </tr>
+{% end %}
+{% end %}
+</table>
+</body>
+</html>
+]]
+
+local html_render
+
+local function try_render_html(data)
Review Comment:
Do we have a test case to test the rendered html？
--
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] monkeyDluffy6017 commented on pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9151:
URL: https://github.com/apache/apisix/pull/9151#issuecomment-1487805828
Could you rely in the channel? Thanks @kingluo
--
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] monkeyDluffy6017 commented on a diff in pull request #9151: feat: Upstream status report
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9151:
URL: https://github.com/apache/apisix/pull/9151#discussion_r1152693425
##########
docs/zh/latest/control-api.md:
##########
@@ -96,111 +96,99 @@ APISIX 中一些插件添加了自己的 control API。如果你对他们感兴
```json
[
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
- },
- {
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/routes/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.1",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "routes"
- }
+ {
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1",
+ "type": "http"
+ }
]
+
```
每个 entry 包含以下字段：
-* src_type：表示 health checker 的来源。值是 `[routes,services,upstreams]` 其中之一
-* src_id：表示创建 health checker 的对象的 id。例如，假设 id 为 1 的 Upstream 对象创建了一个 health checker，那么 `src_type` 就是 `upstreams`，`src_id` 就是 1
-* name：表示 health checker 的名称
-* nodes：health checker 的目标节点
-* healthy_nodes：表示 health checker 检测到的健康节点
+* name: 资源 ID，健康检查的报告对象。
+* type: 健康检查类型，取值为 `["http", "https", "tcp"]`。
+* nodes: 检查节点列表。
+* nodes[i].ip: IP 地址。
+* nodes[i].port: 端口。
+* nodes[i].status: 状态：`["healthy", "unhealthy", "mostly_healthy", "mostly_unhealthy"]`。
+* nodes[i].counter.success: 成功计数器。
+* nodes[i].counter.http_failure: HTTP 访问失败计数器。
+* nodes[i].counter.tcp_failure: TCP 连接或读写的失败计数器。
+* nodes[i].counter.timeout_failure: 超时计数器。
用户也可以通过 `/v1/healthcheck/$src_type/$src_id` 来获取指定 health checker 的状态。
例如，`GET /v1/healthcheck/upstreams/1` 返回：
```json
{
- "healthy_nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- }
- ],
- "name": "upstream#/upstreams/1",
- "nodes": [
- {
- "host": "127.0.0.1",
- "port": 1980,
- "priority": 0,
- "weight": 1
- },
- {
- "host": "127.0.0.2",
- "port": 1988,
- "priority": 0,
- "weight": 1
- }
- ],
- "src_id": "1",
- "src_type": "upstreams"
+ "nodes": [
+ {
+ "ip": "52.86.68.46",
+ "counter": {
+ "http_failure": 0,
+ "success": 2,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "healthy"
+ },
+ {
+ "ip": "100.24.156.8",
+ "counter": {
+ "http_failure": 5,
+ "success": 0,
+ "timeout_failure": 0,
+ "tcp_failure": 0
+ },
+ "port": 80,
+ "status": "unhealthy"
+ }
+ ],
+ "name": "/apisix/routes/1"
Review Comment:
`type` is missing
--
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