counters for specific http status code

[Ruoshan Huang]

hi,
I use the hrsp4xx/hrsp5xx a lot to do timeseries metric collecting, 
sometimes the these counters lack details. I'm wondering what's the badside of 
no collecting all the HTTP status (just an array of 500 length? I know there're 
many empty slot in it).
Currently I have to do the accounting on the log file. but on a high load 
machine, it cost a lot to processing the log itself. There're ways like 
logstash or log to remote machine. but do the accounting on HA seems to be a 
very light approach.
I've a draft patch for this feature, just adding a stats socket command 
like `show httprsp[backend]`. Wish to get some idea 
on this. Thanks



0001-supporting-dumping-http-response-status-code.patch
Description: Binary data


--
Good day!
ruoshan

Re: counters for specific http status code

[Willy Tarreau]

Hi Ruoshan,

On Tue, Jul 05, 2016 at 12:28:43PM +0800, Ruoshan Huang wrote:
> hi,
> I use the hrsp4xx/hrsp5xx a lot to do timeseries metric collecting,
> sometimes the these counters lack details. I'm wondering what's the
> badside of no collecting all the HTTP status (just an array of 500
> length? I know there're many empty slot in it).

What I'm seeing is that it will consume 4 more kB of memory in each
proxy. That can be a bit problematic for people who use a *lot* of
backends. The largest configs I've been aware were 30 backends
(yes I know that sounds insane). This would add 1.2 GB of RAM just
to add these extra counters. We could imagine dynamically storing
them into a tree but it would consume so much more per individual
entry that it would not be worth it.

> Currently I have to do the accounting on the log file. but on a high load
> machine, it cost a lot to processing the log itself.

Well, on my local machine, halog takes only 0.98 second to process a 2.3 GB
file and give me the status distribution (8.4 million lines). That's 2.4 GB
per second or 8.6 million lines per second. This is not something I would
consider as a high load, because if you rotate your log files past 2 GB,
you'd eat less than 1 second of CPU for each analysis. It's probably less
than what can be lost per hour due to cache misses when adding 4kB per
proxy to a large config in fact :-/

> There're ways like
> logstash or log to remote machine. but do the accounting on HA seems to
> be a very light approach.
> I've a draft patch for this feature, just adding a stats socket command
> like `show httprsp[backend]`. Wish to get some
> idea on this. Thanks

I'm not opposed to trying to store a bit more stats, but as you saw we'd
be eating a lot of memory to store a lot of holes. Could you please do me
a favor, and count the number of different status codes you're seeing on
your side ? There's a compact variant of the ebtree which saves two pointers
and which would require only 2 pointers, a key and a value per node, hence
around 26 bytes, or 18 or even 14 with relative addressing if we limit the
number of different statuses per proxy. Maybe this combined with dynamic
allocation to move that part out of the struct proxy would make sense
because 1) it would eat much less memory per proxy for small number of
statuses (eg: break-even around 150 different statuses) and would not
eat memory for backends with little or no traffic. It would also allow
us to remove the rsp[6] array since we would be able to compute these
values by walking over the tree.

Thanks,
Willy


0001-supporting-dumping-http-response-status-code.patch
Description: Binary data

Re: counters for specific http status code

[Ruoshan Huang]

hi,

