invalid docker label with 0.16.2

[MikeMichel]

i scrape cadvisor with prometheus. 0.16.2 complains about "invalid label works.weave.role" which is set by the weave container itself. as a result the server is in state off. 0.16.1 works fine

[fabxc]

0.16.2 contains added validation logic in the upstream common/model package.
Label names with periods in them are indeed invalid. cAdvisor should not expose them on its Prometheus metrics endpoint in the first place (probably should be normalized by replacing . with _).

We had a discussion in the past whether we should allow periods because it allows unambiguous namespacing – unfortunately that didn't lead anywhere.

[MikeMichel]

where can i find this discussion? docker even recommends to use namespaces https://docs.docker.com/engine/userguide/labels-custom-metadata/#label-keys-namespaces

even when cadvisor would normalize it, i then need to query for "bla_blub_com" when i actualy was looking for "bla.blub.com"!? this somehow breaks the flow.

[jimmidyson]

Agree it's very annoying both for Docker labels & other systems that allow dots & slashes in labels, e.g. Kubernetes, that you want to bring over with your metrics & query in the same way as your actual resources.

[brian-brazil]

There are many many systems we integrate with that have a wide variety of characters that are standard for them, but not valid Prometheus label names or metric names. It's not sane to support everything.

This was never something we supported, and I'm surprised it wasn't noticed that cadvisor wasn't following the spec sooner.

[jimmidyson]

While I agree we shouldn't try to support every possible character, dots & slashes are pretty ubiquitous in label schemes. Supporting just those 2 extra chars would broaden support for the vast majority of systems.

[grobie]

For reference, here is the previous related discussion around that: #1178

The scope was a bit different to just extending the range of valid label key characters.

[beorn7]

@MikeMichel Note that you can put into a label value whatever you want, even emojis... So you might do something like {dockerlabel="bla.blub.com:whatever"}.

After a short look into cadvisor sources, it looks to me they are cheating with the metric descriptors, which is why the client library didn't detect the invalid label name. (It would have, if runtime checks would have been enabled.) In the new version of the client library, such checks will always happen.

We should really soon send a fix to cadvisor, otherwise many people will have problems.

[jimmidyson]

I'll be working on a fix for it in cadvisor in next few days if noone else gets there first.

Problem with {dockerlabel="bla.blub.com:whatever"} is handling multiple labels...

[beorn7]

Thanks @jimmidyson .

Possibilities for multiple labels (I'm not saying it's nice):

{dockerlabel1="bla.blub:foo", dockerlabel2="dings.bums:bar"}

{docker_name_1="bla.blub", docker_value_1="foo", docker_name_2="dings.bums", docker_value_2="bar"}

In practice, I'd probably flatten all special characters into "_" (with a safeguard against duplication, perhaps). I vaguely remember something similar from another monitoring system that has absolutely nothing to do with Prometheus. ;-)

To our defense: OpenTSDB has similarly strict rules for label values, too. With our UTF-8 label values, we thought, we were good for everything. Label names are in the domain of the PromQL syntax, so everything that might be an operator, is out of the question. (I shortly thought about optional quoting or escaping in label names, but that's probably a rabbit hole…) The "." (dot) operator in particular might serve us as a namespace separator (as "/" is already taken). Or Brian's old dream of the dot-job notation…

[fabxc]

OpenTSDB also has a maximum of 8 labels. Shortcomings of other systems are
not a valid reason to do the same.

The period to select the job does not conflict with allowing periods in
label names as far as I can tell.
While we are at it, neither would / since unquoted label names only occur
within curly braces.

On Thu, Jan 28, 2016, 11:26 PM Björn Rabenstein notifications@github.com
wrote:

Thanks @jimmidyson https://github.com/jimmidyson .

Possibilities for multiple labels (I'm not saying it's nice):

{dockerlabel1="bla.blub:foo", dockerlabel2="dings.bums:bar"}

{docker_name_1="bla.blub", docker_value_1="foo",
docker_name_2="dings.bums", docker_value_2="bar"}

In practice, I'd probably flatten all special characters into "_" (with a
safeguard against duplication, perhaps). I vaguely remember something
similar from another monitoring system that has absolutely nothing to do
with Prometheus. ;-)

To our defense: OpenTSDB has similarly strict rules for label values,
too. With our UTF-8 label values, we thought, we were good for everything.
Label names are in the domain of the PromQL syntax, so everything that
might be an operator, is out of the question. (I shortly thought about
optional quoting or escaping in label names, but that's probably a rabbit
hole…) The "." (dot) operator in particular might serve us as a namespace
separator (as "/" is already taken). Or Brian's old dream of the dot-job
notation…

—
Reply to this email directly or view it on GitHub
#1352 (comment)
.

[brian-brazil]

We've already had this discussion and agreed to leave it as-is, let's not go over it again. There's no new arguments here.

[beorn7]

google/cadvisor#1082 is already in the works. So nothing more to do here.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.