UTF8: Content negotiation (encoding side)

[ywwg]

This PR builds on #537 by adding content negotation during scraping.

Scrapers can announce that they support utf-8 names by adding "escaping=allow-utf-8" in the Content-Type header. In cases where utf8 is not available, metric providers can be configured to escape names in a few different ways: values (U__ utf value escaping for perfect round-tripping), underscores (all invalid chars become _), dots (dots become _dot_, _ becomes __, all other values become __). Escaping can either be a global default (model.NameEscapingScheme) or can also be specific in Content-Type with the "escaping=" term, which can be "allow-utf-8" (for UTF8-compatible), "underscores", "dots", or "values".

This is still a noop for existing configurations because scrapers will not be passing the escaping key in the Content-Type header. Existing functionality is maintained.

[ywwg]

This will be squashed :)

[ywwg]

this specifies a default escaping scheme when none other is specified. not sure if this is the right thing to do

[beorn7]

General direction looks OK. I will only manage to do a code level review next week.

Some broad thoughts:

First the versioning. See the individual comment below. It feels weird to pretend to support a version that doesn't exist yet. (But as said, I'm not an expert and I don't know how this is usually done.)

Another gut feeling: validchars and escaping never show up together. Maybe they can be merged into one option? escaping=none could take the role of validchars=utf8, and escaping=values is the default. (Maybe that default should be part of the (upcoming) standard and hardwired in the code. It feels weird that the default is set via a package variable and that would change the behavior for the same accept header.)

[beorn7]

I guess for the classic text format, anything goes because it hasn't really been formally specified, but here I would definitively add a comment that this doesn't imply that we are really implementing OM v2 from the future.

On a more general note, I'm wondering if pretending we are supporting a version of the standard that doesn't exist yet is the right way to go. I'm not an expert in those content-type shenanigans. Maybe we should add some "alpha" or "pre" in here?

[beorn7]

Another thought: Maybe we don't need to change version numbers at all right now. Any escaping that is not none will produce valid classic text format or OMv1. So we could just document the escaping=none (currently validchars=utf8, see comment above) as an experimental feature that creates a slightly different format, that will be part of a future standard. We can later decide if we want to keep supporting the "illegal" combination of OMv1 and escaping=none, but that's still better than asking clients to negotiate OMv2 although OMv2 will certainly look much differently once it really exists.

[ywwg]

I like the idea of using escaping to also select for utf8 mode. maybe instead of "none" we call it escaping=utf8-ok or something like that, to make it more explicit?

[ywwg]

can't remember if dashes are allowed in content-type parameters though

[beorn7]

Yes, let's do that. I.e. only one parameter escaping that defines the behavior (missing = old behavior, any escaping schema = do the escaping, allow-utf-8 or something = use the new format).

I would also say, for now, that we don't change the version numbers but "tolerate" that the client can ask for allow-utf-8 and we pragmatically create an exposition format that is technically invalid. Just add a comment in the code that says that this is just for experimentation and that there will be new versions of the expositions formats eventually that properly describe the new syntax and should be selected whenever UTF-8 names are requested.

And yes, dashes are allowed in the content type, we use it for charset=utf-8 and encoding=compact-text.

The former reminds me that we should try to be consistent and spell UTF-8 as UTF-8 where possible.

[beorn7]

The Go test errors seem to have to do with Go modules. Probably need to run go mod tidy or something.

Could you fix that and work in the ideas above? I think the code otherwise is technically fine. I'll do a final review once that's done.

[ywwg]

notes should all be addressed

[ywwg]

oh except for adding comments

[beorn7]

Looks good. I assume you will squash the commits yourself and then take care of DCO signing.

[ywwg]

squashed

[beorn7]

Thank you very much.