> On Jul 12, 2016, at 12:05 PM, Willy Tarreau  wrote:
> 
> Hi Ruoshan,
> 
> On Tue, Jul 05, 2016 at 12:28:43PM +0800, Ruoshan Huang wrote:
>> hi,
>>I use the hrsp4xx/hrsp5xx a lot to do timeseries metric collecting,
>>sometimes the these counters lack details. I'm wondering what's the
>>badside of no collecting all the HTTP status (just an array of 500
>>length? I know there're many empty slot in it).
> 
> What I'm seeing is that it will consume 4 more kB of memory in each
> proxy. That can be a bit problematic for people who use a *lot* of
> backends. The largest configs I've been aware were 30 backends
> (yes I know that sounds insane). This would add 1.2 GB of RAM just
> to add these extra counters. We could imagine dynamically storing
> them into a tree but it would consume so much more per individual
> entry that it would not be worth it.

Thank you so much for elaborating in such detail !

Indeed I only care about 20 status code or so, like 400, 401, 403 (I have been 
bitten many times by treating those codes as 404 :( ), and 50x also helps a lot 
in our alerting system as a first method for diagnosing abnormal things. By and 
large, the "big" array does waste a lot of memory (I know), but that's the fast 
way I try to make a demo.

> 
>>Currently I have to do the accounting on the log file. but on a high load
>>machine, it cost a lot to processing the log itself.
> 
> Well, on my local machine, halog takes only 0.98 second to process a 2.3 GB
> file and give me the status distribution (8.4 million lines). That's 2.4 GB
> per second or 8.6 million lines per second. This is not something I would
> consider as a high load, because if you rotate your log files past 2 GB,
> you'd eat less than 1 second of CPU for each analysis. It's probably less
> than what can be lost per hour due to cache misses when adding 4kB per
> proxy to a large config in fact :-/

havn't used `halog` for parsing, will try it. but I prefer collecting metrics 
remotely, there are many hosts and stat sock can dump the stick table (stick 
table is realy amazing, thank you for developing this!)

> 
>> There're ways like
>>logstash or log to remote machine. but do the accounting on HA seems to
>>be a very light approach.
>>I've a draft patch for this feature, just adding a stats socket command
>>like `show httprsp[backend]`. Wish to get some
>>idea on this. Thanks
> 
> I'm not opposed to trying to store a bit more stats, but as you saw we'd
> be eating a lot of memory to store a lot of holes. Could you please do me
> a favor, and count the number of different status codes you're seeing on
> your side ? There's a compact variant of the ebtree which saves two pointers
> and which would require only 2 pointers, a key and a value per node, hence
> around 26 bytes, or 18 or even 14 with relative addressing if we limit the
> number of different statuses per proxy. Maybe this combined with dynamic
> allocation to move that part out of the struct proxy would make sense
> because 1) it would eat much less memory per proxy for small number of
> statuses (eg: break-even around 150 different statuses) and would not
> eat memory for backends with little or no traffic. It would also allow
> us to remove the rsp[6] array since we would be able to compute these
> values by walking over the tree.
> 

At first I was thinking whether we could track the response status in stick 
table, then it may be neat. but currently there isn't `http-response track-sc?` 
directive. can it?

Using a dynamic ebtree will eventually recreating a tracking table or something 
like the stick table, so same question here: is it possible do the stick 
counter tracking in response phase, so more things can be tracked.

--
Good day!
ruoshan

Re: counters for specific http status code

[Willy Tarreau]

On Tue, Jul 12, 2016 at 12:29:56PM +0800, Ruoshan Huang wrote:
> Indeed I only care about 20 status code or so, like 400, 401, 403 (I have
> been bitten many times by treating those codes as 404 :( ), and 50x also
> helps a lot in our alerting system as a first method for diagnosing abnormal
> things. By and large, the "big" array does waste a lot of memory (I know),
> but that's the fast way I try to make a demo.

I have no problems with demos, don't worry :-) Most often a small piece
of code or a patch speaks better than a long paragraph!

> >>Currently I have to do the accounting on the log file. but on a high 
> >> load
> >>machine, it cost a lot to processing the log itself.
> > 
> > Well, on my local machine, halog takes only 0.98 second to process a 2.3 GB
> > file and give me the status distribution (8.4 million lines). That's 2.4 GB
> > per second or 8.6 million lines per second. This is not something I would
> > consider as a high load, because if you rotate your log files past 2 GB,
> > you'd eat less than 1 second of CPU for each analysis. It's probably less
> > than what can be lost per hour due to cache misses when adding 4kB per
> > proxy to a large config in fact :-/
> 
> havn't used `halog` for parsing, will try it. but I prefer collecting metrics
> remotely, there are many hosts and stat sock can dump the stick table (stick
> table is realy amazing, thank you for developing this!)

That could possibly be ssh $host "halog -st < /var/log/haproxy.log" or
anything like this.

> At first I was thinking whether we could track the response status in stick
> table, then it may be neat. but currently there isn't `http-response
> track-sc?` directive. can it?

Interesting. No it isn't, just because I think we never found a valid
use case for it. It's possible that you found the first one in fact :-)

> Using a dynamic ebtree will eventually recreating a tracking table or
> something like the stick table, so same question here: is it possible do the
> stick counter tracking in response phase, so more things can be tracked.

I think it would be a nice approach, we'd be reusing existing infrastructure
and possibly even take routing decisions based on certain stats (eg: # of
404 per second) suggesting routing to a cache or a backup instance.

There's also something I'm not happy with regarding the stats approach, it
is that it would be the first set of stats counters which are not dumped
into "show stats" but would instead require their own command. And as you
guessed, dumping 500 empty fields on each frontend/backend line is not
something even remotely imaginable.

Please take a look at how track-sc are added  to http-request, look for
"ACT_ACTION_TRK_SC0" in proto_http.c, you'll find your way through it,
and see if you can duplicate this to the response. There should be a few
tricks such as indicating that you want data present in the response
instead of the request, but nothing terrible. I think you shouldn't have
too many issues adding support for this. Please note however that by
definition it will not count haproxy's own status codes but only the
ones it is forwarding. But in general that shouldn't be an issue as
when you want such details you're more interested in the status codes
generated by the application than the ones generated by haproxy.

Willy

Re: counters for specific http status code

[Ruoshan Huang]

> On Jul 12, 2016, at 12:42 PM, Willy Tarreau  wrote:
> 
> Please take a look at how track-sc are added  to http-request, look for
> "ACT_ACTION_TRK_SC0" in proto_http.c, you'll find your way through it,
> and see if you can duplicate this to the response. There should be a few
> tricks such as indicating that you want data present in the response
> instead of the request, but nothing terrible. I think you shouldn't have
> too many issues adding support for this. Please note however that by
> definition it will not count haproxy's own status codes but only the
> ones it is forwarding. But in general that shouldn't be an issue as
> when you want such details you're more interested in the status codes
> generated by the application than the ones generated by haproxy.


Will do! Thanks

--
Good day!
ruoshan

Re: counters for specific http status code

[Jonathan Matthews]

On 12 Jul 2016 05:43, "Willy Tarreau"  wrote:
> That could possibly be ssh $host "halog -st < /var/log/haproxy.log" or
> anything like this.

On behalf of people running busy load balancers / edge proxies / etc,
please don't do this ;-)
Instead

laptop$ halog -st <(ssh -C $balancer "cat /var/log/haproxy.log")

... is likely to be slightly kinder to my contended servers :-)

J

Re: counters for specific http status code

[Holger Just]

Hi Willy,

Willy Tarreau wrote:
>> At first I was thinking whether we could track the response status in stick
>> table, then it may be neat. but currently there isn't `http-response
>> track-sc?` directive. can it?
> 
> Interesting. No it isn't, just because I think we never found a valid
> use case for it. It's possible that you found the first one in fact :-)

Having this capability would also solve a long-standing itch for myself.

We have some authenticated services (via Basic Auth and other means)
which signal an authentication failure via a 403 status. We want to
throttle and finally block IPs which cause too many authentication
failures. Stick tables would be great for that as long as we could store
the response status to use it in subsequent requests.

I think, right now, we could build a crutch with `http-response
sc-inc-gpc0` but having real http-response track-sc actions would make
thing much easier and cleaner.

As such, I'm all in favor of this :)

Regards,
Holger

Re: counters for specific http status code

[Willy Tarreau]

Hi Holger,

On Wed, Jul 13, 2016 at 12:09:51PM +0200, Holger Just wrote:
> Hi Willy,
> 
> Willy Tarreau wrote:
> >> At first I was thinking whether we could track the response status in stick
> >> table, then it may be neat. but currently there isn't `http-response
> >> track-sc?` directive. can it?
> > 
> > Interesting. No it isn't, just because I think we never found a valid
> > use case for it. It's possible that you found the first one in fact :-)
> 
> Having this capability would also solve a long-standing itch for myself.

Ah good!

> We have some authenticated services (via Basic Auth and other means)
> which signal an authentication failure via a 403 status.

More likely 401 instead.

> We want to
> throttle and finally block IPs which cause too many authentication
> failures. Stick tables would be great for that as long as we could store
> the response status to use it in subsequent requests.

That's a good point. Normally we do that by checking the http_err
counter, but it will count various types of errors which are not
necessarily of interest in your case.

> I think, right now, we could build a crutch with `http-response
> sc-inc-gpc0` but having real http-response track-sc actions would make
> thing much easier and cleaner.

Yes I agree.
Otherwise you can track sc0 on the request and inc-gpc0 during the response.
But having something easier is definitely better.

Regards,
Willy

Re: counters for specific http status code

[Ruoshan Huang]

On Jul 12, 2016, at 12:42 PM, Willy Tarreau  wrote:Please take a look at how track-sc are added  to http-request, look for"ACT_ACTION_TRK_SC0" in proto_http.c, you'll find your way through it,and see if you can duplicate this to the response.I just baked some draft patchs. and get some questions:1. I didn't follow the early code cleanup, and now in v1.6 branch, there is a struct list called `http_res_actions`, should new actions be put in that list instead? the refactoring history spawned many commits, I couldn't figure out the orginal intention.2. what's the purpose of there two macro: `STKCTR_TRACK_BACKEND` and `STKCTR_TRACK_CONTENT`, couldn't find too much comment.3. it feels a little wired to me to store the stick counter data type like `http_req_rate` or `conn_rate` in a http response phase.

0001-add-httprsp-track-sc.patch
Description: Binary data


0002-rename-http_req_trk_idx-to-http_trk_idx.patch
Description: Binary data

--Good day!ruoshan

Re: counters for specific http status code

[Willy Tarreau]

Hi Ruoshan,

On Wed, Jul 13, 2016 at 07:29:57PM +0800, Ruoshan Huang wrote:
> 
> > On Jul 12, 2016, at 12:42 PM, Willy Tarreau  wrote:
> > 
> > Please take a look at how track-sc are added  to http-request, look for
> > "ACT_ACTION_TRK_SC0" in proto_http.c, you'll find your way through it,
> > and see if you can duplicate this to the response.
> 
> I just baked some draft patchs. and get some questions:
> 
> 1. I didn't follow the early code cleanup, and now in v1.6 branch, there is a
> struct list called `http_res_actions`, should new actions be put in that list
> instead? the refactoring history spawned many commits, I couldn't figure out
> the orginal intention.

Ah I forgot about this. If you find how to do it, yes please. But I know
we've not converted the track-sc to this yet, and I guess you won't be
able to use this for now due to the fact that this list contains plain
words only and that here we have optional arguments and even a configurable
number of counters. So most likely you'll have to do as is done for the
other track-sc actions and add dedicated code.

The intent of the refactoring was to be able to register a same action for
use in multiple places. We used to have too much duplication and there was
a strong temptation to only implement certain actions at certain places,
which was counter-productive.

> 2. what's the purpose of there two macro: `STKCTR_TRACK_BACKEND` and
> `STKCTR_TRACK_CONTENT`, couldn't find too much comment.

- the first one indicates that a tracking pointer was set from the backend
  and thus that when a keep-alive request goes to another backend, the track
  must cease.

- the second one indicates that the tracking pointer was set in a
  content-aware rule (tcp-request content or http-request) and that the
  tracking has to be performed in the stream and not in the session, and
  will cease for a new keep-alive request over the same connection.

In your case you'll set at least STKCTR_TRACK_CONTENT, and you'll add
STKCTR_TRACK_BACKEND when you find that strm->be != sess->fe.

> 3. it feels a little wired to me to store the stick counter data type like
> `http_req_rate` or `conn_rate` in a http response phase.

I understand your feeling, it looks strange at first. The terms are maybe
not the best ones but they're the ones we have. Here it will correspond to
a number of request having triggerred your rule for example. If you see
them as L4 events or L7 events, it may sound more natural.

Regards,
Willy

Re: counters for specific http status code

[Ruoshan Huang]

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.



0001-implementing-http-response-track-sc-directive.patch
Description: Binary data


0002-add-doc-for-http-response-track-sc.patch
Description: Binary data

--
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.

I'm leaving your mail marked "unread" so that I avoid to miss it. If
you don't get any feedback by the end of next week, please ping again.

Thanks,
Willy

Re: counters for specific http status code

[Ruoshan Huang]

hi Willy,    please check this thread again when you are free. thanks

--
ruoshan
from Outlook mobile




On Thu, Jul 21, 2016 at 3:30 AM +0800, "Willy Tarreau"  wrote:










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.

I'm leaving your mail marked "unread" so that I avoid to miss it. If
you don't get any feedback by the end of next week, please ping again.

Thanks,
Willy

Re: counters for specific http status code

[Willy Tarreau]

On Mon, Jul 25, 2016 at 05:58:52AM +, Ruoshan Huang wrote:
> hi Willy,    please check this thread again when you are free. thanks

Hi Ruoshan,

yes I've added it to my todo list. Hopefully today.

thanks,
Willy

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(&s->stkctr[http_trk_idx(rule->action)], STKCTR_TRACK_CONTENT);
if (sess->fe != s->be)

stkctr_set_flags(&s->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

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]

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]

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

[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(&stktable_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

[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(&stktable_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]

>
>
> 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