HYPERFLEET-1361 - feat: Add Grafana dashboard for API and DB metrics - #311
HYPERFLEET-1361 - feat: Add Grafana dashboard for API and DB metrics#311tirthct wants to merge 3 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds the “HyperFleet API Metrics” Grafana dashboard with Prometheus panels for API, reconciliation, database, connection-pool, and request-path metrics. Reconciliation pending, stuck, and maximum stuck-duration queries now fall back to Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Grafana
participant Prometheus
participant HyperFleetAPI
Grafana->>Prometheus: Query API, reconciliation, and database metrics
Prometheus->>HyperFleetAPI: Scrape exported metric series
Prometheus-->>Grafana: Return time-series data or zero fallback
Grafana-->>Grafana: Render dashboard panels
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Risk Score: 2 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 1219 lines (>500) | +2 |
| Sensitive paths | none | +0 |
Computed by hyperfleet-risk-scorer
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/Chart.yaml`:
- Line 5: Add a short note to charts/templates/NOTES.txt mentioning the bundled
charts/dashboards/hyperfleet-api.json dashboard, alongside the existing API,
validation schema, and JWT settings notes, so install and upgrade output
includes this user-facing artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ee8bf80a-5749-467a-af9e-0ec817167be8
📒 Files selected for processing (5)
CHANGELOG.mdcharts/Chart.yamlcharts/README.mdcharts/dashboards/hyperfleet-api.jsondocs/metrics.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
| description: HyperFleet API - Cluster Lifecycle Management Service | ||
| type: application | ||
| version: 1.1.0 | ||
| version: 1.2.0 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -p 'NOTES.txt' charts | xargs -I{} sh -c 'echo "--- {} ---"; cat {}'Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 2053
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== chart files referencing dashboard =="
rg -n --hidden --glob 'charts/**' -i 'dashboard|grafana' charts || true
echo
echo "== chart NOTES file =="
cat -n charts/templates/NOTES.txtRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 2608
Add the bundled dashboard to charts/templates/NOTES.txt
charts/dashboards/hyperfleet-api.json ships with the chart, but the NOTES file only covers the API, validation schema, and JWT settings. Add a short dashboard note so install/upgrade output covers the user-facing artifact.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/Chart.yaml` at line 5, Add a short note to charts/templates/NOTES.txt
mentioning the bundled charts/dashboards/hyperfleet-api.json dashboard,
alongside the existing API, validation schema, and JWT settings notes, so
install and upgrade output includes this user-facing artifact.
Source: Path instructions
| description: HyperFleet API - Cluster Lifecycle Management Service | ||
| type: application | ||
| version: 1.1.0 | ||
| version: 1.2.0 |
There was a problem hiding this comment.
Why the version bump when the json chart is not part of the helm output yet?
| "type": "prometheus", | ||
| "uid": "${datasource}" | ||
| }, | ||
| "expr": "max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation_stuck)", |
There was a problem hiding this comment.
When you append or vector(0) to any of the expressions, e.g. max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation_stuck) or vector(0) the output graph looks better imo, the y axis defaults to 0, reads better than No data and there are no gaps in the graph. What do you think?
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/dashboards/hyperfleet-api.json (1)
1156-1156: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAdd coverage for
hyperfleet_api_active_connections.The architecture contract includes this metric, but none of the 12 panel targets queries it. The dashboard therefore misses a production visibility signal despite the PR objective claiming complete API/DB metric coverage. Add a panel or correct the metric inventory and documentation before merge (CWE-682).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/dashboards/hyperfleet-api.json` at line 1156, Update the dashboard metric inventory around the panel target definitions to cover hyperfleet_api_active_connections by adding a panel target that queries it, preserving the existing dashboard structure and coverage conventions; alternatively, if the metric is intentionally unsupported, remove it from the architecture contract and update the related documentation.Source: Linked repositories
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/dashboards/hyperfleet-api.json`:
- Around line 564-570: Update the PromQL expressions for “Resources Pending
Reconciliation” at charts/dashboards/hyperfleet-api.json:564-570, :656-662, and
:748-754 to use a label-aware global fallback with `or on() vector(0)`,
preventing an unlabeled zero series from being unioned with labeled data. Apply
the same expression change at all three sites.
---
Outside diff comments:
In `@charts/dashboards/hyperfleet-api.json`:
- Line 1156: Update the dashboard metric inventory around the panel target
definitions to cover hyperfleet_api_active_connections by adding a panel target
that queries it, preserving the existing dashboard structure and coverage
conventions; alternatively, if the metric is intentionally unsupported, remove
it from the architecture contract and update the related documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 90635993-b78d-43a5-86ae-b06fff5e18ca
📒 Files selected for processing (2)
charts/dashboards/hyperfleet-api.jsondocs/metrics.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/metrics.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/dashboards/hyperfleet-api.json`:
- Line 564: Update the PromQL expressions for this panel and the other two
related panels so fallback zeros preserve or generate the expected resource_type
and is_delete label combinations, rather than using a single unlabeled
vector(0); alternatively, revise each panel description to accurately describe
the unlabeled whole-vector fallback. Keep the query behavior and panel text
consistent across all three panels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 16b2db62-a80f-431a-ad35-458e6c32f114
📒 Files selected for processing (1)
charts/dashboards/hyperfleet-api.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
| "type": "prometheus", | ||
| "uid": "${datasource}" | ||
| }, | ||
| "expr": "max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation) or on() vector(0)", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
: "${PROM_URL:?Set PROM_URL to a Prometheus base URL}"
query='max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation) or on() vector(0)'
curl -fsS -G "$PROM_URL/api/v1/query" \
--data-urlencode "query=$query" |
jq '.data.result[] | {metric, value}'Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 235
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='charts/dashboards/hyperfleet-api.json'
# Show the changed query blocks and nearby panel text.
python3 - <<'PY'
from pathlib import Path
import json
p = Path('charts/dashboards/hyperfleet-api.json')
data = json.loads(p.read_text())
# Print panels containing the target metric/query fragments.
for panel in data.get('panels', []):
s = json.dumps(panel)
if 'hyperfleet_api_resource_pending_reconciliation' in s or 'resource_type' in s and 'is_delete' in s:
print('--- PANEL ---')
print('title:', panel.get('title'))
print('description:', panel.get('description'))
targets = panel.get('targets', [])
for t in targets:
expr = t.get('expr')
if expr and 'hyperfleet_api_resource_pending_reconciliation' in expr:
print('expr:', expr)
print()
PYRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1099
Make the fallback label-aware or update the panel text. or on() vector(0) only adds a single unlabeled zero when the whole vector is empty; it does not fill missing (resource_type, is_delete) combinations (CWE-682). The description says “for that combination,” so the query and copy disagree. Apply the same fix to the other two panels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/dashboards/hyperfleet-api.json` at line 564, Update the PromQL
expressions for this panel and the other two related panels so fallback zeros
preserve or generate the expected resource_type and is_delete label
combinations, rather than using a single unlabeled vector(0); alternatively,
revise each panel description to accurately describe the unlabeled whole-vector
fallback. Keep the query behavior and panel text consistent across all three
panels.
|
|
||
| ### Added | ||
|
|
||
| - Grafana dashboard for API and database metrics (`charts/dashboards/hyperfleet-api.json`) — covers HTTP request rate/latency, reconciliation pending/stuck gauges, DB query duration/errors, connection pool, and build info ([#TBD](https://github.com/openshift-hyperfleet/hyperfleet-api/pull/TBD)) |
| }, | ||
| "gridPos": { | ||
| "h": 8, | ||
| "w": 12, |
There was a problem hiding this comment.
consider making this a little smallee? Do we need it to take up half the page?
| } | ||
| ], | ||
| "title": "Resources Pending Reconciliation", | ||
| "description": "Live-query gauge — falls back to 0 when no resources are pending for that combination.", |
There was a problem hiding this comment.
| "description": "Live-query gauge — falls back to 0 when no resources are pending for that combination.", | |
| "description": "Live-query gauge — displays 0 when no resources are pending.", |
| ] | ||
| }, | ||
| "unit": "short" | ||
| }, |
There was a problem hiding this comment.
| }, | |
| "noValue" : "0" | |
| }, |
| "type": "prometheus", | ||
| "uid": "${datasource}" | ||
| }, | ||
| "expr": "max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation) or on() vector(0)", |
There was a problem hiding this comment.
| "expr": "max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation) or on() vector(0)", | |
| "expr": "max by (resource_type, is_delete) (hyperfleet_api_resource_pending_reconciliation)", |
In correspondence with:
https://github.com/openshift-hyperfleet/hyperfleet-api/pull/311/changes#r3691681058
https://github.com/openshift-hyperfleet/hyperfleet-api/pull/311/changes#r3691676826
|
https://github.com/openshift-hyperfleet/hyperfleet-api/pull/311/changes#r3691682412 Added a note to change Panel 6 to use the default "noValue" instead of "or on() vector(0)" Also should update Panel 7,8 |
Summary
charts/dashboards/hyperfleet-api.json) covering all 11hyperfleet_api_*andhyperfleet_db_*Prometheus metrics across 12 panelsdocs/metrics.mdto replace the stub dashboard section with actual import instructions, coverage inventory, and behavioral notesDashboard panels
requests_totalsum by (code) (rate(...))build_inforequest_duration_secondshistogram_quantileP50/P95/P99requests_total{code=~"5.."}sum by (path) (rate(...))reconciliation_started_totalsum by (resource_type, is_delete) (rate(...))resource_pending_reconciliationmax by (resource_type, is_delete)resource_pending_reconciliation_stuckmax by (resource_type, is_delete)resource_pending_reconciliation_stuck_duration_secondsmax by (resource_type, is_delete)db_query_duration_secondshistogram_quantileP50/P95/P99db_errors_totalsum by (error_type) (rate(...))db_connections_open+db_connections_in_usesum(...)requests_totaltopk(10, ...)instant tableDesign decisions
max by(identical SQL results), connection pool usessum(per-instance)Test plan
helm-testCI check passes (make helm-docsalready run)charts/README.mdreflects version 1.2.0Screenshots