feat: implement namespace details page #6690
Open
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
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
|
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 afe4554 in 2 minutes and 28 seconds
More details
- Looked at
5623lines of code in33files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/Traces/NamespaceTraces.tsx:54
- Draft comment:
Theoffsetstate is initialized but never updated, which could lead to pagination issues. Consider updating the offset based on user interaction or data fetching. - 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 use URL-based pagination through paginationQueryData rather than local state. The offset state seems to be just a fallback value. The pagination controls are likely handled by the parent component. Without seeing the parent component's code, I can't be certain if this is a real issue or an intentional design choice.
I could be wrong about the pagination being handled at a higher level. Maybe the offset state is actually meant to be updated but was forgotten.
While possible, the consistent use of paginationQueryData throughout the code and the presence of URL query handling suggests this is an intentional design using URL-based pagination rather than local state.
The comment should be deleted as it appears to misunderstand the pagination implementation, which is URL-based rather than state-based.
2. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/Traces/constants.ts:137
- Draft comment:
Theidfield is hardcoded with a UUID. Consider generating a new UUID for each query instance to ensure uniqueness. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
Since this is a constants file, the function getNamespaceTracesQueryPayload is likely meant to generate a consistent, reusable query structure. The hardcoded UUID might be intentional as an identifier for this specific type of query. Without more context about the query system's requirements for uniqueness, suggesting UUID generation could be premature. The comment is speculative about uniqueness requirements.
I might be wrong about the purpose of the ID field - perhaps query IDs do need to be unique per instance for tracking or caching purposes.
Even if query IDs should be unique, this would be a system-level requirement that would need more context and investigation to verify. The comment is making assumptions without clear evidence of a problem.
The comment should be deleted as it makes speculative suggestions about UUID generation without clear evidence that unique IDs are required per query instance.
3. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/Logs/NamespaceLogs.styles.scss:66
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values like#1D212Dfor consistency in design and theming. This issue is also present in other files. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_R4RNKGGYA6ugQ6dw
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.
| return options.queryKey; | ||
| } | ||
|
|
||
| return [REACT_QUERY_KEY.GET_HOST_LIST, requestData]; |
There was a problem hiding this comment.
The query key should be REACT_QUERY_KEY.GET_K8S_NAMESPACES_LIST instead of REACT_QUERY_KEY.GET_HOST_LIST.
| items: [], | ||
| op: 'and', | ||
| }, | ||
| orderBy: { columnName: 'cpu', order: 'desc' }, |
There was a problem hiding this comment.
The orderBy column name should be 'cpuUsage' instead of 'cpu'.
| 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 like #1D212D for consistency in design and theming. This issue is also present in other files.
amlannandy
force-pushed
the
feat/infra-monitoring-k8s-namespace-details
branch
from
afe4554 to
f311d5f
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f311d5f in 38 seconds
More details
- Looked at
1694lines of code in4files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/Metrics/NamespaceMetrics.tsx:116
- Draft comment:
Remove commented-out code to keep the codebase clean and maintainable. - Reason this comment was not posted:
Confidence changes required:50%
The PR includes a lot of commented-out code, which should be removed for cleanliness and maintainability.
2. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:376
- Draft comment:
Remove commented-out code to keep the codebase clean and maintainable. - Reason this comment was not posted:
Confidence changes required:50%
The PR includes a lot of commented-out code, which should be removed for cleanliness and maintainability.
3. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:381
- Draft comment:
Avoid using inline styles. Use external stylesheets or styled components instead. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_BmT0mnvq2QIN64V4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 0972327 in 39 seconds
More details
- Looked at
101lines of code in2files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:118
- Draft comment:
Ensure that the change from 'namespace' to 'Namespace' is consistent with the naming conventions used throughout the codebase for Kubernetes objects. - Reason this comment was not posted:
Confidence changes required:50%
The change from 'namespace' to 'Namespace' in the value field is correct as it aligns with the typical naming conventions for Kubernetes objects.
2. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:151
- Draft comment:
Ensure consistency in logEvent messages across the file. The change to 'Namespaces list details page visited' should be reflected in all similar logEvent calls. - Reason this comment was not posted:
Confidence changes required:50%
The change in the logEvent function call is correct, but it's important to ensure consistency across all similar logEvent calls in the file.
3. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:199
- Draft comment:
Ensure consistency in logEvent messages across the file. The change to 'Namespaces list details time updated' should be reflected in all similar logEvent calls. - Reason this comment was not posted:
Confidence changes required:50%
The change in the logEvent function call is correct, but it's important to ensure consistency across all similar logEvent calls in the file.
4. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:219
- Draft comment:
Ensure consistency in logEvent messages across the file. The change to 'Namespaces list details logs filters applied' should be reflected in all similar logEvent calls. - Reason this comment was not posted:
Confidence changes required:50%
The change in the logEvent function call is correct, but it's important to ensure consistency across all similar logEvent calls in the file.
5. frontend/src/container/InfraMonitoringK8s/Namespaces/NamespaceDetails/NamespaceDetails.tsx:426
- 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.
Workflow ID: wflow_4GFu8wmewrMAPk9D
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
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.
Summary
Implement the namespaces details page in Infra Monitoring
Related Issues / PR's
N/A
Screenshots
N/A
Affected Areas and Manually Tested Areas
Infra Monitoring section
Important
Implements a namespace details page in Infra Monitoring with components for metrics, logs, traces, and events, including API integration and UI enhancements.
NamespaceDetailscomponent with tabs for metrics, logs, traces, and events.NamespaceMetrics,NamespaceLogs,NamespaceTraces, andNamespaceEventscomponents.getK8sNamespacesListingetK8sNamespacesList.tsfor fetching namespace data.useGetK8sNamespacesListhook for querying namespace data.NamespaceDetails,NamespaceMetrics,NamespaceLogs,NamespaceTraces, andNamespaceEventscomponents..scssfiles.utils.tsxfor formatting and handling namespace data.constants.ts.This description was created by
for 0972327. It will automatically update as commits are pushed.