Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a helper to fill missing labels #355

Closed
beorn7 opened this issue Oct 4, 2017 · 4 comments
Closed

Create a helper to fill missing labels #355

beorn7 opened this issue Oct 4, 2017 · 4 comments
Assignees
Milestone

Comments

@beorn7
Copy link
Member

beorn7 commented Oct 4, 2017

When mirroring external label sets into Prometheus (prime example are Kubernetes labels), the situation arises that those label sets don't fulfill the Prometheus semantics of constant dimensionality, i.e. the set of label names of the same metrics is not always the same.

Example: kube_pod_labels, a metric exported by kube_state_metrics, carries the Kubernetes labels of each pod, each prefixed by label_, as can be seen in this real-life example:

# HELP kube_pod_labels Kubernetes labels converted to Prometheus labels.
# TYPE kube_pod_labels gauge
kube_pod_labels{label_component="dns-forwarder",label_env="production",label_pod_template_generation="10",label_system="k8s",label_version="1360-71-ceae78f",namespace="k8s",pod="k8s-dns-forwarder-0ggrs"} 1

Pod labels can be arbitrary. So another pod can have a label label_foo="bar". This breaks the Prometheus requirement for consistent dimensionality. The Prometheus server ingests inconsistent metrics just fine. It essentially pretends the missing labels exist but with an empty label value. (That's the semantics PromQL deals with it.)

However, the registry implementation in the Go client disallows inconsistencies. Both the client library and Prometheus are following the robustness principle here.

To keep it that way, missing labels needed to be added (with empty strings as label values). This can happen in the user code using the client library. The library can provide tooling to make that task easier, the exact form of which needs to be determined. It could be helper functions, a Collector wrapper or a special Registry.

@simonpasquier
Copy link
Member

@beorn7 Is this issue still relevant? I can give it a try but I want to make sure beforehand there is a consensus on it.

Thanks to google/cadvisor#1831, cAdvisor now provides consistent label names for the same metric but other applications (eg kube-state-metrics) have modified their Prometheus client to overcome the situation (see kubernetes/kube-state-metrics#192).

@brian-brazil
Copy link
Contributor

Yes, the issue is still relevant (though less urgent). Cadvisor and the associated Kubernetes code have been fixed, however kube-state-metrics only has a hacky workaround in place. There are 1-2 other places where this has also come up.

Where there is disagreement is on the implementation. I believe this should be a wrapper around a Collector or some form of helper function, whereas last I checked @beorn7 was for a new type of lenient registry.

@beorn7
Copy link
Member Author

beorn7 commented Dec 1, 2017

I have an idea for a solution that hits two birds with one stone. I'd like to explain, but implementing it would probably be faster. It's on my list of things to do ASAP. (Just that that P is quite a limitation...)

@beorn7
Copy link
Member Author

beorn7 commented Jun 7, 2018

Things have changed here in the meantime. With OpenMetrics explicitly allowing inconsistent label dimensions in expositions, it should also be allowed in client_golang. We have decided to keep the checks at registration time so that normal direct instrumentation is still ensuring label consistency. However, if the Collect call returns metrics with inconsistent label dimensions, this is fine from now on unless you use a PedanticRegistry (and even that will be possible cleanly with #47 fixed, which I will tackle now for real and really really soon…).

Sorry @matthiasr and @brancz for making you jump through hoops in kube-state-metrics and the statsd_exporter. At least we have defined everything clearly now and (hopefully) made the right call finally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants