access log: add DYNAMIC_METADATA_UNQUOTED formatter

[rgs1]

This is useful when you are using the header-to-metadata filter to clean
up headers and then you want to send that to the access log as plain, non
quoted, text.

Signed-off-by: Raul Gutierrez Segales [email protected]

[rgs1]

Per @fishcakez , DYNAMIC_METADATA_TRIM() is possibly a better name for this formatter. Thoughts?

[rgs1]

Per @fishcakez , DYNAMIC_METADATA_TRIM() is possibly a better name for this formatter. Thoughts?

FWIW, I prefer DYNAMIC_METADATA_UNQUOTED() since we are already using it like that :-)

[zuercher]

I have to say I'm leery of this change. Feels like we're in this perpetual state of adding more and more little tweeks for this format or that.

[rgs1]

I have to say I'm leery of this change. Feels like we're in this perpetual state of adding more and more little tweeks for this format or that.

I know. I am also not super excited about having to add this, but I am not sure what other options we have.

There's no way to add internal formatters and the cleanest way for us to scrub/clean headers that needed to be logged is:

• header-to-metadata -> apply regex
• DYNAMIC_METADATA

but the latter isn't happy with old school logging (e.g.: plain)....

[zuercher]

I think it should be possible to allow extensions to provide their own formatters. But maybe that's one extension type too many.

[rgs1]

@zuercher so shall we let this one land or do we want to explore the extensions path?

[mattklein123]

FWIW we already have an extension point for access log filters. Having one for formatters seems like it might be worth it also.

[zuercher]

The only problem I have with landing it that it's forever. I think I'd prefer some change to the parsing of DYNAMIC_METADATA(NAMESPACE:KEY) that triggered quote stripping.

[rgs1]

The only problem I have with landing it that it's forever. I think I'd prefer some change to the parsing of DYNAMIC_METADATA(NAMESPACE:KEY) that triggered quote stripping.

Alas, I don't think it would be pretty because:

• namespace should have reverse DNS format [0], but we don't really enforce that so we can't assume anything
• key is really free form, so same thing

which means we can't -- cleanly -- add more parameters within the parentheses.

After the final ), we must have the max length param. We could make it such that 0 means unset, so
if you wanted an extra param -- without setting max length -- you'd end with:

DYNAMIC_METADATA(envoy.lb:foo.bar):0,true

to get what we want.... which is a bit yucky.

Hmm. Thoughts?

[0] https://github.com/envoyproxy/envoy/blob/master/include/envoy/network/filter.h#L276

[rgs1]

FWIW we already have an extension point for access log filters. Having one for formatters seems like it might be worth it also.

Sounds like this would be the way forward then, given my above comment about extending DYNAMIC_METADATA()...

[zuercher]

I think moving to extensions would be the best thing.

For the suffix to control output, you could have a further :-separated list of things. So :10:strip would limit length to 10 and strip quotes or just :strip would strip quotes without affecting the length.

[rgs1]

Closing this in favor of #13411.