HYPERFLEET-1361 - feat: Add Grafana dashboard for API and DB metrics - #311
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds a new Grafana dashboard JSON file (charts/dashboards/hyperfleet-api.json) that defines panels for API request rate/status, request latency, server errors, reconciliation activity/stuck resources, database query duration/errors, connection pool usage, and top request paths. docs/metrics.md is updated to document the shipped dashboard, its panel categories, import steps, and gauge behavior. CHANGELOG.md records the addition. Estimated code review effort: 3 (Moderate) | ~20 minutes CWE-1104 applies: this JSON dashboard is untracked configuration with no schema validation shown in the diff. Confirm the file is pinned to a specific Grafana dashboard schema version and that Prometheus datasource templating does not allow query injection via the templated variable (CWE-89 analog for PromQL). No code identifiers change; supply-chain surface is limited to this one new artifact file plus doc text. No CI/CD, dependency, or IDE config changes present in this diff — verify that claim holds outside the given scope. 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
tirthct
changed the title
| 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)
|
|
||
| ### 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?
| "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 |
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 (2)
charts/dashboards/hyperfleet-api.json (2)
1159-1200: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMake the dashboard part of the Helm release.
- Set
charts/Chart.yamlversionandappVersionto1.2.0.- Add values and a template to load
charts/dashboards/hyperfleet-api.json.- Update
charts/templates/NOTES.txtwith dashboard access instructions.🤖 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` around lines 1159 - 1200, Update the chart metadata in Chart.yaml so both version and appVersion are 1.2.0, add the required values configuration and template to provision dashboards/hyperfleet-api.json as part of the Helm release, and extend templates/NOTES.txt with instructions for accessing the deployed dashboard.Source: Path instructions
176-188: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake
API Statusdeterministic across replicas.
hyperfleet_api_build_inforeturns one series per scraped replica, andlastNotNullreduces each series independently. Aggregate the status query to one value and expose build versions separately to avoid duplicate or ambiguous Stat values during rollouts.🤖 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` around lines 176 - 188, Update the `API Status` panel target using `hyperfleet_api_build_info` to aggregate replica series into a single status value, and add a separate query or panel representation for exposing build versions. Ensure the Stat visualization no longer receives duplicate or ambiguous values during rollouts, while retaining the existing datasource and dashboard context.Source: MCP tools
🤖 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 531-532: Remove the noValue override from the reconciliation
dashboard panels and update their descriptions and docs/metrics.md to reflect
the no-data behavior when ReconciliationCollector.Collect emits no samples;
alternatively, add and test a PromQL fallback that preserves accurate
zero-versus-no-data semantics.
---
Outside diff comments:
In `@charts/dashboards/hyperfleet-api.json`:
- Around line 1159-1200: Update the chart metadata in Chart.yaml so both version
and appVersion are 1.2.0, add the required values configuration and template to
provision dashboards/hyperfleet-api.json as part of the Helm release, and extend
templates/NOTES.txt with instructions for accessing the deployed dashboard.
- Around line 176-188: Update the `API Status` panel target using
`hyperfleet_api_build_info` to aggregate replica series into a single status
value, and add a separate query or panel representation for exposing build
versions. Ensure the Stat visualization no longer receives duplicate or
ambiguous values during rollouts, while retaining the existing datasource and
dashboard context.
🪄 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: bc05e32c-dca7-46db-88d6-2e330a8af715
📒 Files selected for processing (2)
CHANGELOG.mdcharts/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)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.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 `@cmd/hyperfleet-api/server/logging/request_logging_middleware.go`:
- Around line 33-35: Update the header logging condition in the request logging
middleware to verify that masking is enabled, not merely that masker is non-nil.
Add or reuse an Enabled method exposing the masking state from the masking
middleware, and only append masked headers when it reports enabled; otherwise
omit headers entirely.
🪄 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: b4aa7c0e-e993-407d-ba13-12f444f09cb1
📒 Files selected for processing (3)
CHANGELOG.mdcharts/dashboards/hyperfleet-api.jsoncmd/hyperfleet-api/server/logging/request_logging_middleware.go
🔗 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 (2)
- CHANGELOG.md
- charts/dashboards/hyperfleet-api.json
| 3. Select your Prometheus datasource from the dropdown | ||
| 4. Click **Import** | ||
|
|
||
| The dashboard uses a parameterized `datasource` variable (no hardcoded UIDs) so it works across environments. Reconciliation gauge panels (pending, stuck, max stuck duration) fall back to 0 when no resources match the query. |
There was a problem hiding this comment.
Update comment with no data instead of fallback to 0
There was a problem hiding this comment.
Hmm, these paChanged
| } | ||
| ], | ||
| "title": "Resources Pending Reconciliation", | ||
| "description": "Live-query gauge — falls back to 0 via PromQL when no resources are pending.", |
There was a problem hiding this comment.
doesn't it not fall back to 0 now? Update 6,7, 8?
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ma-hill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
e8fb197
into
openshift-hyperfleet:main
3 participants
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