-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: added metric page in messaging queues #6399
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> |
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 2dee246 in 2 minutes and 4 seconds
More details
- Looked at
1686
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_yZ3XDS0JPZUP0kJ9
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.
frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricPage.styles.scss
Show resolved
Hide resolved
frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricPageUtil.ts
Show resolved
Hide resolved
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> |
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 d6ca85b in 41 seconds
More details
- Looked at
163
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:86
- Draft comment:
The change frommqServiceView
toselectedView
in thevalue
prop is correct and ensures consistency with the state management logic. - Reason this comment was not posted:
Confidence changes required:0%
The change frommqServiceView
toselectedView
in thevalue
prop of theSelect
component is correct and aligns with the state management logic.
2. frontend/src/pages/MessagingQueues/MQDetails/DropRateView/DropRateView.tsx:233
- Draft comment:
The removal of thedrop-rat-title
class is appropriate as the title is now directly usingMessagingQueuesViewType.dropRate.label
. - Reason this comment was not posted:
Confidence changes required:0%
The removal of thedrop-rat-title
class and its associated styles is appropriate since the title is now directly using theMessagingQueuesViewType.dropRate.label
.
3. frontend/src/pages/MessagingQueues/MQDetails/MQDetails.tsx:63
- Draft comment:
The removal of theComingSoon
component and the associated logic forPartitionHostMetrics
is appropriate as it is no longer needed. - Reason this comment was not posted:
Confidence changes required:0%
The removal of theComingSoon
component and the associated logic forPartitionHostMetrics
is appropriate since it is no longer needed.
4. frontend/src/pages/MessagingQueues/MQDetails/MQDetails.tsx:21
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is also applicable to other similar imports in this PR. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_4atVWBkWPIFXrJoz
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Added doc not required as this is a part of a feature and can't have doc required for every single part PR, also we are making sure to have docs for Kafka 2.0 |
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> |
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 ef1c710 in 28 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:174
- Draft comment:
The class namemiddle-card
was removed from this div, but it is still present in other similar divs (e.g., lines 159 and 195). Ensure this change is intentional for consistent styling. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:174
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This applies to theMessagingQueues.tsx
file. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_ZC4aYr87G2VcqcYB
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> |
frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricColumnGraphs.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricColumnGraphs.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricColumnGraphs.tsx
Show resolved
Hide resolved
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 9a27917 in 28 seconds
More details
- Looked at
346
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricColumnGraphs.tsx:108
- Draft comment:
Optional chaining is unnecessary here sincegraphCount
is always defined in themetricsData
array. Consider removing the optional chaining.
graphCount={metric.graphCount}
- Reason this comment was not posted:
Confidence changes required:50%
The translation keys in theMetricColumnGraphs
component are being used correctly, but there is a potential issue with thegraphCount
property. ThegraphCount
is being accessed with optional chaining, which is unnecessary sincegraphCount
is always defined in themetricsData
array.
2. frontend/src/pages/MessagingQueues/MQDetails/MetricPage/MetricPage.tsx:94
- Draft comment:
There's a typo in the keybokerJVMMetrics
. It should bebrokerJVMMetrics
to match the translation keys.
key: 'brokerJVMMetrics',
- Reason this comment was not posted:
Comment was on unchanged code.
3. frontend/src/pages/MessagingQueues/MQDetailPage/MQDetailPage.tsx:130
- Draft comment:
Avoid using inline styles in JSX. Consider using CSS classes or styled components instead. This applies to other inline styles in this file as well. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_2wgsz6nRqEim4zKi
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary
Added metric page with 5 categories and 22 dashboards
Related Issues / PR's
Screenshots
Overview Page -
Screen.Recording.2024-11-08.at.2.26.17.AM.mov
Affected Areas and Manually Tested Areas
Important
Added a new metric page for Kafka in messaging queues with detailed graphs and metrics.
MetricPage
component inMQDetailPage.tsx
for Kafka metrics.metricPage
inMessagingQueuesViewType
inMessagingQueuesUtils.ts
.MessagingQueues.tsx
to include navigation to the metric page.MetricPage
,MetricColumnGraphs
,MetricPageGridGraph
inMQDetails/MetricPage/
.MetricPage
has collapsible sections for metric categories.MetricPage.styles.scss
for new metric page styling.MetricPageUtil.ts
for widget data for Kafka metrics.messagingQueues.json
andmessagingQueuesKafkaOverview.json
for English and British English locales.This description was created by for 9a27917. It will automatically update as commits are pushed.