Add additional metrics to PVC Page #5763
Conversation
openshift-ci-robot
added
do-not-merge/work-in-progress
bipuladh
force-pushed
the
pvc_updates
branch
2 times, most recently
from
7b9db50
to
a44ae1b
Compare
bipuladh
force-pushed
the
pvc_updates
branch
2 times, most recently
from
079c726
to
ed1e4d3
Compare
bipuladh
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
| const query = _.template('kubelet_volume_stats_used_bytes${name}'); | ||
| return name ? query({ name: `{persistentvolumeclaim='${name}'}` }) : query(); |
There was a problem hiding this comment.
@anmolsachan please review this query. Thanks.
|
@openshift/team-ux-review adding some metrics for PVC based on an old story. PTAL. |
|
/assign @TheRealJon |
Oops! changed variable name and dint update it. My bad. |
|
Donut grows to fill entire available horizontal space:
|
|
the designs you shared looks great @bipuladh. I think I need to sync up with the OCP team next week to see if they are ok with that. I think the behavior should be that in large screens we are showing it like that and in small screens, the donut chart will be above the content on the left side. we have a meeting next week with the OCP team so we can sync there. |
| // @ts-ignore | ||
| import { useDispatch } from 'react-redux'; | ||
| import { sortable } from '@patternfly/react-table'; | ||
| import { ChartDonut } from '@patternfly/react-charts'; |
There was a problem hiding this comment.
So, we have a collection of prometheus driven graphs in frontend/public/components/graphs. There is currently no donut chart defined there, but I feel like this would be a good time to create one and use it here. That way it will be reusable if future components need to use a donut chart. There is a gauge component there that is essentially a special donut case, so you could use that component as a starting point.
There was a problem hiding this comment.
I can take it as a follow-up PR.
|
Also, I am still seeing the issue with the chart size increasing to fill the entire space on the right. |
@TheRealJon do we really need to listen to the window's size? I don't think a user would resize their window IMO. The current approach I have taken only runs the poll if the initial width is greater than a certain threshold. If she later increases the size of the window then she would have to refresh. |
That's fair, I think it would definitely be an edge case, but what if initial width is smaller than 1450, then the window gets maximized, put in full screen, or moved to another display with a larger resolution? Then the used capacity column would be shown, and no data would ever populate unless they refresh. |
@TheRealJon I cannot reproduce it on my setup are you sure you have the latest commit? |
bipuladh
force-pushed
the
pvc_updates
branch
2 times, most recently
from
7facd71
to
985df15
Compare
There was a problem hiding this comment.
/lgtm
/approve
This isn't a new problem, but when adding the "GiB" units inside the donut, it becomes really crowded. I think that if we had a triple digit value, it would probably overflow. After some discussion, I think the fix might be outside the scope of this PR, since it is one of many consistency problems we've had with donuts. IMO, we probably need to create a common component to apply consistent styling and usage.
Created https://issues.redhat.com/browse/CONSOLE-2305 to track.
Also, minor nit that could be fixed in a follow-on: There should probably be a 20px margin below the donut chart to match the margin below the heading.
|
/lgtm |
|
/approve |
openshift-ci-robot
added
the
approved
Introduces Donut Chart in Details Page Introduces Used Capacity in List Page
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bipuladh, spadgett, TheRealJon 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 |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
Introduces Donut Chart in Details Page
Introduces Used Capacity in List Page
Moving PVC Metrics from
Mi/Gi/TitoMiB/GiB/TiBAdding missing items from https://issues.redhat.com/browse/KNIP-638