On Mon, Feb 01, 2021 at 11:30:18AM +0100, Christopher Faulet wrote:
> 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:
definitely worth it indeed. I let you do the amend
> --- 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.
ah true, I will send an update about it soon.
--
William
