-
Notifications
You must be signed in to change notification settings - Fork 121
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: cpu memory metrics #2332
base: main
Are you sure you want to change the base?
feat: cpu memory metrics #2332
Conversation
adarsh0728
commented
Jan 15, 2025
•
edited
Loading
edited
- Adds Memory and CPU pattern in metrics config at pod level
- Adds Memory and CPU pattern in metrics config at container level [filter values added for container dropdown for a particular pod]
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2332 +/- ##
=======================================
Coverage 67.47% 67.47%
=======================================
Files 351 351
Lines 45822 45827 +5
=======================================
+ Hits 30918 30922 +4
Misses 13828 13828
- Partials 1076 1077 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
@@ -115,6 +116,7 @@ export function PodDetail({ | |||
pipelineId={pipelineId} | |||
type={type} | |||
vertexId={vertexId} | |||
podDetails={podDetails} |
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.
since we are using pod name out of podDetails, can we just pass it instead?
required: false | ||
- name: mono_vertex_pod_cpu_memory_utilization |
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 merge monovertex and pipeline patterns for pod and containers, since it is repetitive?
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.
I can think of two ways:
- Introducing new field (
Objects
) which is a list. Ensures backward compatibility (no change in existing config), only discovery API needs to check two fields now instead of one. - Making
Object
field as a list. It has an advantage of having single field but have to changeObject
field for existing patterns and make sure it doesn't break anything.
@veds-g WDYT?
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.
lets make it Objects
which acts as a llist.
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>