-
-
Notifications
You must be signed in to change notification settings - Fork 4
feat:Resource Consumption Visualization UI #63
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
Conversation
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.
Pull request overview
This PR adds resource consumption visualization capabilities to the admin dashboard, introducing real-time monitoring of CPU, memory, network, and disk I/O metrics through Chart.js visualizations.
Key Changes:
- New
ResourceMonitorclass inbin/lib/resources.jsfor collecting system and per-service metrics usingpidusageandsysteminformationlibraries - Integration of Chart.js into the admin dashboard for real-time metric visualization with line and doughnut charts
- New
/api/metricsendpoint providing system-wide resource metrics - Comprehensive integration tests validating dashboard functionality, API endpoints, and Chart.js integration
Reviewed changes
Copilot reviewed 1 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tests/resource-monitoring-integration.test.js |
Adds integration tests for resource monitoring dashboard, API endpoints, graceful shutdown, and Chart.js visualization |
bin/lib/resources.js |
Introduces ResourceMonitor class with metric collection, PID detection, platform-specific disk I/O monitoring, and history management |
bin/lib/admin.js |
Extends dashboard with Chart.js integration, metric visualization UI components, and /api/metrics endpoint using systeminformation library |
| adminProcess = execa('node', [ | ||
| path.join(process.cwd(), 'bin', 'index.js'), | ||
| 'admin', | ||
| '--port', '9999' | ||
| ], { | ||
| cwd: testWorkspace, | ||
| stdio: 'pipe' | ||
| }); | ||
|
|
||
| // Wait for dashboard to start | ||
| await new Promise(resolve => setTimeout(resolve, 3000)); |
Copilot
AI
Nov 25, 2025
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.
Duplicated dashboard startup code. The code for starting the admin dashboard and waiting for it to initialize is repeated in all test cases. Consider extracting this into a helper function to reduce code duplication and make tests more maintainable.
| expect(adminProcess.killed).toBe(false); | ||
|
|
||
| // Test if dashboard is responding | ||
| const response = await makeRequest('GET', 'http://localhost:9999/', {}, 5000); |
Copilot
AI
Nov 25, 2025
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.
Hardcoded timeout may cause flaky tests. Using a fixed 5000ms timeout for HTTP requests could lead to test flakiness on slower systems or under heavy load. The helper function already accepts a timeout parameter - consider making this configurable or documenting why 5 seconds is appropriate for all test environments.
- Move resource metrics charts to dedicated Resources page - Reduce chart sizes with better aspect ratios (160px height) - Improve chart styling with modern colors and compact legends - Fix CPU metrics to show decimal precision - Fix disk calculation to only use root volume (was duplicating mounts) - Fix memory calculation to use active memory instead of total used - Change network display from MB/s to KB/s for better accuracy - Fix list-services.mjs syntax error in scaffold template
|
lgtm |
…ect disk I/O to disk usage Signed-off-by: kaifcoder <kaifmohd2014@gmail.com>
No description provided.