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

Added Feature to display Node Allocation through Gauge Chart #55

Merged
merged 3 commits into from
Sep 23, 2022

Conversation

Imads608
Copy link
Contributor

@Imads608 Imads608 commented Sep 22, 2022

Screenshot of the Feature below
Component can also display Gauge Charts for Memory and CPU if props are provided.

@javier-op / @dgkanatsios let me know if anything else needs to be done here :)

image

@javier-op
Copy link
Collaborator

javier-op commented Sep 22, 2022

Hey, the gauge looks great!
Some small comments, you can see them in the gif below:

  1. When you have multiple nodes, there is a shadow under the rows that leaves a blank
  2. When you hover over the row, the text moves due to the new "View metric" tag
  3. The tag itself is not clickable
  4. The metric is called Capacity, but new standby servers can be created by K8s until we reach the max value, so the metric that could be more useful to us would be Allocation % = (# active) / (# standby + # active )

node_gauge

@Imads608
Copy link
Contributor Author

Thanks Javier! I'll take a look and try to make those changes.

…and replaced capacity with allocation metric
@Imads608 Imads608 changed the title Added Feature to display Node Capacity through Gauge Chart Added Feature to display Node Allocation through Gauge Chart Sep 23, 2022
@Imads608
Copy link
Contributor Author

@javier-op , I've made the changes as you suggested.
I'm not sure if I understood your first point on the shadows under the rows when we have multiple nodes.
But I think the other two should be fixed.

Let me know your thoughts!

Copy link
Collaborator

@javier-op javier-op left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing :shipit:

@Imads608
Copy link
Contributor Author

Amazing :shipit:

Thanks! Could you merge?
Seems I don't have permissions haha

@javier-op javier-op merged commit d6bb300 into PlayFab:main Sep 23, 2022
@Imads608
Copy link
Contributor Author

Actually, sorry I think forgot to change the Gauge's configuration to be orange/red from 75-100% instead of 0-25%
That would make more sense to show that for Allocation.

What do you guys think @javier-op / @dgkanatsios

@dgkanatsios
Copy link
Collaborator

yeah, let's do it! also, any chance we can show all gauges for all Nodes at the same time? (definitely for a separate PR :D)

@javier-op
Copy link
Collaborator

Ah the colors of the gauge, yeah that would make more sense. Could you make a separate pr?

@Imads608
Copy link
Contributor Author

@javier-op yeah will do!
@dgkanatsios sounds like a great idea :D, maybe I can start to be an open-source contributor for the project instead of just the hackathon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants