-
Notifications
You must be signed in to change notification settings - Fork 17
feat: add network tab to node page with peer connectivity visualization #2826
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: adameat <34044711+adameat@users.noreply.github.com>
@copilot no, you needed to add another tab on the node page. like storage, tablets, threads - there is should be a network tab, very similar to database's network tab. but with filter by current node. |
Co-authored-by: adameat <34044711+adameat@users.noreply.github.com>
You're absolutely right! I've corrected the implementation to add a proper "Network" tab to the node page (alongside Storage, Tablets, Threads tabs) instead of just adding network info to the node information panel. The new Network tab shows:
This matches the database network tab pattern but filtered specifically for the current node. Changes in commit d420599. |
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.
2 files reviewed, no comments
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.
4 files reviewed, 3 comments
: {}; | ||
|
||
// Group peers by type | ||
const peersGrouped = groupNodesByField(peers, 'NodeType'); |
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.
logic: missing performance optimization - peersGrouped
should be wrapped in useMemo
with dependencies [peers]
per AGENTS.md requirement to "ALWAYS use useMemo
for expensive computations"
const peersGrouped = groupNodesByField(peers, 'NodeType'); | |
const peersGrouped = React.useMemo(() => groupNodesByField(peers, 'NodeType'), [peers]); |
const currentNodeGrouped: Record<string, TNetNodeInfo[]> = currentNode | ||
? {[currentNode.NodeType]: [currentNode]} | ||
: {}; |
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.
logic: missing performance optimization - currentNodeGrouped
should be wrapped in useMemo
with dependencies [currentNode]
per AGENTS.md requirement
const currentNodeGrouped: Record<string, TNetNodeInfo[]> = currentNode | |
? {[currentNode.NodeType]: [currentNode]} | |
: {}; | |
const currentNodeGrouped: Record<string, TNetNodeInfo[]> = React.useMemo( | |
() => (currentNode ? {[currentNode.NodeType]: [currentNode]} : {}), | |
[currentNode] | |
); |
let problemNodesCount = 0; | ||
|
||
const result = Object.keys(nodes).map((key, j) => { | ||
const nodesGroupedByRack = groupNodesByField(nodes[key], 'Rack'); |
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.
style: missing performance optimization - nodesGroupedByRack
should be wrapped in useMemo
as it's computed inside a map function, per AGENTS.md requirement
This PR adds a Network tab to the individual node page, displaying connectivity data that helps users understand network topology and relationships between nodes.
Changes Made
New Network Tab
UI Components
NodeNetwork
component that follows the same pattern as the database network tabNodeNetwork
component from the database diagnostics for consistent visualizationTab Configuration
NODE_TABS_IDS
andNODE_TABS
configurationrenderTabContent
switch in Node.tsxInternationalization
Technical Implementation
The implementation leverages the existing network API (
networkApi.useGetNetworkInfoQuery
) and filters the results to focus on the current node and its peers. The network tab uses the same visual components and interaction patterns as the database network tab, providing a consistent user experience.The network information is fetched only when a tenant name is available, and the component gracefully handles cases where no network data or peers are found.
Visual Impact
The network tab provides users with:
This enhancement improves the monitoring and debugging experience for YDB cluster administrators by making network relationships more visible and accessible at the individual node level.
Fixes #2428.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
Greptile Overview
Updated On: 2025-11-01 08:24:10 UTC
Greptile Summary
This PR adds a Network tab to the individual node page, displaying the current node and its network peers with connectivity visualization.
Key Changes:
NodeNetwork
component that filters network data to show current node and its peersNodeNetwork
component from database diagnostics for consistent visualizationIssues Found:
useMemo
) for computed valuescurrentNodeGrouped
andpeersGrouped
, which violates AGENTS.md mandatory requirements for performanceNodes
component computesnodesGroupedByRack
inside a map without memoizationConfidence Score: 3/5
useMemo
wrapping for computed values. These are not optional optimizations - they're explicitly marked as "ALWAYS" and "MANDATORY" in the project guidelines. The missing optimizations could cause unnecessary re-renders and performance issues.Important Files Changed
File Analysis
Sequence Diagram