HYPERFLEET-1363 - feat: add Grafana dashboard provisioning to Helm chart - #241
HYPERFLEET-1363 - feat: add Grafana dashboard provisioning to Helm chart#241Ruclo wants to merge 1 commit 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
WalkthroughThis pull request adds eight Prometheus-backed Grafana panels for HyperFleet Sentinel metrics. Helm creates a Grafana-discoverable ConfigMap when dashboard provisioning is enabled. Tracing moves under Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Helm
participant ConfigMap
participant Grafana
participant Prometheus
Helm->>ConfigMap: Render dashboard when monitoring.dashboard.enabled=true
ConfigMap->>Grafana: Apply dashboard discovery labels
Grafana->>Prometheus: Query Sentinel metrics using dashboard variables
Prometheus-->>Grafana: Return filtered metric series
Grafana-->>Grafana: Render eight dashboard panels
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Risk Score: 1 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 208 lines (>200) | +1 |
| 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: 3
🤖 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/values.yaml`:
- Line 244: Preserve backward compatibility for the renamed monitoring values:
update the ServiceMonitor templates to fall back from
monitoring.serviceMonitor.labels to monitoring.serviceMonitor.additionalLabels,
and support the previous top-level tracing path alongside monitoring.tracing
with an explicit precedence rule. Apply the same compatibility handling wherever
these values are consumed, or clearly mark the chart change as breaking and
provide migration guidance.
- Line 244: Update charts/values.schema.json to remove the obsolete top-level
tracing definition and add schemas for monitoring.tracing, monitoring.dashboard,
and monitoring.serviceMonitor.labels matching the corresponding
charts/values.yaml paths. Define obsolete-key handling so unsupported
configuration keys are rejected or otherwise explicitly validated, preventing
unvalidated chart values.
- Around line 271-291: Update charts/Chart.yaml to bump the Helm chart version,
add CHANGELOG.md entries covering dashboard provisioning, tracing,
ServiceMonitor configuration renames, and the parentbased_always_on sampler
default, marking configuration renames as BREAKING CHANGE, then regenerate
charts/README.md so documented paths and defaults match the current values
configuration.
🪄 Autofix
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: 39877c45-1aee-42c7-9e54-cfdaca476f88
📒 Files selected for processing (5)
charts/dashboards/sentinel-metrics.jsoncharts/templates/configmap-dashboard.yamlcharts/templates/deployment.yamlcharts/templates/servicemonitor.yamlcharts/values.yaml
🔗 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)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/templates/configmap-dashboard.yaml (1)
10-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFail fast if the dashboard file is missing.
.Files.Getreturns an empty string when the path does not resolve. It does not raise an error. Ifdashboards/sentinel-metrics.jsongets renamed or moved, this template silently renders a ConfigMap with emptydata. The Grafana sidecar then finds a dashboard entry with no content, and nobody gets a signal at deploy time.Wrap the call with
requiredto fail the render instead.Proposed fix
data: sentinel-metrics.json: |- - {{- .Files.Get "dashboards/sentinel-metrics.json" | nindent 4 }} + {{- required "dashboards/sentinel-metrics.json not found in chart" (.Files.Get "dashboards/sentinel-metrics.json") | nindent 4 }}🤖 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/templates/configmap-dashboard.yaml` around lines 10 - 11, Update the dashboard content expression under the sentinel-metrics.json ConfigMap key to wrap .Files.Get for dashboards/sentinel-metrics.json with Helm’s required helper, supplying a descriptive missing-file message so template rendering fails when the file is absent while preserving the existing nindent formatting.
🤖 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: Update the chart version in Chart.yaml from 1.1.0 to 2.0.0 to reflect
the breaking values contract, then regenerate the associated version badge and
chart documentation so they match the new major version.
---
Nitpick comments:
In `@charts/templates/configmap-dashboard.yaml`:
- Around line 10-11: Update the dashboard content expression under the
sentinel-metrics.json ConfigMap key to wrap .Files.Get for
dashboards/sentinel-metrics.json with Helm’s required helper, supplying a
descriptive missing-file message so template rendering fails when the file is
absent while preserving the existing nindent formatting.
🪄 Autofix
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: e1bbb2d2-7b8c-4c6c-af01-56a07aba5f38
📒 Files selected for processing (9)
CHANGELOG.mdcharts/Chart.yamlcharts/README.mdcharts/dashboards/sentinel-metrics.jsoncharts/templates/configmap-dashboard.yamlcharts/templates/deployment.yamlcharts/templates/servicemonitor.yamlcharts/values.schema.jsoncharts/values.yaml
🔗 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 (4)
- charts/templates/deployment.yaml
- charts/values.yaml
- charts/dashboards/sentinel-metrics.json
- charts/templates/servicemonitor.yaml
| description: HyperFleet Sentinel - Kubernetes service that polls HyperFleet API and publishes CloudEvents | ||
| type: application | ||
| version: 1.0.0 | ||
| version: 1.1.0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a major chart version for the breaking values contract.
Line 5 publishes 1.1.0, but this release removes the top-level tracing path and renames monitoring.serviceMonitor.additionalLabels. An upgrade within the 1.x range can silently lose tracing configuration or ServiceMonitor labels. Change the chart version to 2.0.0, then regenerate the version badge and chart documentation.
As per path instructions, charts/** changes affect all deployments and require a Chart.yaml version bump.
🤖 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, Update the chart version in Chart.yaml from
1.1.0 to 2.0.0 to reflect the breaking values contract, then regenerate the
associated version badge and chart documentation so they match the new major
version.
Source: Path instructions
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 `@docs/deployment.md`:
- Around line 62-66: Update all tracing-related command examples in
docs/deployment.md, especially the examples near the tracing configuration table
and the later tracing examples, to use the monitoring.tracing.* key prefix
instead of tracing.*. Keep the existing option names and example values
unchanged.
🪄 Autofix
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: 3f3d4610-3545-4e80-99a5-83bc70c18fff
📒 Files selected for processing (13)
CHANGELOG.mdcharts/Chart.yamlcharts/README.mdcharts/dashboards/sentinel-metrics.jsoncharts/templates/configmap-dashboard.yamlcharts/templates/deployment.yamlcharts/templates/servicemonitor.yamlcharts/values.schema.jsoncharts/values.yamldocs/deployment.mddocs/metrics.mddocs/runbook.mddocs/sentinel-for-gke-dev.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 (8)
- charts/templates/deployment.yaml
- CHANGELOG.md
- charts/Chart.yaml
- charts/templates/servicemonitor.yaml
- charts/values.yaml
- charts/README.md
- charts/values.schema.json
- charts/dashboards/sentinel-metrics.json
|
/retest |
…ng section Consolidate all observability configuration under a single monitoring: section for consistency across HyperFleet charts. - Move tracing from top-level to monitoring.tracing - Move dashboard JSON from deployments/dashboards/ to charts/dashboards/ - Add configmap-dashboard.yaml for Grafana sidecar provisioning - Rename serviceMonitor additionalLabels to labels - Simplify honorLabels handling (remove hasKey guard) - Default sampler to parentbased_always_on
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
01f0298 to
027125c
Compare
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 participant
Why
Part of HYPERFLEET-1363 — unify observability values under
monitoring:across all HyperFleet component charts and consolidate dashboard provisioning.Changes
tracingundermonitoring:sectioncharts/dashboards/, add ConfigMap template (grafana_dashboard: "1")additionalLabels→labelsparentbased_always_onTest plan
make test-helmpassesOBSERVABILITY_ENABLED=true— dashboard appears in Grafana