Re: Getting JSON encoded data from the stats socket.

[ge...@riseup.net]

Hi,

On 16-07-26 21:47:55, Willy Tarreau wrote:
> I'd like to wait for other people to have the time to participate to
> this discussion, I know that some people are very careful about the
> relevance and accuracy of the stats, some people may want to report
> other suggestions.

I can't add that much, and have no specific suggestions, so just this:

(I'm a long time user of HAProxy, my setups aren't that big, mostly
around 50 backends, but I absolutely love the software. Thanks for this
great work!)

Regarding the topic: I absolutely support the proposal to dump the stats
into json. In my opinion, this is a much more easily parseable (and
modern) format, instead of csv. I think that grouping by process makes
sense, but to include "overall stats" as well. Additionally, I support
your view Willy, about the amount of the data to dump: I would speak in
favor of "dumping as much as possible", because, not sure if I got this
right, it's already possible to do so, it just needs support to dump to
json. Better safe then sorry, let's include all the data which _might_
be of interest, instead of data which _is now_ of interest. If some
"useless" (for now) data would be dumped...so what?

Thanks for the proposal Pavlos!

Cheers,
Georg


signature.asc
Description: Digital signature

Re: Getting JSON encoded data from the stats socket.

[Willy Tarreau]

On Tue, Jul 26, 2016 at 09:06:05PM +0200, Pavlos Parissis wrote:
> > You probably have not looked at the output of "show stats typed", it
> > gives you the nature of each value letting you know how to aggregate
> > them (min, max, avg, sum, pick any, etc).
> > 
> 
> I have seen it but it isn't available on 1.6. It could simplify my code, I
> should give a try.

Ah indeed you're right. Well it's not in 1.6 mainline but we backported
it to hapee-1.6 in case that's relevant to the machines you're interested
in.

> >> The stats are already aggregated and few metrics are excluded. For example 
> >> all status stuff.
> >> Each process performs healthchecking, so they act as little brains which 
> >> never agree on the
> >> status of a server as they run their checks on different interval.
> > 
> > Absolutely, but at least you want to see their stats. For example how many
> > times a server has switched state per process then in total (meaning a
> > proportional amount of possibly visible issues).
> > 
> 
> True, but in setups with ECMP in front of N HAProxy nodes which run in nbproc 
> mode you offload
> application healthchecking to a dedicated daemon which runs on servers(service
> discovery+service availability with consul/zookeeper stuff) and you only run 
> TCP checks
> from HAProxy.
> 
> In our setup we don't real care about how many times a server flapped, it 
> doesn't tell us
> something we don't know already, application is in broken state.

In such a case I agree.

> But, other people may find it useful.

Anyway that was just an example, what I meant by this is that we must
take care not to selectively pick some elements and not other ones. I
prefer that the output contains 10% of useless stuff and that we never
have anything special to do for the upcoming stuff to automatically
appear than to have to explicitly add new stuff all the time! When
you see the size of the csv dump function right now, it's a joke
and I really expect the JSON dump to follow the same philosophy.

