Re: [PATCH] external_acl_type: add %EXT_LOG

On Wed, Mar 23, 2011 at 9:43 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On Wed, 23 Mar 2011 09:30:26 +1100, James Bowe wrote:
>>
>> On Wed, Mar 23, 2011 at 9:12 AM, Amos Jeffries <squid3_at_treenet.co.nz>
>> wrote:
>>>
>>> On Wed, 23 Mar 2011 00:38:08 +1100, James Bowe wrote:
>>>>
>>>> Attached is a patch for adding %EXT_LOG as a format specifier to
>>>> external_acl_type statements.
>>>> This is filled with the extacl_log field (log message set from
>>>> previous external acls).
>>>>
>>>> The patch only adds EXT_LOG, and not EXT_MESSAGE, since EXT_MESSAGE is
>>>> likely a longer user-oriented string.
>>>>
>>>> Cheers,
>>>> James Bowe
>>>
>>> +1. Looks fine.
>>> Can you explain the need for this though please? we already re-pass the
>>> tag= from one ACL test to the next. This log= is defined as being a
>>> message
>>> for the log file.
>>>
>>> Amos
>>>
>>>
>>
>> We use the message for the log file to tie a request with our own
>> record keeping and data structures. Creating these structures can be
>> an expensive process, and the data is needed in multiple external acls
>> (as well as when processing access log lines).
>>
>> I suppose a similar thing could be achieved if the unique sequence
>> number was available for external acls, but EXT_LOG is more flexible
>> as we have full control over that value.
>>
>> Cheers
>> James Bowe
>
>
> Thanks.
>
> Have you tried using tag= for this scenario?
> once set tag persists, whereas log= is replaced by each external ACL test
> (helpers which do not return one set it to empty).
>
> Amos
>
>

Yes tag= could also work for this purpose, although I don't see a
format specifier for tag in the external_acl_type docs either.

Although looking at the code in external_acl.cc:
        if (entry->log.size())
            ch->request->extacl_log = entry->log;
shows that the log= is only replaced if the external_acl returns a log message.

So
-for a string that never changes after it is set, tag= is suitable.
-for a string that may need updating or overwriting by a later
external_acl, log= is suitable.

Under both circumstances it is conceivable that later external_acls
may need access to the tag= or log= values after they have been set
(e.g. for external_acl debugging, merging log messages, etc). I have
updated the patch to add %TAG format specifier too.

Cheers,
James Bowe

P.S. Sorry for the earlier direct reply to Amos and not squid-dev,
lack of sleep.