Re: [PATCH] meta_header option

On 10/12/2012 01:06 AM, Amos Jeffries wrote:
>>> I got a request to implement stacked tags in external ACL the other day.
>>> It would seem they are suited well for combining with this feature. How
>>> about "tag" or "meta_tag" ?
>> I assume stacked tags are key:value pairs. Today, tags are essentially
>> anonymous or valueless annotations (values without keys or keys without
>> values). Please correct me if I am wrong.
>
> You are correct. I consider them as a key of "tag" and a value since
> they arrive from the helper as "tag=X".

"tag":value is a good way to map current tags provided we want to
support having two distinct key:value pairs with the same key. The
proposed code supports that.

> The no-replace semantics from
> secondary helpers or tags given is a small problem, which we will need
> to work around. But that can remain isolated to external ACL code for
> now until people are used to the.
> I was going to use these meta-X items as stacked tags and allow the
> admin to pick their own arbitrary key name.

If I understand the "stacked tags" feature correctly, we would need to
add something like this to the external acl keywords support:

    note=<key>:<value>

and leave the old "tag=value" as a deprecated way of saying

    note=tag:<value>

There should be some limits on what keys can be, of course. For example,
they must not contain HTTP delimiters.

Note that there is nothing "stacked" about these annotations in the
proposed implementation. They are treated as HTTP extension #value
headers with a "comma-separated list" value syntax (with key being the
header name), essentially. For a given key, the values are listed in the
arrival order. There is no specific order among different keys.

Unlike HTTP #value headers, key:value duplicates are ignored.

>> The reason I did not include "tag" in the above names is because I was
>> worried that preserving compatibility with the existing tags will create
>> a mess. Here are a few potentially messy areas:
>
> I'm not so worried. Elsewhere the concept of tagging things is common.
> Squid is the weird case where we only allow one tag per transaction (so
> far).

Oh, I do think that the annotation/tagging concept itself is very
useful! I am just worried (more than you are) about us trying to mix the
new and old features, each having its own little nuances that may not
mash together in corner cases.

> Just had another idea. "key" or "keys".

Since each annotation contains both a key and a value, it seems wrong to
name the whole concept "key".

>> Overall, I agree that it would be nice to have one name/concept for all
>> annotations, but reusing "tag"s may create a mess that will either
>> significantly delay the new feature acceptance or will cause
>> upgrade problems. It may be better to leave "tag"s to old configs and
>> standardize on new "note"s going forward (e.g., add them to external
>> ACLs). What do you think?
>
> I think its not as bad as you think. But agree to avoid it for now.

OK, so here is the summary of the required changes for the proposed
meta_header patch:

  1. Rename proposed "meta_header" directive to "note".
  2. Rename proposed "%meta_header" logformat code to "%note".
  3. Enforce restrictions on characters in keys if not already
     enforced.

Going forward, if somebody wants to support "stacked tags" using
external ACLs, they will:

  1. Add a "note=key:value" keyword support to external ACLs
  2. Add a new "note" ACL type that will match key:value annotations.
  3. Adjust external ACL code to store tags as tag:<value> annotations,
     using annotations API added by the meta_headers patch.
  4. Adjust "tag" ACL type code to look for tag:<value> annotations,
     using annotations API added by the meta_headers patch.
  5. Make sure that external ACL annotations are not sent to adaptation
     services (enabling such sending may be a separate project).

Any objections to this plan?

Thank you,

Alex.
Received on Fri Oct 12 2012 - 16:00:42 MDT