> > My issue is that if the *format* doesn't support per-process stats, we'll 
> > have
> > to emit a new format 3 months later for all the people who want to process 
> > it.
> > We've reworked the stats dump to put an end to the problem where depending 
> > on
> > the output format you used to have different types of information, and there
> > was no single representation carrying them all at once. For me now it's
> > essential that if we prepare a new format it's not stripped down from the
> > info people need, otherwise it will automatically engender yet another 
> > format.
> > 
> 
> Agree. I am fine giving per process stats for servers/frontends/backends.
> Adding another top level key 'per_process' in my proposal should be a good 
> start:
> 
> {
> "per_process": {
> "proc1": {
> "frontend": {
> "www.haproxy.org": {
> "bin": "",
> "lbtot": "55",
> ...
(...)

Yes, I think so and that's also more or less similar to what Mark proposed.
Also I'm not much worried by the extra output size, if we dump this through
HTTP we'll have it gzipped.

Also, we want to have the values typed otherwise you're fucked as we used
to be with the CSV dump in the past. The current code supports this and
that's important. I don't know how it may impact the JSON output. Maybe
some parts will be just "numbers", but I remember that certain of them
have some properties (eg: max, limit, age, percentage, PID, SNMP ID, etc).
I'm less worried about the strings, we basically have identifiers,
descriptions and outputs from what I remember. But taking a look at this
will help refine the format.

I'd like to wait for other people to have the time to participate to this
discussion, I know that some people are very careful about the relevance
and accuracy of the stats, some people may want to report other suggestions.

Cheers,
Willy

Re: Getting JSON encoded data from the stats socket.

[Pavlos Parissis]

On 26/07/2016 06:56 μμ, Willy Tarreau wrote:
> On Tue, Jul 26, 2016 at 05:51:08PM +0200, Pavlos Parissis wrote:
>> In all my setups I have nbproc > 1 and after a lot of changes and on how I 
>> aggregate HAProxy
>> stats and what most people want to see on graphs, I came up that with 
>> something like the following:
>>
>> {
>> "frontend": {
>> "www.haproxy.org": {
>> "bin": "",
>> "lbtot": "55",
>> ...
>> },
>> "www.haproxy.com": {
>> "bin": "",
>> "lbtot": "55",
>> ...
>> },
>> },
>> "backend": {
>> "www.haproxy.org": {
>> "bin": "",
>> "lbtot": "55",
>> 
>> "server": {
>> "srv1": {
>> "bin": "",
>> "lbtot": "55",
>>
>> },
>> ...
>> },
>> },
>> },
>> "haproxy": {
>> "PipesFree": "555",
>> ...
>> ,
>> "per_process": {
>> "id1": {
>> "PipesFree": "555",
>> "Process_num": "1",
>> ...
>> },
>> "id2": {
>> "PipesFree": "555",
>> "Process_num": "2",
>> ...
>> },
>> ...
>> },
>> },
>> "server": {
>> "srv1": {
>> "bin": "",
>> "lbtot": "55",
>> ...
>> },
>> ...
>> },
>> }
>>
>>
>> Let me explain a bit:
>>
>> - It is very useful and handy to know stats for a server per backend but 
>> also across all
>> backends. Thus, I include a top level key 'server' which holds stats for 
>> each server across all
>> backends. Few server's stats has to be excluded as they are meaningless in 
>> this context.
>> For example, status, lastchg, check_duration, check_code and few others. For 
>> those which aren't
>> counters but fixed numbers you want to either sum them(slim) or get the 
>> average(weight). I
>> don't do the latter in my setup.
> 
> You probably have not looked at the output of "show stats typed", it
> gives you the nature of each value letting you know how to aggregate
> them (min, max, avg, sum, pick any, etc).
> 

I have seen it but it isn't available on 1.6. It could simplify my code, I 
should give a try.

>> - Aggregation across multiple processes for haproxy stats(show info output)
> 
> It's not only "show info", this one reports only the process health.
> 
>> As you can see I provide stats per process and across all processes.
>> It has been proven very useful to know the CPU utilization per process. We 
>> depend on the kernel
>> to do the distribution of incoming connects to all processes and so far it 
>> works very well, but
>> sometimes you see a single process to consume a lot of CPU and if you don't 
>> provide percentiles
>> or stats per process then you are going to miss it. The metrics about 
>> uptime, version,
>> description and few other can be excluded in the aggregation.
> 
> These last ones are in the "pick any" type of aggregation I was talking about.
> 
>> - nbproc > 1 and aggregation for frontend/backend/server
>> My proposal doesn't cover stats for frontend/backend/server per haproxy 
>> process.
> 
> But that's precisely the limitation I'm reporting :-)
> 
>> The stats are already aggregated and few metrics are excluded. For example 
>> all status stuff.
>> Each process performs healthchecking, so they act as little brains which 
>> never agree on the
>> status of a server as they run their checks on different interval.
> 
> Absolutely, but at least you want to see their stats. For example how many
> times a server has switched state per process then in total (meaning a
> proportional amount of possibly visible issues).
> 

True, but in setups with ECMP in front of N HAProxy nodes which run in nbproc 
mode you offload
application healthchecking to a dedicated daemon which runs on servers(service
discovery+service availability with consul/zookeeper stuff) and you only run 
TCP checks
from HAProxy.

In our setup we don't real care about how many times a server flapped, it 
doesn't tell us
something we don't know already, application is in broken state.

But, other people may find it useful.

> My issue is that if the *format* doesn't support per-process stats, we'll have
> to emit a new format 3 months later for all the people who want to process it.
> We've reworked the stats dump to put an end to the problem where depending on
> the output format you used to have different types of information, and there
> was no single representation carrying them all at once. For me now it's
> essential that if we prepare a new format it's not stripped down from the
> info people need, otherwise it will automatically engender yet another 

Re: Getting JSON encoded data from the stats socket.

[Willy Tarreau]

On Tue, Jul 26, 2016 at 05:51:08PM +0200, Pavlos Parissis wrote:
> In all my setups I have nbproc > 1 and after a lot of changes and on how I 
> aggregate HAProxy
> stats and what most people want to see on graphs, I came up that with 
> something like the following:
> 
> {
> "frontend": {
> "www.haproxy.org": {
> "bin": "",
> "lbtot": "55",
> ...
> },
> "www.haproxy.com": {
> "bin": "",
> "lbtot": "55",
> ...
> },
> },
> "backend": {
> "www.haproxy.org": {
> "bin": "",
> "lbtot": "55",
> 
> "server": {
> "srv1": {
> "bin": "",
> "lbtot": "55",
>
> },
> ...
> },
> },
> },
> "haproxy": {
> "PipesFree": "555",
> ...
> ,
> "per_process": {
> "id1": {
> "PipesFree": "555",
> "Process_num": "1",
> ...
> },
> "id2": {
> "PipesFree": "555",
> "Process_num": "2",
> ...
> },
> ...
> },
> },
> "server": {
> "srv1": {
> "bin": "",
> "lbtot": "55",
> ...
> },
> ...
> },
> }
> 
> 
> Let me explain a bit:
> 
> - It is very useful and handy to know stats for a server per backend but also 
> across all
> backends. Thus, I include a top level key 'server' which holds stats for each 
> server across all
> backends. Few server's stats has to be excluded as they are meaningless in 
> this context.
> For example, status, lastchg, check_duration, check_code and few others. For 
> those which aren't
> counters but fixed numbers you want to either sum them(slim) or get the 
> average(weight). I
> don't do the latter in my setup.

You probably have not looked at the output of "show stats typed", it
gives you the nature of each value letting you know how to aggregate
them (min, max, avg, sum, pick any, etc).

> - Aggregation across multiple processes for haproxy stats(show info output)

It's not only "show info", this one reports only the process health.

> As you can see I provide stats per process and across all processes.
> It has been proven very useful to know the CPU utilization per process. We 
> depend on the kernel
> to do the distribution of incoming connects to all processes and so far it 
> works very well, but
> sometimes you see a single process to consume a lot of CPU and if you don't 
> provide percentiles
> or stats per process then you are going to miss it. The metrics about uptime, 
> version,
> description and few other can be excluded in the aggregation.

These last ones are in the "pick any" type of aggregation I was talking about.

> - nbproc > 1 and aggregation for frontend/backend/server
> My proposal doesn't cover stats for frontend/backend/server per haproxy 
> process.

But that's precisely the limitation I'm reporting :-)

> The stats are already aggregated and few metrics are excluded. For example 
> all status stuff.
> Each process performs healthchecking, so they act as little brains which 
> never agree on the
> status of a server as they run their checks on different interval.

Absolutely, but at least you want to see their stats. For example how many
times a server has switched state per process then in total (meaning a
proportional amount of possibly visible issues).

My issue is that if the *format* doesn't support per-process stats, we'll have
to emit a new format 3 months later for all the people who want to process it.
We've reworked the stats dump to put an end to the problem where depending on
the output format you used to have different types of information, and there
was no single representation carrying them all at once. For me now it's
essential that if we prepare a new format it's not stripped down from the
info people need, otherwise it will automatically engender yet another format.

Thanks,
Willy

Re: Getting JSON encoded data from the stats socket.

[Pavlos Parissis]

On 26/07/2016 03:30 μμ, Willy Tarreau wrote:
> Hi Pavlos!
> 
> On Tue, Jul 26, 2016 at 03:23:01PM +0200, Pavlos Parissis wrote:
>> Here is a suggestion { "frontend": { "www.haproxy.org": { "bin": 
>> "", "lbtot":
>> "55", ... }, "www.haproxy.com": { "bin": "", "lbtot": 
>> "55", ... }, }, 
>> "backend": { "www.haproxy.org": { "bin": "", "lbtot": "55", 
>>  "server":
>> { "srv1": { "bin": "", "lbtot": "55",  }, ... }
>> 
>> }, }, "haproxy": { "id1": { "PipesFree": "555", "Process_num": "1", ... }, 
>> "id2": { 
>> "PipesFree": "555", "Process_num": "2", ... }, ... }, }
> 
> Thanks. How does it scale if we later want to aggregate these ones over 
> multiple processes
> and/or nodes ? The typed output already emits a process number for each 
> field. Also, we do
> have the information of how data need to be parsed and aggregated. I suspect 
> that we want to
> produce this with the JSON output as well so that we don't lose information 
> when dumping in
> JSON mode. I would not be surprized if people find JSON easier to process 
> than our current
> format to aggregate their stats, provided we have all the fields :-)
> 
> Cheers, Willy
> 

I am glad you asked about aggregation as I deliberately didn't include 
aggregation.
In all my setups I have nbproc > 1 and after a lot of changes and on how I 
aggregate HAProxy
stats and what most people want to see on graphs, I came up that with something 
like the following:

{
"frontend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",
...
},
"www.haproxy.com": {
"bin": "",
"lbtot": "55",
...
},
},
"backend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",

"server": {
"srv1": {
"bin": "",
"lbtot": "55",
   
},
...
},
},
},
"haproxy": {
"PipesFree": "555",
...
,
"per_process": {
"id1": {
"PipesFree": "555",
"Process_num": "1",
...
},
"id2": {
"PipesFree": "555",
"Process_num": "2",
...
},
...
},
},
"server": {
"srv1": {
"bin": "",
"lbtot": "55",
...
},
...
},
}


