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

Performing application of labels: no grouping allowed for "and" operation #2670

Closed
EdSchouten opened this Issue May 1, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@EdSchouten
Copy link
Contributor

EdSchouten commented May 1, 2017

[ This bug report applies to Prometheus 1.6.1 ]

Consider the following metrics, generated by Kubernetes/CAdvisor:

machine_cpu_cores{instance="hostname1",k8s_node_type="prod",...}
machine_cpu_cores{instance="hostname2",k8s_node_type="prod",...}
machine_cpu_cores{instance="hostname3",k8s_node_type="staging",...}
machine_cpu_cores{instance="hostname4",k8s_node_type="staging",...}

And these metrics, generated by kube-state-metrics:

kube_pod_container_requested_cpu_cores{node="hostname1",pod="pod1",...}
kube_pod_container_requested_cpu_cores{node="hostname1",pod="pod2",...}
kube_pod_container_requested_cpu_cores{node="hostname2",pod="pod3",...}
kube_pod_container_requested_cpu_cores{node="hostname2",pod="pod4",...}
kube_pod_container_requested_cpu_cores{node="hostname2",pod="pod5",...}
kube_pod_container_requested_cpu_cores{node="hostname4",pod="pod6",...}

The question is, how can one compute the CPU request utilisation per k8s_node_type? In this specific example, it's annoying that kube_pod_container_requested_cpu_cores does not have a k8s_node_type label. We somehow want to copy that label from machine_cpu_cores while aggregating. I came up with recording rules that look like this:

k8s_node_type:kube_pod_container_requested_cpu_cores:sum =
    sum(
        sum(label_replace(kube_pod_container_requested_cpu_cores, "instance", "$0", "node", ".*"))
        by (instance)
        + on (instance) group_right
        machine_cpu_cores * 0
    ) by (k8s_node_type)
k8s_node_type:machine_cpu_cores:sum =
    sum(machine_cpu_cores)
    by (k8s_node_type)
k8s_node_type:kube_pod_container_requested_cpu_cores:ratio =
    k8s_node_type:kube_pod_container_requested_cpu_cores:sum
    /
    k8s_node_type:machine_cpu_cores:sum

Though this recording rule works pretty well, I'm not happy with this part:

sum(a)
by (instance)
+ on (instance) group_right
b * 0

In this case I'm just looking for a binary operator that retains the left hand side; a ⨂ b = a. The addition and the multiplication by zero effectively emulate this, as I know machine_cpu_cores is never NaN.

Interestingly enough, Prometheus already has an operator that does a ⨂ b = a: it's called and. My question is, could we please extend and to also allow grouping? It would be nicer if I could write:

sum(a)
by (instance)
and on (instance) group_right
b

I understand that having grouping for the or operator is harder, as that is effectively a selection between the left hand side and the right hand side. The and operator, however, is semantically not very different from other operators like +, *, etc., right?

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented May 1, 2017

and and or already support grouping for vector matching, and due to their semantics handle many-to-many operations with no additional modifiers.

I don't see how this helps you though. Your last example would (after fixing the syntax error as group_right is unnecessary) would merely return the LHS.

May I suggest a feature request to k8 to have the kubelet expose allocated cores?

PromQL questions are best asked at https://groups.google.com/forum/#!aboutgroup/prometheus-users

@EdSchouten

This comment has been minimized.

Copy link
Contributor Author

EdSchouten commented May 1, 2017

Brian,

Sorry, but I don't really understand your answer here. First of all, why should I do a feature request at k8s to ask them to change the metrics format? The metrics format is all right as it is right now. In our case, Prometheus' Kubernetes service discovery adds the k8s_node_type label based on our custom relabelling rules. Requiring that kube-state-metrics also exposes metrics with those labels makes little sense.

Also your remark that grouping is already supported without any additional modifiers: doesn't the use case described above demonstrate that the existing mechanism is not sufficient? Right now and retains the value of the left-hand side, while also doing an implicit group_left. In this specific bug report, I'm providing an example of where we want to retain the left-hand side, while doing a group_right.

On a final note, why close this bug report already? If you provided a query that literally showed me that the existing query language is already capable of doing this, I'd agree that closing makes sense. Right now we haven't reached any consensus yet.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented May 1, 2017

First of all, why should I do a feature request at k8s to ask them to change the metrics format? The metrics format is all right as it is right now.

That you have to write such complex PromQL queries indicates otherwise.

Right now and retains the value of the left-hand side, while also doing an implicit group_left.

That's not how it works.

On a final note, why close this bug report already?

I see no behaviour here which indicates a bug in PromQL.

Our policy is to close the many user support questions we get and redirect them to the mailing list, so that everyone can benefit from the answer.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 23, 2019

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.

@lock lock bot locked and limited conversation to collaborators Mar 23, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.