Re: [PATCH] log received body size

Hi all.
   I am sending the second version.
Amos Jeffries wrote:
>
> * It took me a long while to see that the -1 values being used as
> defaults were intended to be converted to dash in the long. Please
> update all the relevant places to document that fact.

OK I add some documentation in various places

>
>
> src/Server.cc:
>
> ServerStateData::addVirginReplyBody(const char *data, ssize_t len)
> {
> + adjustBodyBytesRead(+len);
>
> whats with the +? we don't overload the unary operator anywhere and the
> libc default is no-op AIUI. If len is required to be positive here thats
> grounds for an abs(), Must() or assert() to prevent data corruption and
> highlight the bugs.

the "+" removed

>
> src/adaptation/icap/ModXact.cc:
>
> src/cf.data.pre:
>
> The use of [http::] is a bit congruous here. Since the body data is not
> necessarily HTTP protocol from the receiver.
> Given that there is no other obvious scope, can you change the docs to
> say "HTTP-equivalent body data" from the upstream source? That should
> allow then the comment about FTP listings.
OK. I modified a little the comment

> As a corollary what about gopher/whois listings, and other internal
> loaded/generated objects such as errors and icons?
Unfortunately the patch currently does not handle gopher and whois
protocols. I am not sure if it is useful, but if required, i think we
can include these protocols.

About the internal objects it logs a "-"

>
>
> * I've been a bit uncomfortable since the beginning about the
> de-chunking happening before the body bytes are accounted. This
> introduces some loss into the protocol accounting apparently being done.
> Are those lots bytes accounted for as headers? or is the total reply
> bytes value going to be larger than the sum of headers+body ?
> Do we stop at de-chunking? what about un-zipping? etc...

My opinion is that zipped and chunked data are not exactly the same.
Squid does not handle zipped and chunked data with the same way..

>
> It's your implementation but I'm of the opinion this should log the
> on-wire data size of the segment used for body. Including the chunking
> bytes, and excluding the Squid-added markup bytes on FTP/Gopher listings.

Maybe in the future we will implement a "<rbs" (raw body bytes received)
log formating code...

>
> Amos