Let me explain a bit:

- It is very useful and handy to know stats for a server per backend but also 
across all
backends. Thus, I include a top level key 'server' which holds stats for each 
server across all
backends. Few server's stats has to be excluded as they are meaningless in this 
context.
For example, status, lastchg, check_duration, check_code and few others. For 
those which aren't
counters but fixed numbers you want to either sum them(slim) or get the 
average(weight). I
don't do the latter in my setup.

- Aggregation across multiple processes for haproxy stats(show info output)
As you can see I provide stats per process and across all processes.
It has been proven very useful to know the CPU utilization per process. We 
depend on the kernel
to do the distribution of incoming connects to all processes and so far it 
works very well, but
sometimes you see a single process to consume a lot of CPU and if you don't 
provide percentiles
or stats per process then you are going to miss it. The metrics about uptime, 
version,
description and few other can be excluded in the aggregation.


- nbproc > 1 and aggregation for frontend/backend/server
My proposal doesn't cover stats for frontend/backend/server per haproxy process.
The stats are already aggregated and few metrics are excluded. For example all 
status stuff.
Each process performs healthchecking, so they act as little brains which never 
agree on the
status of a server as they run their checks on different interval. But, if 
nbproc == 1 then
these metrics have to be included.


Cheers,
Pavlos







