-
Notifications
You must be signed in to change notification settings - Fork 124
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
fix: filter out deamon pod and unnecessary containers from metrics output #2394
Conversation
adarsh0728
commented
Feb 12, 2025
•
edited
Loading
edited
- Filter out daemon pod from metrics response.
- Filter in only pod's containers from metrics response.
Signed-off-by: adarsh0728 <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2394 +/- ##
==========================================
- Coverage 71.10% 71.02% -0.08%
==========================================
Files 367 367
Lines 53042 53042
==========================================
- Hits 37715 37674 -41
- Misses 14247 14293 +46
+ Partials 1080 1075 -5 ☔ View full report in Codecov by Sentry. |
Signed-off-by: adarsh0728 <[email protected]>
@adarsh0728 what is the condition to filter out pods and containers? keep in mind that in future we will support builtins like kafka, sqs, etc., we cannot filter those out. |
For pods: we call pods API and filter out only daemon. we are not filtering out builtins with this |
ui/src/components/common/MetricsModalWrapper/partials/MetricsModal/index.tsx
Outdated
Show resolved
Hide resolved
...artials/Graph/partials/NodeInfo/partials/Pods/partials/PodDetails/partials/Metrics/index.tsx
Outdated
Show resolved
Hide resolved
...als/NodeInfo/partials/Pods/partials/PodDetails/partials/Metrics/partials/LineChart/index.tsx
Show resolved
Hide resolved
for filtering the containers, we do not have the pipeline or monovertex context is the metrics response. So in case of multiple containers with the same name, how do we ensure that we showing data for the particular container belonging to that pipeline vertex or monovertex? |
...als/NodeInfo/partials/Pods/partials/PodDetails/partials/Metrics/partials/LineChart/index.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
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.
LGTM
…tput (#2394) Signed-off-by: adarsh0728 <[email protected]> Co-authored-by: Vedant Gupta <[email protected]>