[PATCH] Log all transactions

On 06/28/2010 05:54 PM, Amos Jeffries wrote:
> On Mon, 28 Jun 2010 12:17:16 -0600, Alex Rousskov wrote:
>
>> What is the motivation behind excluding HTTP transactions with
>> LOG_TAG_NONE logType and zero out.size?
>
> Maybe protecting the access.log from SYN-floods of ICP spam.
>
>>
>>> void
>>> ClientHttpRequest::logRequest()
>>> {
>>> if (out.size || logType) {
>>
>> This guard might keep valuable information from the cache administrator.
>> Should we work on removing the guard?
>
> +1.
> I suspect it will result in uncovering some error pathways which can be
> short-circuited a lot earlier than logging. If not then it's probably
> useless.

The attached patch makes Squid log all transactions, including those
with LOG_TAG_NONE logType and zero out.size. Excluding those
transactions hides valuable information.

For example, it hides certain aborted client transactions which
sometimes indicate Squid bugs or configuration problems that the
administrator can fix. It also leads to wrong statistics reporting if
the reporting tools are based on access log information.

If ICP spam is a concern here, it should be dealt with using ACLs or
other protection mechanisms rather than being blissfully ignored, I guess.

This patch has been tested in production environments.

I left the "if (true)" guard to prevent indentation changes to the
pretty long logRequest method, making the patch more difficult to
understand. I will remove that otherwise useless guard during commit
unless you think it should stay.

Thank you,

Alex.