signature.asc
Description: OpenPGP digital signature

Re: Getting JSON encoded data from the stats socket.

[Willy Tarreau]

On Tue, Jul 26, 2016 at 03:06:35PM +0100, Mark Brookes wrote:
> Could we perhaps group by the node then process_num then?
> {nodename:value:
> {pid: pid1: {
> haproxy: {
> Uptime_sec:100,
> PoolFailed:1
> }
>   stats: { "frontend": {
> "www.haproxy.org": {
> "bin": "",
> "lbtot": "55",
> ...
(...)

Yes I think it's fine this way because in practice, clients will consult
a single process at a time so it's easier to have per-process dumps to
aggregate later.

Willy

Re: Getting JSON encoded data from the stats socket.

[Mark Brookes]

Could we perhaps group by the node then process_num then?
{nodename:value:
{pid: pid1: {
haproxy: {
Uptime_sec:100,
PoolFailed:1
}
  stats: { "frontend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",
...
},
"www.haproxy.com": {
"bin": "",
"lbtot": "55",
...
},
},
"backend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",

"server": {
"srv1": {
"bin": "",
"lbtot": "55",
   
},
...
}

},
{pid: pid2: { haproxy: {
Uptime_sec:100,
PoolFailed:1
}
  stats: { "frontend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",
...
},
"www.haproxy.com": {
"bin": "",
"lbtot": "55",
...
},
},
"backend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",

"server": {
"srv1": {
"bin": "",
"lbtot": "55",
   
},
...
}

},

ignore the close brackets im pretty sure they are wrong, but you get the idea.

On 26 July 2016 at 14:30, Willy Tarreau  wrote:
> Hi Pavlos!
>
> On Tue, Jul 26, 2016 at 03:23:01PM +0200, Pavlos Parissis wrote:
>> Here is a suggestion
>> {
>> "frontend": {
>> "www.haproxy.org": {
>> "bin": "",
>> "lbtot": "55",
>> ...
>> },
>> "www.haproxy.com": {
>> "bin": "",
>> "lbtot": "55",
>> ...
>> },
>> },
>> "backend": {
>> "www.haproxy.org": {
>> "bin": "",
>> "lbtot": "55",
>> 
>> "server": {
>> "srv1": {
>> "bin": "",
>> "lbtot": "55",
>>
>> },
>> ...
>> }
>>
>> },
>> },
>> "haproxy": {
>> "id1": {
>> "PipesFree": "555",
>> "Process_num": "1",
>> ...
>> },
>> "id2": {
>> "PipesFree": "555",
>> "Process_num": "2",
>> ...
>> },
>> ...
>> },
>> }
>
> Thanks. How does it scale if we later want to aggregate these ones over
> multiple processes and/or nodes ? The typed output already emits a
> process number for each field. Also, we do have the information of how
> data need to be parsed and aggregated. I suspect that we want to produce
> this with the JSON output as well so that we don't lose information when
> dumping in JSON mode. I would not be surprized if people find JSON easier
> to process than our current format to aggregate their stats, provided we
> have all the fields :-)
>
> Cheers,
> Willy

Re: Getting JSON encoded data from the stats socket.

[Willy Tarreau]

Hi Pavlos!

On Tue, Jul 26, 2016 at 03:23:01PM +0200, Pavlos Parissis wrote:
> Here is a suggestion
> {
> "frontend": {
> "www.haproxy.org": {
> "bin": "",
> "lbtot": "55",
> ...
> },
> "www.haproxy.com": {
> "bin": "",
> "lbtot": "55",
> ...
> },
> },
> "backend": {
> "www.haproxy.org": {
> "bin": "",
> "lbtot": "55",
> 
> "server": {
> "srv1": {
> "bin": "",
> "lbtot": "55",
>
> },
> ...
> }
> 
> },
> },
> "haproxy": {
> "id1": {
> "PipesFree": "555",
> "Process_num": "1",
> ...
> },
> "id2": {
> "PipesFree": "555",
> "Process_num": "2",
> ...
> },
> ...
> },
> }

Thanks. How does it scale if we later want to aggregate these ones over
multiple processes and/or nodes ? The typed output already emits a
process number for each field. Also, we do have the information of how
data need to be parsed and aggregated. I suspect that we want to produce
this with the JSON output as well so that we don't lose information when
dumping in JSON mode. I would not be surprized if people find JSON easier
to process than our current format to aggregate their stats, provided we
have all the fields :-)

Cheers,
Willy

Re: Getting JSON encoded data from the stats socket.

[Pavlos Parissis]

On 26/07/2016 03:08 μμ, Willy Tarreau wrote:
> On Tue, Jul 26, 2016 at 02:05:56PM +0100, Mark Brookes wrote:
>>> So for sure I definitely support this proposal :-)
>>
>> Thats great news. Do you have a JSON structure in mind?
>> Or would you like me to come up with something?
> 
> I'm probably the worst ever person to suggest a JSON structure. If you
> have any ideas, please bring them on the list. You know how it works,
> once nobody criticizes anymore, your design is fine. And you'll just
> have to ignore people who complain after the work is done :-)
> 
> Cheers,
> Willy
> 

Here is a suggestion
{
"frontend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",
...
},
"www.haproxy.com": {
"bin": "",
"lbtot": "55",
...
},
},
"backend": {
"www.haproxy.org": {
"bin": "",
"lbtot": "55",

"server": {
"srv1": {
"bin": "",
"lbtot": "55",
   
},
...
}

},
},
"haproxy": {
"id1": {
"PipesFree": "555",
"Process_num": "1",
...
},
"id2": {
"PipesFree": "555",
"Process_num": "2",
...
},
...
},
}

Cheers,
Pavlos




signature.asc
Description: OpenPGP digital signature

Re: counters for specific http status code

[Ruoshan Huang]

>
>
> Yes you're right. For me this one fixes it (both with http-request and
> multiple http-response), is that OK for you ?
>
>
Yes, looks good to me.

