Le 30/01/2021 à 16:21, William Dauchy a écrit :
Hi Christopher,
Sorry for this long series but I believe this one is fairly easy to
review:
- the major change is for health checks on prometheus side, there are
more details in the commit message; this is something I wanted early
so people can start testing it, especially on large setup. I however
think we will need to improve the `scope` filtering. The good point is
that after this patch, this metric will be way easier to use; the bad
point is that it is a breaking change between v2.3 and v2.4. But I
believe this is acceptable following the previous state changes
already merged.
- next patch is the continuation of the cleaning work:
* try to improve descriptions so we can use them in both case
* merge them when possible, override otherwise
- I finished with a bit of minor cleaning which pointed me a few missing
fields. I know we probably can improve that to avoid forgetting adding
those fields in the future, but I assume it is ok for now to simply
come back with a sane result. The postive point is I did not had to
add description and implementation, as we make use of stats.c
Hi Willian,
Don't be sorry to send patches ! If you think the number of labels for the
check_status metric is not a problem, even for huge configurations, I trust you.
And I guess we can reduce this number to 16 status only, removing all status
with an unknown check result (HCHK_STATUS_UNKNOWN, HCHK_STATUS_INI,
HCHK_STATUS_START, HCHK_STATUS_L57DATA) and the status regarding the agent-check
only (HCHK_STATUS_CHECKED).
For info, HCHK_STATUS_UNKNOWN is only used for uninitialized health-check
(during configuration parsing). HCHK_STATUS_INI is used for initialized
health-check and before the first run. HCHK_STATUS_START and HCHK_STATUS_L57DATA
are dummy status and never assigned to a health-check. And as said,
HCHK_STATUS_CHECKED is only used for agent-check.
It is a slight improvement, but still better than nothing. To do so, I propose
you to add a function in check.c to get the corresponding check result of a
status (the attached patch). This way, in your first patch, we can filter on
check result. I may amend your first patch this way if you're ok:
--- a/contrib/prometheus-exporter/service-prometheus.c
+++ b/contrib/prometheus-exporter/service-prometheus.c
@@ -880,6 +880,8 @@ static int promex_dump_srv_metrics(struct appctx *appctx,
struct htx *htx)
goto next_sv;
for (i = 0; i < HCHK_STATUS_SIZE; i++) {
+ if
(get_check_status_result(i) < CHK_RES_FAILED)
+ continue;
val = mkf_u32(FO_STATUS,
sv->check.status == i);
check_state =
get_check_status_info(i);
labels[2].name =
ist("state");
No comments about the other patches. Except the README is now outdated and
does not reflect recent changes. It could be good to keep it up-to-date as far
as possible.
--
Christopher Faulet
>From 9eb33b4ed7bd2017422dff1c4076c105a23a1e2d Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Mon, 1 Feb 2021 11:00:49 +0100
Subject: [PATCH] MINOR: checks: Add function to get the result code
corresponding to a status
The function get_check_status_result() can now be used to get the result
code (CHK_RES_*) corresponding to a check status (HCHK_STATUS_*). It will be
used by the Prometheus exporter when reporting the check status of a server.
---
include/haproxy/check.h | 1 +
src/check.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/haproxy/check.h b/include/haproxy/check.h
index 2e697ee78..8d70040a3 100644
--- a/include/haproxy/check.h
+++ b/include/haproxy/check.h
@@ -29,6 +29,7 @@
extern struct data_cb check_conn_cb;
extern struct proxy checks_fe;
+short get_check_status_result(short check_status);
const char *get_check_status_description(short check_status);
const char *get_check_status_info(short check_status);
int httpchk_build_status_header(struct server *s, struct buffer *buf);
diff --git a/src/check.c b/src/check.c
index 0528372d2..879fe84ce 100644
--- a/src/check.c
+++ b/src/check.c
@@ -152,6 +152,15 @@ static inline int unclean_errno(int err)
return err;
}
+/* Converts check_status code to result code */
+short get_check_status_result(short check_status)
+{
+ if (check_status < HCHK_STATUS_SIZE)
+ return check_statuses[check_status].result;
+ else
+ return check_statuses[HCHK_STATUS_UNKNOWN].result;
+}
+
/* Converts check_status code to description */
const char *get_check_status_description(short check_status) {
--
2.29.2
