On Sun, Oct 04, 2009 at 09:35:33PM +0200, Krzysztof Piotr Oledzki wrote:
> >From 1299a2fe5768c502786ef28cd78dae83a31f0c83 Mon Sep 17 00:00:00 2001
> From: Krzysztof Piotr Oledzki <o...@ans.pl>
> Date: Sun, 4 Oct 2009 21:28:53 +0200
> Subject: [MINOR] Capture & display more data from health checks
>
> Capture & display more data from health checks, like
> strerror(errno) for L4 failed checks or a first line
> from a response for L7 successes/failed checks.
I have a few questions about this one.
> Non ascii or HTML control characters are masked.
I don't see the reason for masking HTML characters, since the output
is sent over syslog. I'm OK for masking non-ascii characters however.
> Feature can be disabled by defining HCHK_DESC_LEN to 0.
What could be the reason for disabling this feature ? After
all, if people enable log-health-checks, it means they want
more verbose info.
> @@ -656,50 +679,78 @@ static int event_srv_chk_r(int fd)
> (memcmp(trash, "HTTP/1.", 7) != 0 ||
> (trash[12] != ' ' && trash[12] != '\r')) ||
> !isdigit(trash[9]) || !isdigit(trash[10]) ||
> !isdigit(trash[11])) {
> - set_server_check_status(s, HCHK_STATUS_L7RSP);
> +
> + desc = trash;
> + p = strchr(desc, '\r');
> + if (p)
> + *p = '\0';
Here we should also check for '\n' alone, let's add this function
to standard.h to find and replace CR/LFs with a zero. It's small
and efficient enough to be reused for generic uses :
static inline char *cut_crlf(char *s)
{
do {
if (*s == '\n' || *s == '\r')
*s = 0;
} while (*s++);
return s;
}
Then :
desc = trash;
cut_crlf(desc);
> +
> + set_server_check_status(s, HCHK_STATUS_L7RSP, desc);
> +
> goto out_wakeup;
> }
>
> s->check_code = str2uic(&trash[9]);
>
> + desc = &trash[12];
> + p = strchr(desc, '\r');
> + if (p)
> + *p = '\0';
same above
> + while(*desc == ' ')
> + desc++;
> +
(...)
> else if (s->proxy->options & PR_O_SMTP_CHK) {
> /* Check if the server speaks SMTP */
> if ((len < strlen("000\r")) ||
> (trash[3] != ' ' && trash[3] != '\r') ||
> !isdigit(trash[0]) || !isdigit(trash[1]) ||
> !isdigit(trash[2])) {
> - set_server_check_status(s, HCHK_STATUS_L7RSP);
> +
> + desc = trash;
> + p = strchr(desc, '\r');
> + if (p)
> + *p = '\0';
and here.
> +
> + set_server_check_status(s, HCHK_STATUS_L7RSP, desc);
> +
> goto out_wakeup;
> }
>
> s->check_code = str2uic(&trash[0]);
>
> + desc = &trash[3];
> + p = strchr(desc, '\r');
> + if (p)
> + *p = '\0';
and here.
I think that if the outputs are small enough, we'll be able to
make them appear as automatic "popups" in the HTML stats page
when the check column is highlighted.
Regards,
Willy