-- 
Good day!
ruoshan

Re: Getting JSON encoded data from the stats socket.

[Willy Tarreau]

On Tue, Jul 26, 2016 at 02:05:56PM +0100, Mark Brookes wrote:
> >So for sure I definitely support this proposal :-)
> 
> Thats great news. Do you have a JSON structure in mind?
> Or would you like me to come up with something?

I'm probably the worst ever person to suggest a JSON structure. If you
have any ideas, please bring them on the list. You know how it works,
once nobody criticizes anymore, your design is fine. And you'll just
have to ignore people who complain after the work is done :-)

Cheers,
Willy

Re: Getting JSON encoded data from the stats socket.

[Mark Brookes]

>So for sure I definitely support this proposal :-)

Thats great news. Do you have a JSON structure in mind?
Or would you like me to come up with something?

On 5 July 2016 at 18:04, Willy Tarreau  wrote:
> Hi Mark,
>
> On Tue, Jul 05, 2016 at 10:05:13AM +0100, Mark Brookes wrote:
>> Hi Willy/All
>>
>> I wondered if we could start a discussion about the possibility of
>> having the stats socket return stats data in JSON format.
>>
>> Im primarily interested in the data that is returned by issuing a
>> 'show stat' which is normally returned as a csv.
>>
>> I wont go into specifics as to how the data would be structured, we
>> can decide on that later (Assuming you are happy with this idea).
>>
>> Ive approached Simon Horman and hes happy to do the work for us.
>>
>> Please let me know your thoughts
>
> Well, I completely reworked the stats internals recently for two
> purposes :
>   1) bringing the ability to dump them in another format such as JSON ;
>   2) making it easier to aggregate them over multiple processes/nodes
>
> So for sure I definitely support this proposal :-)
>
> Best regards
> Willy

