cadvisor assigns labels to prometheus metrics with empty values

[jaloren]

when cadvisor, exports metrics for docker containers, there is a root cgroup (/) and cgroup for a docker container (/docker/uuid). If docker container has a label on it, then this label is applied to all containers including the root container. Because some containers don't have that label, the label will have an empty value.

For example, lets say I have a python flask app in a docker container and I have labelled it in docker with the label python_version and the value 3.6. The root container will also get this metric label except it will be python_version=""

It looks like the cause of this is the code here. It loops through all the containers, getting labels for each container and then combines all of those labels into a single slice.

https://github.com/google/cadvisor/blob/master/metrics/prometheus.go#L853

I can't think of any valid usecase in which a user would want labels with no values, so I am wondering if its a bug that a label with an empty value is being applied to the metric.

[dashpole]

prometheus does not allow sending a metric with the same name, but different labels. So we had to "fake" it by adding empty label values based on the set of all labels for a given metric.

[jaloren]

Given those constraints that makes sense.

Out of curiosity, has anyone considered what it would look if cadvisor supported the ability to embedded the container name in the metric itself? This would then allow a user to avoid having empty labels like this.

[dashpole]

By "in the metric" I presume you mean in the metric name? I think this goes against the prometheus model for metrics. Labels are a big reason why prometheus became popular. Older metric systems used to make the naming of the metric contain information about where it came from: e.g. you would have a metric named: cluster_a.namespace_b.pod_c.container_d.memory_usage. You end up with a tree of metrics. But this confines you to a single hierarchy. Someone who comes along and wants an infrastructure based tree would want cluster_a.node_b.pod_c.container_d.memory_usage instead, so they can aggregate by node, rather than namespace. But you can only get one of the two with any tree. Labels allow arbitrary groupings, and can meet many different needs without conforming to any hierarchy.

[jaloren]

Fair point. Okay, let me take this from a different angle.

Is there a way to filter what docker labels are turned into prometheus labels or alternatively simply turn "convert docker labels into prom labels" off? I use a large number of docker labels that vary a lot across images so the set of labels will be enormous, where most of the labels will be empty.

In my case, labels provide a useful metadata set for a variety of reasons outside of prometheus and many of them are of questionable value as a prometheus label.

In other words, could this func get some filtering options?

func DefaultContainerLabels(container *info.ContainerInfo) map[string]string {}

PS I'd be happy to submit a PR along those lines if this something the project would consider accepting.

[dashpole]

Sorry for the slow response, but this is definitely something we should tackle.

[dashpole]

I'm happy to review a PR that allows either label whitelisting, or a boolean for not including docker labels as prometheus labels.

[jaloren]

@dashpole done. Note that I defaulted the flag to true since that's the current behavior so this won't be a breaking change. On the other hand, I suspect my scenario is the norm and that people would actually want to opt into this behavior but that's just a gut feeling backed by no empirical data.

[dashpole]

Sorry, I should have updated here. I went ahead and implemented label whitelisting yesterday: https://github.com/google/cadvisor/pull/1981. Maybe label whitelisting is overkill... Ill take a look later today.

[dashpole]

Let me know what your opinion is too @jaloren

[jaloren]

@dashpole Hmmm, that's interesting. I am leaning towards overkill since its unclear to me at least whether this functionality would be useful to docker containers. On the other hand, cadvisor is for control groups and thus covers a broader range of use cases outside of docker so perhaps it makes sense in that case? Either way, It gives me the ability to turn this functionality off which is all that I was looking for so either sounds fine to me.

[jaloren]

To avoid an XY problem, I will throw out this idea. For docker containers at least, the current label exporting behavior is sub-optimal because it forces all containers on a system to get labels from every other container. So even with whitelisting, it doesn't address the problem of wanting to export a metric for one container but not another. And as you explained, this is was done to deal with the design of prometheus.

But what if cadvisor could actually open a port for each container such that each container metric list is a separate endpoint + whitelisting? Then, labels of one container would have no impact on another when exporting metrics. To deal with the service discovery problem, cadvisor could publish a GET rest endpoint that allowed a client to download a JSON document that contains a metric port mapping for each container. A tool could then use the JSON doc to register the service endpoint into a service discovery service eg consul.

Understood this would be a much more significant change and increased complexity so might be undesireable for that reason but I thought I'd at least ask even if the answer is no. :)

[dashpole]

Dealing with lots of endpoints seems like a harder task for users than dealing with empty labels IMO.

My thinking was that if users wanted a particular docker label that is relevant for their metrics (e.g. "app:frontend" vs "app:backend"), and exists across all containers, it may be useful to be able to include that as well so they can use labels to do more useful aggregation. But as no one has asked for that capability yet, I am inclined to omit it for now.

[jaloren]

@dashpole fair enough whitelabel change or my PR. Either sounds find to me.

[carlosflorencio]

Hi,

Sorry for creating entropy here, but what's the problem of having metrics with empty labels? Prometheus seems to drop them at ingestion time, so no issue there.