Skip to content

Discovery/Kubernetes: If target is Node, attach labels of it.#3748

Closed
cofyc wants to merge 1 commit intoprometheus:masterfrom
cofyc:support_node_target
Closed

Discovery/Kubernetes: If target is Node, attach labels of it.#3748
cofyc wants to merge 1 commit intoprometheus:masterfrom
cofyc:support_node_target

Conversation

@cofyc
Copy link
Contributor

@cofyc cofyc commented Jan 26, 2018

What this PR does

For all targets backed by a node, all labels of the role: node
discovery are attached.

Why we need it

We are currently using prometheus-operator to scrape metrics from kubelet. prometheus-operator maintains a endpoint object (kubelet by default), e.g:

apiVersion: v1
kind: Endpoints
metadata:
  labels:
    k8s-app: kubelet
  name: kubelet
  namespace: kube-system
subsets:
- addresses:
  - ip: 192.168.128.116
    targetRef:
      kind: Node
      name: nodea
      uid: 1c75a366-1e85-11e7-bacc-f23c9184ec92
  - ip: 192.168.130.217
    targetRef:
      kind: Node
      name: nodeb
      uid: 5165df10-888b-11e7-83d7-f23c9184ec92
  ports:
  - name: http-metrics
    port: 10255
    protocol: TCP
  - name: cadvisor
    port: 4194
    protocol: TCP
  - name: https-metrics
    port: 10250
    protocol: TC

In current prometheus, if kubernetes discovery role is endpoint, it does not attach labels of role: node. Metrics scraped from kubelet on each node does not contain node labels. In many cases, we need to do vector matching with others metrics (e.g. kube_node_labels from kube-state-metrics). But without node name label, it's hard to do.

IMHO, it will be convenient, if targets backed by a node, all labels of the role: node
discovery are attached, like Pod target.

@cofyc
Discovery/Kubernetes: If target is Node, attach labels of it.
38aa978 
For all targets backed by a node, all labels of the `role: node`
discovery are attached.
@cofyc
Copy link
Contributor Author

cofyc commented Jan 27, 2018

cc @brian-brazil @fabxc @brancz

cofyc added a commit to cofyc/prometheus that referenced this pull request Jan 29, 2018
@cofyc
Sync from prometheus#3748.
2882075
cofyc added a commit to kirk-enterprise/prometheus that referenced this pull request Jan 29, 2018
@cofyc
Merge pull request #3 from cofyc/ke-release-1.8.2
8894b1f 
Sync from prometheus#3748.
@brancz
Copy link
Member

brancz commented Jan 30, 2018

I don't think the entire node labels are necessary. A generic target kind and name would be the correct mapping to Prometheus meta labels I think.

@brian-brazil
Copy link
Contributor

brian-brazil commented Jan 30, 2018

I'm a big confused by this, can you share the prometheus.yml you are using?

@cofyc
Copy link
Contributor Author

cofyc commented Jan 31, 2018 •
edited
Loading

@brian-brazil
Here: https://pastebin.com/u6kj0x1t, it is generated and managed by prometheus-operator.

In short, we want to vector matching on metrics from kubelet and kube-state-metrics. But if kubelet target are discoveried through endpoints role (in prometheus-operator, all ServiceMonitor are backed by endpoints), no node infos are attached. So I try to attach node labels if endpoint target is backed by node, like Pod target.

@brancz That would be much simpler! I created a separate PR to address this: #3770

@cofyc cofyc mentioned this pull request Jan 31, 2018
Merged
@brancz
Copy link
Member

brancz commented Feb 1, 2018

Got it, that makes sense. As an explanation why the nodes are in the Endpoints object, is because it basically simulates self-hosted kubelets, until we actually have them self-hosted.

@cofyc cofyc closed this Mar 25, 2018
This was referenced Apr 23, 2018
Closed
Closed
@cofyc cofyc mentioned this pull request Jun 5, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cofyc @brancz @brian-brazil