Bug 2037513: Fix dashboards having container_fs* metrices in queries #1554
Conversation
|
@slashpai: This pull request references Bugzilla bug 2037513, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/bugzilla refresh |
|
@slashpai: This pull request references Bugzilla bug 2037513, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Looks like d72bec8#diff-08daaad7f1c27b41ec86bee6cb6af0259066e38448b11e5389412c720813b255R25-R32 included more changes than expected |
2b55d7c to
d390c55
Compare
|
/bugzilla refresh |
|
@slashpai: This pull request references Bugzilla bug 2037513, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@slashpai: This pull request references Bugzilla bug 2037513, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
I will create a separate PR for kube-prometheus sync and generate to avoid confusion |
|
/hold I have created #1556 to update dependencies. I will rebase this branch once thats merged |
d390c55 to
61f355a
Compare
|
/hold cancel |
|
/retest |
|
cc: @arajkumar @philipgough PTAL when you get a chance |
|
/retest |
|
/hold Testing this change locally |
|
Had added deviceDeviceSelector in container_fs* queries as part of kubernetes-mixin PR since diskDeviceSelector includes only node_exporter label values figuring out what all need to be updated to make it work for cadvisor label values as well Because dashboard still gives no datapoints as there is no matching regex for diskDeviceSelector |
Had added deviceDeviceSelector in container_fs* queries as part of kubernetes-monitoring/kubernetes-mixin#737 since diskDeviceSelector includes only node_exporter label values updated disDeviceSelector to include cadvisor devices too (/dev)
ff7c393 to
9981ce6
Compare
Since Prometheus doesn't collect the per-container fs metrics dropping this dashboard instead of showing no datapoints Signed-off-by: Jayapriya Pai <janantha@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: raptorsun, slashpai The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
|
/skip |
|
/retest |
|
@slashpai: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest |
|
@slashpai: All pull requests linked via external trackers have merged: Bugzilla bug 2037513 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@juzhao we have dropped two rows in Kubernetes / Compute Resources / Pod" 'Storage IO - Distribution(Containers)' and 'Storage IO - Distribution' |
|
/cherry-pick 4.10 |
|
@slashpai: cannot checkout In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-4.10 |
|
@slashpai: #1554 failed to apply on top of branch "release-4.10": In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
container_fs_.* metrics doesn't have "container" label after
/pull/1402 which caused regression
for dashboards using "container" label since it gives empty datapoints
for all queries using "container" label in queries containing
container_fs_.* metrices.
Inorder to fix the dashboards added containerfsSelector in
kubernetes-monitoring/kubernetes-mixin/pull/737 and overrided the containerfsSelector
default value from container!="" to harmless id!="" in this commit.
We really don't need that selector since we dropped the container label
in OCP for container_fs_.* metrices but we need to override the
selector for the queries involving container_fs_.* to work. Dropped the dashboard
"Kubernetes / Compute Resources / Pod" since Prometheus doesn't collect the
per-container fs metrics (discussed in #1685 (comment))
Had added deviceDeviceSelector in container_fs* queries as part of
kubernetes-monitoring/kubernetes-mixin#737 since diskDeviceSelector
includes only node_exporter label values updated disDeviceSelector
to include cadvisor devices too (/dev)
More Details:
Bugzilla Comment 9
Bugzilla Comment 10
cc: @arajkumar @philipgough