Re: [PATCH] Enhanced access logging, added ICAP logging, chunked requests, bug #2495 fix.

Alex Rousskov wrote:
> Hello,
>
> Please consider the following changes for Squid3 trunk inclusion.
> Most of them have been accumulating on the 3p0-plus lp branch and have
> been tested in production. All have been tested in the lab. I compressed
> the 150KB patch, but the attached text file shows squid.conf
> documentation describing most of the features listed below.
>
> Many thanks to Christos Tsantilas for help with coding and testing.
>
> If approved, I will try to do a "bzr merge" instead of a raw patch,
> provided Robert helps me with convincing bzr to obey :-).

I note that there are a lot of different fixes going on here. Is it easy
to pull out these ones for separate merges?

  - Bug 2495
  - chunked requests
  - DNS timers and logging
  - ICAP logging changes

>
> Thank you,
>
> Alex.
> ------------------
> Enhanced access logging, added ICAP logging, chunked requests, bug #2495
> fix:
>
> Ported features accumulated on 3p0-plus branch as of r8937:
>
> - A hack to support chunked requests
> (see chunked_request_body_max_size in squid.conf).
>
> - Enhanced access logging
> (<Hs, <sh, >sh, <pt, <tt, icap_total_time, and <icap_last_h)
>
> - ICAP logging
> (see icap_log and log_icap in squid.conf as well as
> http://wiki.squid-cache.org/Features/AdaptationLog).
>
> - ICAP retries based on the ICAP responses status code
> (see icap_retry and icap_retry_limit in squid.conf).
>
>
> New v3.1-only changes:
>
> - Support logging resp. times of adaptation transactions to access.log
> (%adaptation_sum_trs and %adaptation_all_trs)
>
> - Support logging of total DNS wait time to access.log
> (%dt)
>
> - Bug #2495 fix
>
>
> Squid v3.1.0.9-based
> Contains official Squid v3.1 branch changes up to r9605.
>

I'm taking a closer look at this one in light of the logging alterations
that I have coming up.

Your comments suggest ICAP/eCAP are going to be done outside access.log
so the bits for them do not belong in AccessLogEntry. I know ALE is a
mix of uses already, but no need to add even more garbage there before
we fix that bug. Is it easy to make IcapLogEntry a main object in its
own files, possibly inheriting from AccessLogEntry if you need the ALE
fields in the icap.log? If not we shall have to do it after the fact in
the logging updates, but I would rather have it clean beforehand.

When splitting an existing code (Hs in this case) for inbound and
outbound. Please update them to use both directional < and > format
codes. The non-directional then needs to default to the old behavior
with a configu parse WARNING: about the change.

We appear to be fallign into a predictable pattern for LFT. May as well
keep that up...

These don't fit however. What is the difference now?

+ LFT_SENT_HTTP_CODE,
+ LFT_RECEIVED_HTTP_CODE,

These are ICAP headers right? but not with names 'Last-Matched-Icap:'
etc.? So can you make the ICAP ones all LFT_ICAP_*
- LFT_LAST_MATCHED_ICAP_HEADER,
- LFT_LAST_MATCHED_ICAP_HEADER_ELEM,
- LFT_LAST_MATCHED_ICAP_ALL_HEADERS,
+ LFT_ICAP_LAST_MATCHED_HEADER,
+ LFT_ICAP_LAST_MATCHED_HEADER_ELEM,
+ LFT_ICAP_LAST_MATCHED_ALL_HEADERS,

Is the tag form: %adaptation_ and %icap_* set in stone? There was a very
nice 'namespacing' proposition a while back which never seems to appear.
Using the ':' separator syntax like so --> %icap::<field>, or
%adapt::icap::<field>
The HTTP tags are all reserved char-codes. So if we are adding a new tag
format on top it might as well be a nice extensible one.

.. okay thats me to line 1400. More later.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE6 or 3.0.STABLE16
   Current Beta Squid 3.1.0.9