-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: implement clusters in k8s infra monitoring #6628
feat: implement clusters in k8s infra monitoring #6628
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
26a6959
to
b5d57e1
Compare
363d0e7
to
04a2a31
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to 21a78f1 in 2 minutes and 3 seconds
More details
- Looked at
960
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/K8sClustersList.tsx:112
- Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This applies to other instances in the code as well. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_OWnIjk1DnyqVdvEW
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.
21a78f1
to
a3a8f7c
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on a3a8f7c in 1 minute and 19 seconds
More details
- Looked at
922
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:70
- Draft comment:
TheorderBy
column 'cpu' does not exist in the data structure. Consider using a valid column name like 'cpuUsage'. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:53
- Draft comment:
Consider defining a specific type forgroupedByMeta
instead of usingany
to improve type safety. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:58
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_Vuxm2IOGxRYpoF3F
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 26a83fb in 1 minute and 11 seconds
More details
- Looked at
142
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:1
- Draft comment:
The label for 'Memory Utilization' should be 'Memory Utilization (bytes)' instead of 'Memory Utilization (cores)'. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:162
- Draft comment:
Ensure that the label for 'Memory Utilization' indefaultAddedColumns
matches the actual data representation, which is in bytes. - Reason this comment was not posted:
Comment did not seem useful.
3. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:88
- Draft comment:
Avoid using inline styles for column headers. Use external stylesheets, CSS classes, or styled components instead. This issue is also present in other column definitions in this file. - Reason this comment was not posted:
Comment was on unchanged code.
4. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:104
- Draft comment:
Avoid using inline styles for column headers. Use external stylesheets, CSS classes, or styled components instead. This issue is also present in other column definitions in this file. - Reason this comment was not posted:
Marked as duplicate.
5. frontend/src/container/InfraMonitoringK8s/Clusters/utils.tsx:162
- Draft comment:
Avoid using inline styles for column headers. Use external stylesheets, CSS classes, or styled components instead. This issue is also present in other column definitions in this file. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_o6zABtnbfDdrAdq4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Incremental review on f7f8712 in 14 seconds
More details
- Looked at
24
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/hooks/infraMonitoring/useGetK8sClustersList.ts:41
- Draft comment:
The change fromGET_HOST_LIST
toGET_CLUSTER_LIST
in the query key is correct and aligns with the new feature implementation. - Reason this comment was not posted:
Confidence changes required:0%
The change fromGET_HOST_LIST
toGET_CLUSTER_LIST
in the query key is correct and aligns with the new feature implementation.
2. frontend/src/hooks/infraMonitoring/useGetK8sClustersList.ts:41
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This comment applies to any other instances of inline styles in the codebase. - Reason this comment was not posted:
Confidence changes required:50%
The code is well-structured and follows the rules provided. The only issue is the use of inline styles, which is not present here. The code uses external constants and hooks appropriately.
Workflow ID: wflow_ayeWwrXLplRROnxa
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
c89ffab
to
79cae2d
Compare
a294614
to
6855009
Compare
2edb730
to
579c7cb
Compare
579c7cb
to
22a33e3
Compare
f7f8712
to
92bd7d4
Compare
key: 'responseStatusCode-string-tag', | ||
width: 145, | ||
}, | ||
]; |
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.
already exists
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.
Please see and refactor code accordingly to reduce these larger duplications with the already merged code and with the other 3 PRs
f031c78
to
3a5a1b6
Compare
935f11d
to
1d04816
Compare
3a5a1b6
to
3640874
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 3640874 in 1 minute and 51 seconds
More details
- Looked at
332
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Metrics/ClusterMetrics.tsx:84
- Draft comment:
Avoid usingany
type fordata
. Define a specific type or interface for the expected structure ofdata
. This applies to other instances ofany
in this file as well. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Metrics/constants.ts:788
- Draft comment:
Avoid usingany
type fordata
. Define a specific type or interface for the expected structure ofdata
. This applies to other instances ofany
in this file as well. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Metrics/constants.ts:1061
- Draft comment:
Avoid usingany
type fordata
. Define a specific type or interface for the expected structure ofdata
. This applies to other instances ofany
in this file as well. - Reason this comment was not posted:
Marked as duplicate.
4. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Metrics/constants.ts:1250
- Draft comment:
Avoid usingany
type fordata
. Define a specific type or interface for the expected structure ofdata
. This applies to other instances ofany
in this file as well. - Reason this comment was not posted:
Marked as duplicate.
5. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Metrics/constants.ts:1523
- Draft comment:
Avoid usingany
type fordata
. Define a specific type or interface for the expected structure ofdata
. This applies to other instances ofany
in this file as well. - Reason this comment was not posted:
Marked as duplicate.
6. frontend/src/container/InfraMonitoringK8s/commonUtils.tsx:7
- Draft comment:
Avoid using inline styles. Consider using CSS classes or styled components instead. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
7. frontend/src/container/InfraMonitoringK8s/commonUtils.tsx:8
- Draft comment:
Avoid using inline styles. Consider using CSS classes or styled components instead. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_RpeFrxp4pvnCFOtq
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.
const chartData = useMemo( | ||
() => | ||
queries.map(({ data }) => { | ||
const panelType = (data?.params as any)?.compositeQuery?.panelType; |
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.
Avoid using any
type for data
. Define a specific type or interface for the expected structure of data
. This applies to other instances of any
in this file as well.
3640874
to
e7e645e
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on e7e645e in 2 minutes and 30 seconds
More details
- Looked at
5525
lines of code in32
files - Skipped
0
files when reviewing. - Skipped posting
8
drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/ClusterTraces.styles.scss:45
- Draft comment:
Redundant CSS properties:font-size
andfont-weight
are defined twice in.ant-table-thead > tr > th
. Consider removing the duplicates to clean up the code. - Reason this comment was not posted:
Confidence changes required:50%
The code inClusterTraces.styles.scss
contains a redundant CSS rule for.ant-table-thead > tr > th
wherefont-size
andfont-weight
are defined twice. This redundancy should be removed to clean up the code.
2. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/ClusterTraces.tsx:54
- Draft comment:
Theoffset
state is initialized but never updated. If pagination is intended, consider implementing a mechanism to update theoffset
state when pagination changes. - 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 intentionally use URL query parameters for pagination state rather than local state. The offset state seems to be a fallback value when paginationQueryData.offset is not available. The comment is technically correct that offset never changes, but this appears to be by design since pagination is handled through URL parameters instead.
I could be wrong about the intention - maybe this is actually a bug where the offset should be updated but isn't. The presence of the append logic in useEffect suggests pagination was intended.
The use of URL parameters for pagination state is a common pattern, and the code consistently uses paginationQueryData.offset || offset throughout. The append logic works with either source of offset.
The comment should be deleted since the offset behavior appears intentional, using URL parameters instead of local state for pagination.
3. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/ClusterTraces.tsx:138
- Draft comment:
ThetotalCount
is calculated using the length of the list in the first result. Consider summing the lengths of all lists in the results for accuracy. - Reason this comment was not posted:
Confidence changes required:50%
InClusterTraces.tsx
, thetotalCount
is calculated using the length of the list in the first result. This might not be accurate if there are multiple results. Consider summing the lengths of all lists in the results.
4. frontend/src/container/InfraMonitoringK8s/Clusters/K8sClustersList.tsx:65
- Draft comment:
The functionsetselectedClusterName
should be renamed tosetSelectedClusterName
to follow camelCase naming conventions. - Reason this comment was not posted:
Confidence changes required:50%
InK8sClustersList.tsx
, thesetselectedClusterName
function is used with a lowercase 's'. This is inconsistent with the camelCase naming convention and should be corrected tosetSelectedClusterName
.
5. frontend/src/container/InfraMonitoringK8s/Clusters/K8sClustersList.tsx:286
- Draft comment:
Ensure thatrecord.key
is unique and correctly represents the cluster name when settingselectedClusterName
. - Reason this comment was not posted:
Confidence changes required:50%
InK8sClustersList.tsx
, thehandleRowClick
function usesrecord.key
to setselectedClusterName
. Ensure thatrecord.key
is unique and correctly represents the cluster name.
6. frontend/src/container/InfraMonitoringK8s/Clusters/K8sClustersList.tsx:313
- Draft comment:
Consider movingexpandedRowRender
outside the component to avoid re-creation on each render, which can improve performance. - Reason this comment was not posted:
Confidence changes required:50%
InK8sClustersList.tsx
, theexpandedRowRender
function is defined inside the component. Consider moving it outside to avoid re-creation on each render, which can improve performance.
7. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/ClusterTraces.styles.scss:108
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values likergba(255, 255, 255, 0.04)
to maintain consistency in design and theming. - Reason this comment was not posted:
Marked as duplicate.
8. frontend/src/container/InfraMonitoringK8s/Clusters/ClusterDetails/Traces/ClusterTraces.styles.scss:189
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values likergba(0, 0, 0, 0.04)
to maintain consistency in design and theming. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_LH9YwEMpF3M2O2Ig
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.
font-size: 12px; | ||
line-height: 18px; | ||
|
||
background: rgba(171, 189, 255, 0.01); |
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.
Use design tokens or predefined color constants instead of hardcoding color values like rgba(171, 189, 255, 0.01)
to maintain consistency in design and theming.
65f2389
into
feat/infra-monitoring-k8s-deployments
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Summary
Implement the clusters entity in Infra Monitoring
Related Issues / PR's
Screenshots
Affected Areas and Manually Tested Areas
Infra Monitoring section
Important
Implement clusters entity in Kubernetes infrastructure monitoring with list and detail views, including metrics, logs, events, and traces.
K8sClustersList.tsx
.ClusterDetails.tsx
.getK8sClustersList
API function ingetK8sClustersList.ts
.useGetK8sClustersList
hook inuseGetK8sClustersList.ts
for fetching cluster data.ClusterMetrics
,ClusterLogs
,ClusterEvents
, andClusterTraces
components for detailed cluster views..scss
files.InfraMonitoringK8s.tsx
to include clusters in the monitoring dashboard.This description was created by for e7e645e. It will automatically update as commits are pushed.