Combining metrics profiles files #411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
|
@rsevilla87 @mohit-sheth @dry923 can you all please take a look when you get the chance |
Closed
paigerube14
force-pushed
the
metrics_combine
branch
from
0b429c2 to
4f0ee21
Compare
|
@rsevilla87 @dry923 are we able to get this PR merged? The default metrics profile doesn't collect all the data that is available when using all the touchstone-configs here |
|
@rsevilla87 @jtaleric @dry923 any thoughts on this? Can we get this merged?
This small difference is also causing discrepancies in the grafana dashboards in the node section because it’s looking for “podCPU” that isn’t captured in the metrics.yaml file that nodes that aren't for the UUID are being listed |
jtaleric
approved these changes
Sep 29, 2022
|
Think the issue with touchstone is similar to : cloud-bulldozer/benchmark-comparison#57 not sure if it's the exact same thing though |
rsevilla87
reviewed
Sep 30, 2022
Comment on lines
21
to
25
| - query: sum(irate(container_cpu_usage_seconds_total{name!="",namespace=~"openshift-(etcd|oauth-apiserver|.*apiserver|ovn-kubernetes|sdn|ingress|authentication|.*controller-manager|.*scheduler|monitoring|logging|image-registry)"}[2m]) * 100) by (pod, namespace, node) | ||
| metricName: podCPU | ||
|
|
||
| - query: sum(container_memory_rss{name!="",namespace=~"openshift-(etcd|oauth-apiserver|.*apiserver|ovn-kubernetes|sdn|ingress|authentication|.*controller-manager|.*scheduler|monitoring|logging|image-registry)"}) by (pod, namespace, node) | ||
| metricName: podMemory |
There was a problem hiding this comment.
Now that we're using containerCPU and containerMemory metrics in touchstone I think we can skip add these two queries here.
There was a problem hiding this comment.
with this being the only difference in these 2 files can we not just add these 2 queries and combine?
rsevilla87
reviewed
Sep 30, 2022
| @@ -0,0 +1,21 @@ | |||
| { | |||
| "elasticsearch": { | |||
| "ripsaw-kube-burner": [ | |||
There was a problem hiding this comment.
Why 4 configuration files rather than a single one holding all queries?
There was a problem hiding this comment.
I sent a example on slack on how to consolidate this.
|
Hey @paigerube14, I managed to query CPU and memory metrics from pods using the new touchstone config files you've added: I think we can skip podCPU and podMemory metrics in metrics.yaml |
vishnuchalla
pushed a commit
that referenced
this pull request
Sep 6, 2023
* combining metrics profiles * adding container touchstone configs * adding overall container metrics * deleting old files * adding metrics-ovn back
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Want to combine the metrics and metrics-ovn yaml files so the default metrics file has podCPU and podMemory that can be used when creating the benchmark-comparison for kube-burner
Fixes