Re: [PATCH] log virgin HTTP request headers

On Tue, 19 Jan 2010 17:48:20 -0700, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 01/14/2010 03:28 PM, Tsantilas Christos wrote:
>> Amos Jeffries wrote:
>>> Tsantilas Christos wrote:
>>>>> Tsantilas Christos wrote:
>>>>>> Hi all,
>>>>>> This patch adds a new format code which allow the user to log HTTP
>>>>>> request header or header fields before they are adapted.
>>>>>> The existing "http::>h" format code logs HTTP request headers after
>>>>>> adaptation.
>>>>>> The new format code is the "http::>hv".
>>>>>>
>>>>>> This is a Measurement Factory project.
>>>>>>
>>>>>> Regards,
>>>>>> Christos
>>>>>>
>>>>> Um, I would think this makes more sense done the other way around.
>>>>>
>>>>> With the default >h displaying the virgin headers received from the
>>>>> client and some other code ( >ha ?) for the adapted headers. Be it
>>>>> adaptation or headers_access doing the alteration.
>>>>
>>>> The only objection I have is that the >h is already implemented to
log
>>>> headers after adaptation for squid3.0 and squid3.1
>>>
>>> I know. However its documented as merely "request header" with
>>> indication that it's adapted first. It's historic from squid-2 where
>>> no adaptation happened to them. So IMO the fact that it displays the
>>> adapted header is kind of a regression.
>>>
>>>>
>>>> I did not have in my mind the headers_access, so in the patch the
new
>>>> format code activated only if the adaptation is active.
>>>> Should the http::>hv (or http::>ha) be always active?
>>>
>>> Yes. IMO always active.
>>
>> At the end this is not so simple.
>> When we are doing adaptation we are replacing the old HttpRequest
object
>> with a new one. Logging virgin and adapted HttpRequest is simple and
>> does not have a Huge cost, just do not release the virgin HttpRequest
>> object.
>>
>> But the header_access just removes the requested headers from the
virgin
>> HttpRequest object. Requires a different approach. Maybe just make a
>> clone of the HttpRequest is enough, but does it worth the extra cost?
>
> Can we clone once, before header_access and before adaptations? That
> cloned pristine value is what should be logged with one of the >h*
> options. The header value used for the final request to the server
> should be used for the other >h* option.
>
> Personally, I do not care which >h* option is the default. All we need
> to do is to provide a way to log from-the-client and to-the-server HTTP
> request headers.
>
> Thank you,
>
> Alex.

I don't see why not.

Christos:
 It seems to me the place to add a clone is client_side_request.cc line
1263, between http_access testing and the first pre-cache adaptation.

IIRC, the clone mechanism used to create the Squid->Server request is
where header_replace is done. Much later in the processing.

Amos
Received on Wed Jan 20 2010 - 01:47:42 MST