-
Notifications
You must be signed in to change notification settings - Fork 382
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
feat(k8s-views-global): Windows node support #103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jkroepke ,
Just did a static review, still need to test this on my side.
Here's already a few questions and remarks.
Also, you updated k8s-views-global.json
, but the PR and commit message are "feat(global): Windows node support". We can change that later, but just to make sure you wanted to start with k8s-views-global.json
.
"interval": "", | ||
"legendFormat": "Real", | ||
"legendFormat": "Real Linux", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have Real Linux and Real Windows at the same time in graphs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there is a transformations which merge both Real queries together.
Otherwise, the prometheus query for both windows and linux is quite complex and hard to understand. I choice to use dedicated queries for windows and linux to keep the it more simple for other users. (See line 201+)
b4a9e25
to
176713e
Compare
176713e
to
6c9170b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the CPU Utilization by namespace
panel is not working anymore because of the added transformations, can we drop them ?
They don't seems to work anyway (at least on my side)
Yeah, we can drop it. On my site, there are disabled anyways. Sounds strange that they making trouble on your side. |
🎉 This PR is included in version 1.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Pull Request
Required Fields
🔎 What kind of change is it?
🎯 What has been changed and why do we need it?
Optional Fields
✔️ Which issue(s) this PR fixes?
💬 Additional information?