Re: counters for specific http status code

[Willy Tarreau]

On Tue, Jul 26, 2016 at 08:44:59PM +0800, Ruoshan Huang wrote:
> Ah, I just find a problem in this snip
> 
> ```
> +   if ((unsigned)(txn->status - 400) <
> 100)
> +   stream_inc_http_err_ctr(s);
> ```
> calling `stream_inc_http_err_ctr` may cause the http_err_cnt be increased
> twice for other stick counter tracked by `http-request track-sc`. for
> example:
> 
> ```
> frontend fe
> bind *:7001
> stick-table type ip size 100 expire 10m store http_req_cnt,http_err_cnt
> http-request track-sc1 src
> http-response track-sc2 status table dummy
> default_backend be
> 
> backend be
> server b1 127.0.0.1:8000
> 
> backend dummy
> stick-table type integer size 100 expire 10m store
> http_req_cnt,http_err_cnt
> ```
> the `http_err_cnt` in table `fe` will be increased twice for every 4xx
> response.

Ah crap, I didn't think about this case. And I tried to find corner cases :-(
And that just makes me think that using multiple consecutive http-response
track-sc rules will cause the same problem.

> we should only increase the err counter for the current stick counter entry.

Yes you're right. For me this one fixes it (both with http-request and
multiple http-response), is that OK for you ?

Willy

diff --git a/src/proto_http.c b/src/proto_http.c
index d2090bb..3e18a6c 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3816,8 +3816,16 @@ resume_execution:
 * but here we're tracking after this 
ought to have been done so we have
 * to do it on purpose.
 */
-   if ((unsigned)(txn->status - 400) < 100)
-   stream_inc_http_err_ctr(s);
+   if ((unsigned)(txn->status - 400) < 
100) {
+   ptr = stktable_data_ptr(t, ts, 
STKTABLE_DT_HTTP_ERR_CNT);
+   if (ptr)
+   stktable_data_cast(ptr, 
http_err_cnt)++;
+
+   ptr = stktable_data_ptr(t, ts, 
STKTABLE_DT_HTTP_ERR_RATE);
+   if (ptr)
+   
update_freq_ctr_period(_data_cast(ptr, http_err_rate),
+  
t->data_arg[STKTABLE_DT_HTTP_ERR_RATE].u, 1);
+   }
}
}
break;

Re: counters for specific http status code

[Ruoshan Huang]

I'm afraid I have to duplicate some logic here, like:
```
if((unsigned)(txn->status - 400) < 100) {
ptr = stktable_data_ptr(t, ts,
STKTABLE_DT_HTTP_ERR_CNT);
if (ptr)
stktable_data_cast(ptr, http_err_cnt)++;

ptr = stktable_data_ptr(t, ts,
STKTABLE_DT_HTTP_ERR_RATE);
if (ptr)
update_freq_ctr_period(_data_cast(ptr,
http_err_rate),

 t->data_arg[STKTABLE_DT_HTTP_ERR_RATE].u, 1);
}
```

On Tue, Jul 26, 2016 at 8:44 PM, Ruoshan Huang 
wrote:

> Ah, I just find a problem in this snip
>
> ```
> +   if ((unsigned)(txn->status - 400)
> < 100)
> +   stream_inc_http_err_ctr(s);
> ```
> calling `stream_inc_http_err_ctr` may cause the http_err_cnt be increased
> twice for other stick counter tracked by `http-request track-sc`. for
> example:
>
> ```
> frontend fe
> bind *:7001
> stick-table type ip size 100 expire 10m store http_req_cnt,http_err_cnt
> http-request track-sc1 src
> http-response track-sc2 status table dummy
> default_backend be
>
> backend be
> server b1 127.0.0.1:8000
>
> backend dummy
> stick-table type integer size 100 expire 10m store
> http_req_cnt,http_err_cnt
> ```
> the `http_err_cnt` in table `fe` will be increased twice for every 4xx
> response.
>
> we should only increase the err counter for the current stick counter
> entry.
>
> On Tue, Jul 26, 2016 at 8:34 PM, Willy Tarreau  wrote:
>
>> On Tue, Jul 26, 2016 at 06:24:50PM +0800, Ruoshan Huang wrote:
>> > >
>> > > If you're fine with this, I'll simply merge them with the change
>> above,
>> > > it's clean enough, no need to respin. Please just confirm that it's
>> fine
>> > > for you as well.
>> > >
>> > >
>> > Yes, please ship it.
>> > Thank you so much.
>>
>> OK now merged. I tested this config on it :
>>
>> http-response track-sc0 status
>> stick-table type integer size 200k store http_req_cnt,http_err_cnt
>>
>> and it works pretty fine :
>>
>> $ echo "show table l" | socat - /tmp/sock1
>> # table: l, type: integer, size:204800, used:3
>> 0x7ef7b0: key=302 use=0 exp=0 http_req_cnt=1 http_err_cnt=0
>> 0x7ef718: key=404 use=0 exp=0 http_req_cnt=1 http_err_cnt=1
>> 0x7ef070: key=200 use=0 exp=0 http_req_cnt=2 http_err_cnt=0
>>
>> That's cool :-)
>>
>> Thanks,
>> Willy
>>
>
>
>
> --
> Good day!
> ruoshan
>



-- 
Good day!
ruoshan

Re: counters for specific http status code

[Ruoshan Huang]

Ah, I just find a problem in this snip

```
+   if ((unsigned)(txn->status - 400) <
100)
+   stream_inc_http_err_ctr(s);
```
calling `stream_inc_http_err_ctr` may cause the http_err_cnt be increased
twice for other stick counter tracked by `http-request track-sc`. for
example:

```
frontend fe
bind *:7001
stick-table type ip size 100 expire 10m store http_req_cnt,http_err_cnt
http-request track-sc1 src
http-response track-sc2 status table dummy
default_backend be

backend be
server b1 127.0.0.1:8000

backend dummy
stick-table type integer size 100 expire 10m store
http_req_cnt,http_err_cnt
```
the `http_err_cnt` in table `fe` will be increased twice for every 4xx
response.

we should only increase the err counter for the current stick counter entry.

On Tue, Jul 26, 2016 at 8:34 PM, Willy Tarreau  wrote:

> On Tue, Jul 26, 2016 at 06:24:50PM +0800, Ruoshan Huang wrote:
> > >
> > > If you're fine with this, I'll simply merge them with the change above,
> > > it's clean enough, no need to respin. Please just confirm that it's
> fine
> > > for you as well.
> > >
> > >
> > Yes, please ship it.
> > Thank you so much.
>
> OK now merged. I tested this config on it :
>
> http-response track-sc0 status
> stick-table type integer size 200k store http_req_cnt,http_err_cnt
>
> and it works pretty fine :
>
> $ echo "show table l" | socat - /tmp/sock1
> # table: l, type: integer, size:204800, used:3
> 0x7ef7b0: key=302 use=0 exp=0 http_req_cnt=1 http_err_cnt=0
> 0x7ef718: key=404 use=0 exp=0 http_req_cnt=1 http_err_cnt=1
> 0x7ef070: key=200 use=0 exp=0 http_req_cnt=2 http_err_cnt=0
>
> That's cool :-)
>
> Thanks,
> Willy
>



-- 
Good day!
ruoshan

Re: counters for specific http status code

[Willy Tarreau]

On Tue, Jul 26, 2016 at 06:24:50PM +0800, Ruoshan Huang wrote:
> >
> > If you're fine with this, I'll simply merge them with the change above,
> > it's clean enough, no need to respin. Please just confirm that it's fine
> > for you as well.
> >
> >
> Yes, please ship it.
> Thank you so much.

OK now merged. I tested this config on it :

http-response track-sc0 status
stick-table type integer size 200k store http_req_cnt,http_err_cnt

and it works pretty fine :

$ echo "show table l" | socat - /tmp/sock1
# table: l, type: integer, size:204800, used:3
0x7ef7b0: key=302 use=0 exp=0 http_req_cnt=1 http_err_cnt=0
0x7ef718: key=404 use=0 exp=0 http_req_cnt=1 http_err_cnt=1
0x7ef070: key=200 use=0 exp=0 http_req_cnt=2 http_err_cnt=0

That's cool :-)

Thanks,
Willy

Re: counters for specific http status code

[Ruoshan Huang]

>
> If you're fine with this, I'll simply merge them with the change above,
> it's clean enough, no need to respin. Please just confirm that it's fine
> for you as well.
>
>
Yes, please ship it.
Thank you so much.

--
Good day!
ruoshan

Re: counters for specific http status code

[Willy Tarreau]

Hi Ruoshan,

On Thu, Jul 14, 2016 at 03:24:42PM +0800, Ruoshan Huang wrote:
> hi,
> here is my (final?) patch for implementing `http-response track-sc*` 
> directive. mainly I have:
> - duplicate config parsing (`parse_http_res_cond`) and validation 
> (`check_config_validity`) code for track-sc
> - add ACT_ACTION_TRK_SC* case in `http_res_get_intercept_rule` to do the 
> tracking
> - rename http_req_trk_idx to http_trk_idx
> - doc of course
> 
>and the  smp expr for this directive requires smp with capability of 
> `SMP_VAL_FE/BE_HRS_HDR`,
> I try to explain this in the document, but the `fetch_cap` is very 
> complicated, hope I didn't fail explaining it :)
> 
>please review these patchs when you're avaliable, Thanks.

So that's a very good work overall. I only found one minor issue that's easy
to fix : the http error rate was not updated. Normally it's updated immediately
when the response is received. But here, since we're doing the tracking after
this ought to be done, we have to explicitly do it. With the patch below it
works (sorry for the copy paste) :

diff --git a/src/proto_http.c b/src/proto_http.c
index 96f19a6..80e7a4c 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3808,6 +3808,16 @@ resume_execution:

stkctr_set_flags(>stkctr[http_trk_idx(rule->action)], STKCTR_TRACK_CONTENT);
if (sess->fe != s->be)

stkctr_set_flags(>stkctr[http_trk_idx(rule->action)], STKCTR_TRACK_BACKEND);
+
+   /* When the client triggers a 4xx from 
the server, it's most often due
+* to a missing object or permission. 
These events should be tracked
+* because if they happen often, it may 
indicate a brute force or a
+* vulnerability scan. Normally this is 
done when receiving the response
+* but here we're tracking after this 
ought to have been done so we have
+* to do it on purpose.
+*/
+   if ((unsigned)(txn->status - 400) < 100)
+   stream_inc_http_err_ctr(s);
}
}
break;

Regarding the doc, I'd just remove the suggestion about trying and seeing
if it works or not, as that only incites users to be lazy and to rely on
the config parser instead of understanding what they are doing. I'll also
merge your two patches into a single one as it's better to have the doc
with the feature when it comes as a single patch.

If you're fine with this, I'll simply merge them with the change above,
it's clean enough, no need to respin. Please just confirm that it's fine
for you as well.

Thanks,
Willy

[no subject]

[promotion]

Hi Customer,  If you can not see the description below, please click here. 如無法閱讀以下的內容，請按此.To learn more, please visit www.hk-printing.com.hk. 想了解多D請到www.hk-printing.com.hk  印刷 | 設計 | 製作 | 安裝一站式服務   Wedding Products Series打造一個屬於自己的婚禮  Signature Board   Back Drop  背景板Wedding Card   結婚咭簽名板   Wedding Card結婚咭   結婚咭是喜事必備的， 設計別致， 顏色  華麗的結婚咭象微著新人婚後的幸福美滿。精美結婚咭，60款不同款式任你選！           Back Drop背景板  背景板可因應場地位置作出相對安裝方法，我地有轉門安裝團隊為你服務。  如客人有需要可要求到場覆尺  Signature Board簽名板   讓你的親朋好友留下名字，送上祝福。       Red pocket利是封  歡迎咨詢Hot Line：82007559Email：sa...@hk-printing.com.hkStandee人形展架  If our promotional email  have causing you any disturbance, please email(promot...@hk-printing.com.hk)  and acknowledge us for cancelation of the mailing list. 如此郵件對閣下帶來騷擾, 請以EMAIL(promot...@hk-printing.com.hk) 通知我們