Feat/health dashboard#13
Conversation
📝 WalkthroughWalkthroughA Health Dashboard feature is added comprising three new files. An HTML page establishes the dashboard UI structure with designated placeholders for metrics, service statuses, and charts. A CSS stylesheet provides visual styling and layout using responsive grid design, color-coded status indicators, and reusable card components. JavaScript code fetches system health data from an 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08b44519-4a15-4f8c-9814-537c421ccaf6
📒 Files selected for processing (3)
dashboard/dashboard.cssdashboard/dashboard.jsdashboard/index.html
| .status-badge { | ||
| padding: 0.5rem 1.5rem; | ||
| border-radius: 20px; | ||
| font-weight: 600; | ||
| font-size: 0.9rem; | ||
| } | ||
|
|
||
| .status-badge.healthy { background: #10b981; color: white; } | ||
| .status-badge.warning { background: #f59e0b; color: white; } | ||
| .status-badge.critical { background: #ef4444; color: white; } |
There was a problem hiding this comment.
Status badge contrast misses AA.
The healthy, warning, and critical badges use white text at 0.9rem on bright fills, so the dashboard’s primary status indicator is hard to read.
Suggested fix
-.status-badge.healthy { background: `#10b981`; color: white; }
-.status-badge.warning { background: `#f59e0b`; color: white; }
-.status-badge.critical { background: `#ef4444`; color: white; }
+.status-badge.healthy { background: `#047857`; color: white; }
+.status-badge.warning { background: `#b45309`; color: white; }
+.status-badge.critical { background: `#b91c1c`; color: white; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .status-badge { | |
| padding: 0.5rem 1.5rem; | |
| border-radius: 20px; | |
| font-weight: 600; | |
| font-size: 0.9rem; | |
| } | |
| .status-badge.healthy { background: #10b981; color: white; } | |
| .status-badge.warning { background: #f59e0b; color: white; } | |
| .status-badge.critical { background: #ef4444; color: white; } | |
| .status-badge { | |
| padding: 0.5rem 1.5rem; | |
| border-radius: 20px; | |
| font-weight: 600; | |
| font-size: 0.9rem; | |
| } | |
| .status-badge.healthy { background: `#047857`; color: white; } | |
| .status-badge.warning { background: `#b45309`; color: white; } | |
| .status-badge.critical { background: `#b91c1c`; color: white; } |
| async function fetchHealth() { | ||
| try { | ||
| const response = await fetch('/api/health'); | ||
| const data = await response.json(); | ||
| return data; | ||
| } catch (error) { | ||
| console.error('Health check failed:', error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t mark stale data as fresh.
fetch() only rejects on network failures. A 500/503 still reaches response.json(), and lastUpdated is written even when no new state was applied. During an outage, the page can show old metrics with a current timestamp.
Suggested fix
async function fetchHealth() {
try {
const response = await fetch('/api/health');
+ if (!response.ok) {
+ throw new Error(`Health check failed: ${response.status}`);
+ }
const data = await response.json();
return data;
} catch (error) {
console.error('Health check failed:', error);
return null;
@@
async function refreshData() {
const health = await fetchHealth();
-
- if (health) {
- updateMetrics(health);
- updateServices(health.services);
- updateOverallStatus(health.status);
- }
-
- document.getElementById('lastUpdated').textContent = new Date().toLocaleTimeString();
+ if (!health) return;
+
+ updateMetrics(health);
+ updateServices(health.services);
+ updateOverallStatus(health.status);
+ document.getElementById('lastUpdated').textContent = new Date().toLocaleTimeString();
}Also applies to: 29-39
| async function refreshData() { | ||
| const health = await fetchHealth(); | ||
|
|
||
| if (health) { | ||
| updateMetrics(health); | ||
| updateServices(health.services); | ||
| updateOverallStatus(health.status); | ||
| } | ||
|
|
||
| document.getElementById('lastUpdated').textContent = new Date().toLocaleTimeString(); | ||
| } |
There was a problem hiding this comment.
The trend chart is still demo data.
drawChart() renders a hard-coded array once on load. The metric card uses /api/health, but the chart never records avgResponseTime or redraws after refresh, so the two views can disagree.
Suggested fix
function updateMetrics(data) {
+ if (typeof data.avgResponseTime === 'number') {
+ metrics.responseTime = [...metrics.responseTime.slice(-29), data.avgResponseTime];
+ }
document.getElementById('responseTime').textContent = data.avgResponseTime || '--';
@@
updateMetrics(health);
updateServices(health.services);
updateOverallStatus(health.status);
+ drawChart(metrics.responseTime);
document.getElementById('lastUpdated').textContent = new Date().toLocaleTimeString();
}
@@
-function drawChart() {
+function drawChart(data = metrics.responseTime) {
const canvas = document.getElementById('trendChart');
const ctx = canvas.getContext('2d');
@@
- // Sample data
- const data = [120, 115, 130, 125, 140, 118, 122, 135, 128, 142];
+ if (!data.length) {
+ ctx.clearRect(0, 0, width, height);
+ return;
+ }Also applies to: 41-46, 82-131, 134-138
Prevent overlapping refreshes from winning out of order.
The 30s timer here plus the manual refresh button in dashboard/index.html can start another fetch before the first one finishes. A slower, older response can then land last and overwrite newer health data.
Suggested fix
+let refreshToken = 0;
+
async function refreshData() {
+ const token = ++refreshToken;
const health = await fetchHealth();
- if (!health) return;
+ if (!health || token !== refreshToken) return;
updateMetrics(health);
updateServices(health.services);
updateOverallStatus(health.status);
document.getElementById('lastUpdated').textContent = new Date().toLocaleTimeString();
}Also applies to: 134-138
| function updateMetrics(data) { | ||
| document.getElementById('responseTime').textContent = data.avgResponseTime || '--'; | ||
| document.getElementById('successRate').textContent = data.successRate || '--'; | ||
| document.getElementById('requestRate').textContent = data.requestRate || '--'; | ||
| document.getElementById('errorCount').textContent = data.errorCount || '--'; | ||
| } |
There was a problem hiding this comment.
Don’t treat zero as missing.
errorCount: 0 and requestRate: 0 render as -- here. On a health dashboard, that’s wrong telemetry.
Suggested fix
- document.getElementById('responseTime').textContent = data.avgResponseTime || '--';
- document.getElementById('successRate').textContent = data.successRate || '--';
- document.getElementById('requestRate').textContent = data.requestRate || '--';
- document.getElementById('errorCount').textContent = data.errorCount || '--';
+ document.getElementById('responseTime').textContent = data.avgResponseTime ?? '--';
+ document.getElementById('successRate').textContent = data.successRate ?? '--';
+ document.getElementById('requestRate').textContent = data.requestRate ?? '--';
+ document.getElementById('errorCount').textContent = data.errorCount ?? '--';1 participant
No description provided.