Prometheus metrics capture query was incorrect on OCPv4

[agrare]

The prometheus metrics capture option was building a query that didn't return any data:

Metrics missing: [ContainerNode(1)] [#<ManageIQ::Providers::Kubernetes::ContainerManager::ContainerNode:0x00005622f0cbab88> crc-dv9sm-master-0] No data in response

The query string that was being built was incorrect: pod_name instead of pod, container_name instead of container, and the wrong job.

[chessbyte]

@agrare will this work on OCPv3 as well?

[agrare]

I'm not sure, it depends on when this changed on the openshift side. Was it while Prometheus was tech preview on v3.x? Then yes. If it was ok v4 then probably not.

If we need to split this into v3 and v4 Prometheus modules that'll take some more time.

[cben]

This is the source of all the differences - you're getting data from a different source.
There is definitely a cadvisor running in v4, but not sure about data. I'd like to look more into this.

[agrare]

Hm I don't see any service other than kubelet which provides node metrics e.g. id="/" and for e.g. pod / container metrics if I add namespace= kubelet is the only job I see.

[chessbyte]

If we need to split this into v3 and v4 Prometheus modules that'll take some more time.

Prefer to have v4 working first. Then we can add back v3 support, if needed.

[cben]

If different queries are necessary, it's likely possible to craft a PromQL query that will work with either data without Ruby changes.

P.S. It's a shame there is no easy way to capture raw Prometheus data for tests and just evaluate varying PromQL on it. Then we could answer similar questions without a 3.11 cluster, and generally could test metrics better...
Prometheus has infrastructure for this in its own PromQL tests, but it's not conveniently exposed AFAIK.

[agrare]

I got access to a v3.11 cluster and it has the same bug as OCP v4 so it seems this fix would also fix v3.11

Actually it looks like the job="kubelet" but there is still pod_name instead of pod

If you know of a way to support both (all three?) of these that would be greatly appreciated

[chessbyte]

@cben does this look good to merge to support v4 and we can look at getting v3 supported later, if needed?

[cben]

Sure, if you're good with that 👍

Prometheus on 3.x was tech preview, I don't know how much effort it's worth to keep it working.

[oourfali]

Tech preview indeed. And not something we support or plan to support.

…

On Sun, Jun 7, 2020, 09:42 Beni Cherniavsky-Paskin ***@***.***> wrote: Sure, if you're good with that 👍 Prometheus on 3.x was tech preview, I don't know how much effort it's worth to keep it working. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#381 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGLGAIUMPG24ULAKWIWONDRVMZFTANCNFSM4NSZDVKA> .

[simaishi]

Jansa backport details:

$ git log -1
commit 93e0dd6e4d25b050d3edf4d128bfd7e27eec9496
Author: Oleg Barenboim <[email protected]>
Date:   Sun Jun 7 09:45:03 2020 -0400

    Merge pull request #381 from agrare/fix_prometheus_metrics_capture

    Prometheus metrics capture query was incorrect on OCPv4

    (cherry picked from commit 0c9dc8452e674a01f9ec7b060a62fb9c02089b3a)