feat: implement cluster details page #6698
Conversation
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 2f992b1 in 1 minute and 44 seconds
More details
- Looked at
5604lines of code in33files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/ClusterTraces.tsx:54
- Draft comment:
Theoffsetstate is initialized but never updated. Consider updating it to handle pagination correctly. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The code appears to handle pagination through URL query params (paginationQueryData) rather than local state. The offset state is just a fallback value of 0. While technically true that the setter is unused, this appears intentional as the pagination is controlled through the URL instead. The comment might lead to confusion or unnecessary changes.
The comment correctly identifies an unused state setter. Perhaps there's a reason why URL-based pagination was chosen over state-based pagination that I'm missing?
Using URL-based pagination is a valid and common pattern that allows for shareable/bookmarkable pages. The unused setter isn't a bug - it's just that the component uses a different pagination mechanism.
The comment should be deleted. While technically correct about the unused setter, the pagination is intentionally handled through URL params instead of state, making this comment potentially misleading.
2. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/constants.ts:107
- Draft comment:
TheaggregateAttributeis set with empty values. Ensure this is intentional and won't affect query execution. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_lFKd59bAKweoIPMh
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| items: [], | ||
| op: 'and', | ||
| }, | ||
| orderBy: { columnName: 'cpu', order: 'desc' }, |
There was a problem hiding this comment.
The orderBy field is hardcoded to cpu, which might not exist or be intended for ordering. Consider making this dynamic or ensuring the field exists.
| key: `${key}-${dataType}-${type}`, | ||
| width: 145, | ||
| render: (value, item): JSX.Element => { | ||
| const itemData = item.data as any; |
There was a problem hiding this comment.
Using item.data as any can lead to runtime errors. Consider using a more specific type or adding type checks.
| return ( | ||
| <div className="pod-group"> | ||
| {groupByValues.map((value) => ( | ||
| <Tag key={value} color="#1D212D" className="pod-group-tag-item"> |
There was a problem hiding this comment.
Use design tokens or predefined color constants instead of hardcoding color values for consistency.
amlannandy
force-pushed
the
feat/infra-monitoring-k8s-clusters-details
branch
from
2f992b1 to
de61caa
Compare
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on de44425 in 56 seconds
More details
- Looked at
236lines of code in4files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/ClusterDetails.tsx:373
- Draft comment:
Remove the console.log statement to avoid unnecessary console output in production code. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_2eqinKoMwd1XQsZL
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| } | ||
| }, [groupByFiltersData]); | ||
|
|
||
| console.log({ selectedClusterData }); |
There was a problem hiding this comment.
Remove the console.log statement to avoid unnecessary console output in production code.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ee9c392 in 40 seconds
More details
- Looked at
1706lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/K8sClustersList.tsx:435
- Draft comment:
Remove the unnecessaryconsole.logstatement to clean up the code. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/container/InfraMonitoringK8s/Clusters/K8sClustersList.tsx:435
- Draft comment:
Remove the unnecessary console.log statement to clean up the code.
- Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_mrR7KhSsv8jRcmBd
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Summary
Implement the clustersdetails page in Infra Monitoring
Related Issues / PR's
N/A
Screenshots
N/A
Affected Areas and Manually Tested Areas
Infra Monitoring section
Important
Implement detailed Kubernetes cluster view in Infra Monitoring with metrics, logs, traces, and events.
ClusterDetailscomponent for Kubernetes cluster details.ClusterMetrics,ClusterLogs,ClusterTraces, andClusterEventscomponents.NoLogsContainerandNoEventsContainerfor empty states.getK8sClustersListingetK8sClustersList.tsfor fetching cluster data.useGetK8sClustersListhook for querying data.ClusterDetails.styles.scss.constants.tsandutils.tsxfor data formatting.This description was created by
for ee9c392. It will automatically update as commits are pushed.