Re: [PATCH] [MINOR] Capture & display more data from health checks

[Krzysztof Oledzki]

On Mon, 5 Oct 2009, Willy Tarreau wrote:

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.

Sure :)

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.

Because the output is presented on html page - it is added to a popup 
generated by "td title". But you are right - we should also add it to 
syslog.

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.

To save HCHK_DESC_LEN==128 bytes for each server. If you found it 
unnecessary I'm more than happy to remove this check.

@@ -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);

Sure. How about adding a "break" after a first match?

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.

Yep, this is exactly the place where you can find them currently. ;)

Best regards,

                        Krzysztof Olędzki