feat: add Grafana dashboard for operator observability - #265
Conversation
Ship an importable Grafana dashboard and optional sidecar provisioning overlay so cluster operators can visualize mcpserver_* and controller-runtime metrics introduced in kubernetes-sigs#100.
kubernetes-prow
Bot
added
the
do-not-merge/work-in-progress
✅ Deploy Preview for mcp-lifecycle-operator ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ibm-adarsh 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 |
kubernetes-prow
Bot
added
cncf-cla: yes
📝 WalkthroughWalkthroughAdds a Prometheus-backed Grafana dashboard with MCP lifecycle visualizations, Kustomize sidecar provisioning, automated JSON and metric validation, optional live Prometheus checks, and documentation for importing or deploying it. ChangesGrafana dashboard
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Makefile
participant VerifyScript as verify-grafana-dashboard.sh
participant Grafana as Grafana sidecar
participant Prometheus
Makefile->>VerifyScript: Run dashboard verification
VerifyScript->>VerifyScript: Validate JSON and run Go tests
Grafana->>Grafana: Load generated dashboard ConfigMap
Grafana->>Prometheus: Execute dashboard PromQL queries
Prometheus-->>Grafana: Return metric series
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 10
🤖 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 `@config/grafana/dashboard_test.go`:
- Around line 137-142: Update TestDashboardKustomizationBuilds to invoke the
Kustomize build/render process for config/grafana instead of only checking
dashboardFile exists. Assert that rendering succeeds and inspect the generated
ConfigMap contents, including the dashboard data and labels, so malformed YAML
or generated metadata causes the test to fail.
- Around line 90-123: Update the metric extraction logic in the selector
validation test to parse each expression with the PromQL parser and inspect all
vector selectors, including bare metric references such as function arguments.
Populate `seen` from the parsed selector names and validate them against
`knownMetrics`, replacing the regex-only approach while preserving existing
allowlist and unimplemented-metric checks.
In `@config/grafana/kustomization.yaml`:
- Around line 1-3: Update the Grafana provisioning documentation in the
kustomization.yaml comments to state that grafana_folder requires
sidecar.dashboards.folderAnnotation set to grafana_folder and
sidecar.dashboards.provider.foldersFromFilesStructure enabled, or remove the
grafana_folder annotation if those chart settings are not guaranteed.
In `@config/grafana/mcp-lifecycle-operator.json`:
- Around line 893-896: Update the Grafana panel query configuration’s
legendFormat to reference the grouped reason label instead of the absent status
label, so each pie slice is labeled by its failure reason.
- Line 991: Update the unit settings for the Performance panels at the entries
around lines 991, 1047, and 1103: use a CPU/core unit for CPU usage, bytes for
RSS, and a count unit for active workers instead of ops. Preserve the existing
panel configuration and change only the metric-specific unit values.
- Line 1018: Update the process metric queries near the referenced dashboard
panels, including process_cpu_seconds_total, process_resident_memory_bytes, and
rest_client_requests_total, to include the existing operator label filter used
elsewhere in the dashboard. Preserve each query’s current aggregation and rate
behavior while scoping all series to the operator target.
- Around line 2-10: Replace the import-only DS_PROMETHEUS reference throughout
the dashboard with a datasource UID supported by provisioning, or configure a
runtime datasource variable that resolves during dashboard execution. Update all
panels and the mcpserver_namespace variable to use the same resolvable
datasource reference, and remove reliance on the __inputs placeholder.
In `@hack/verify-grafana-dashboard.sh`:
- Around line 34-37: Update the curl invocation in the query loop of
hack/verify-grafana-dashboard.sh to include both --connect-timeout and
--max-time with appropriate bounded durations, while preserving the existing
silent/fail behavior and Prometheus query URL.
In `@site-src/operating/metrics.md`:
- Line 154: Update the metrics sentence in the dashboard documentation to
include process metrics process_cpu_seconds_total and
process_resident_memory_bytes, plus the REST client metric
rest_client_requests_total, alongside the existing mcpserver_* and
controller_runtime_* series. State that these metrics must also be available in
Prometheus, while retaining the note that unimplemented ConfigMap/Secret watch
metrics are excluded.
- Around line 169-171: Add repository checkout prerequisites before the kubectl
apply command in the metrics documentation: instruct readers to clone the
repository and change to its root directory, or replace the local Kustomize path
with a valid remote Kustomize URL.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ef6899e-f83d-46a0-9bb6-08324437ac2a
📒 Files selected for processing (7)
MakefileREADME.mdconfig/grafana/dashboard_test.goconfig/grafana/kustomization.yamlconfig/grafana/mcp-lifecycle-operator.jsonhack/verify-grafana-dashboard.shsite-src/operating/metrics.md
Use PromQL parser validation, real kustomize build tests, datasource template variables, corrected panel units/legends, operator namespace scoping, curl timeouts, and expanded metrics documentation.
Use strings.SplitSeq for modernize lint compliance and ensure kustomize is installed before verify-grafana-dashboard runs in CI.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #265 +/- ##
=======================================
Coverage ? 85.36%
=======================================
Files ? 14
Lines ? 1578
Branches ? 0
=======================================
Hits ? 1347
Misses ? 188
Partials ? 43 🚀 New features to boost your workflow:
|
|
/ok-to-test |
kubernetes-prow
Bot
added
the
ok-to-test
ibm-adarsh
changed the title
kubernetes-prow
Bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@config/grafana/dashboard_test.go`:
- Around line 147-149: Remove the t.Helper() call from the top-level
TestDashboardKustomizationBuilds function, leaving the rest of the test
unchanged so failures report the correct test source location.
In `@go.mod`:
- Line 12: Update the github.com/prometheus/prometheus dependency in go.mod from
v0.303.0 to the latest patched version, refreshing go.sum as needed. Preserve
its existing test-only usage and avoid unrelated dependency changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a526de1e-0466-4947-85f0-236c57677545
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
Makefileconfig/grafana/dashboard_test.goconfig/grafana/kustomization.yamlconfig/grafana/mcp-lifecycle-operator.jsongo.modhack/verify-grafana-dashboard.shsite-src/operating/metrics.md
🚧 Files skipped from review as they are similar to previous changes (4)
- config/grafana/kustomization.yaml
- Makefile
- site-src/operating/metrics.md
- hack/verify-grafana-dashboard.sh
| func TestDashboardKustomizationBuilds(t *testing.T) { | ||
| t.Helper() | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove t.Helper() from the top-level test function.
Calling t.Helper() inside a TestXxx function marks the test itself as a helper. If a failure occurs (e.g., t.Fatalf), the testing framework will skip this function in the call stack and report the failure's location as being inside the Go standard library's internal testing.go runner. This makes it difficult to pinpoint the exact line that failed in the test.
♻️ Proposed fix
func TestDashboardKustomizationBuilds(t *testing.T) {
- t.Helper()
-
kustomize := filepath.Join("..", "..", "bin", "kustomize")📝 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.
| func TestDashboardKustomizationBuilds(t *testing.T) { | |
| t.Helper() | |
| func TestDashboardKustomizationBuilds(t *testing.T) { | |
| kustomize := filepath.Join("..", "..", "bin", "kustomize") |
🤖 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 `@config/grafana/dashboard_test.go` around lines 147 - 149, Remove the
t.Helper() call from the top-level TestDashboardKustomizationBuilds function,
leaving the rest of the test unchanged so failures report the correct test
source location.
| github.com/onsi/ginkgo/v2 v2.31.0 | ||
| github.com/onsi/gomega v1.42.0 | ||
| github.com/prometheus/client_golang v1.23.2 | ||
| github.com/prometheus/prometheus v0.303.0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Update the Prometheus dependency to address high-severity vulnerabilities.
OSV Scanner identified multiple high-severity vulnerabilities (e.g., GHSA-8rm2-7qqf-34qm, GHSA-fw8g-cg8f-9j28) in github.com/prometheus/prometheus v0.303.0. Although this dependency is only used for unit testing and may not be exploitable in production, having vulnerable packages in go.mod can trigger security scanner alerts and fail CI/compliance pipelines.
Please update github.com/prometheus/prometheus to the latest patched version (e.g., by running go get github.com/prometheus/prometheus@latest).
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus: Remote read endpoint allows denial of service via crafted snappy payload in github.com/prometheus/prometheus
(GO-2026-5264)
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus vulnerable to stored XSS via crafted histogram bucket label values in the old web UI heatmap display in github.com/prometheus/prometheus
(GO-2026-5381)
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus has Stored XSS via metric names and label values in Prometheus web UI in github.com/prometheus/prometheus
(GO-2026-5662)
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus Azure AD remote write OAuth client secret exposed via config API in github.com/prometheus/prometheus
(GO-2026-5710)
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus: Remote read endpoint allows denial of service via crafted snappy payload
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus vulnerable to stored XSS via crafted histogram bucket label values in the old web UI heatmap display
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus has Stored XSS via metric names and label values in Prometheus web UI tooltips and metrics explorer
[HIGH] 12-12: github.com/prometheus/prometheus 0.303.0: Prometheus Azure AD remote write OAuth client secret exposed via config API
🤖 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 `@go.mod` at line 12, Update the github.com/prometheus/prometheus dependency in
go.mod from v0.303.0 to the latest patched version, refreshing go.sum as needed.
Preserve its existing test-only usage and avoid unrelated dependency changes.
Source: Linters/SAST tools
ArangoGutierrez
left a comment
ArangoGutierrez
left a comment
There was a problem hiding this comment.
Found 2 issues.
- Eight queries filter on
namespace=~"$mcpserver_namespace", buttemplating.listonly definesdatasourceandoperator_namespace- Grafana passes an undefined variable through as a literal, so these panels (MCPServer count, Accepted/Ready breakdowns, failure and validation rates) render no data on import. Adding themcpserver_namespacequery variable the PR description already mentions (sourced frommcpserver_condition_info) fixes all eight. (config/grafana/mcp-lifecycle-operator.json:91) - This checks the raw JSON text, so a metric name lingering in a panel title or legend keeps the test green even after its query is deleted. The
seenset computed below already holds the selectors actually parsed from everyexpr- checkingrequiredagainstseeninstead closes the gap. (config/grafana/dashboard_test.go:109)
| "uid": "${datasource}" | ||
| }, | ||
| "editorMode": "code", | ||
| "expr": "count(mcpserver_condition_info{type=\"Ready\", namespace=~\"$mcpserver_namespace\"})", |
There was a problem hiding this comment.
Eight queries filter on namespace=~"$mcpserver_namespace", but templating.list only defines datasource and operator_namespace - Grafana passes an undefined variable through as a literal, so these panels (MCPServer count, Accepted/Ready breakdowns, failure and validation rates) render no data on import. Adding the mcpserver_namespace query variable the PR description already mentions (sourced from mcpserver_condition_info) fixes all eight.
| "controller_runtime_reconcile_time_seconds", | ||
| } | ||
| for _, metric := range required { | ||
| if !strings.Contains(content, metric) { |
There was a problem hiding this comment.
This checks the raw JSON text, so a metric name lingering in a panel title or legend keeps the test green even after its query is deleted. The seen set computed below already holds the selectors actually parsed from every expr - checking required against seen instead closes the gap.
2 participants
Summary
Adds a Grafana dashboard for the MCP Lifecycle Operator as a distributable artifact, addressing #102. Builds on the custom
mcpserver_*metrics from #100 (PR #122) and the metrics documentation from #101 (PR #138).config/grafana/mcp-lifecycle-operator.json— importable dashboard with 5 rows and 19 PromQL panelsconfig/grafana/kustomization.yamlfor Grafana Helm sidecar provisioning (grafana_dashboard: "1")config/grafana/dashboard_test.goandhack/verify-grafana-dashboard.shto guard JSON validity and metric referencesmake verify-grafana-dashboardintomake testsite-src/operating/metrics.mdWhy this approach
Before implementing, I surveyed how kubernetes-sigs and adjacent operator projects distribute Grafana dashboards:
examples/grafana-dashboard.json+ raw URL import in docsgrafana/*.json+ manual UI importconfig/grafana/placementconfig/grafana/dashboard.jsonhack/observability/grafana/dashboards/(Tilt dev only)grafana_dashboard: "1"sidecar labelDecision: Ship raw JSON at
config/grafana/(NFD/kubebuilder pattern) with an optional Kustomize overlay for Grafana sidecar users (Strimzi/kube-prometheus-stack pattern). This avoids coupling the operator install to a specific monitoring stack while still supporting auto-provisioning for users who want it.What is included vs. excluded from the original #102 spec
mcpserver_condition_infocontroller_runtime_reconcile_*controller_runtime_reconcile_time_seconds_bucketmcpserver_condition_infomcpserver_*_failures_totalmcpserver_validation_failures_totalprocess_*,rest_client_requests_totalworkqueue_*seriesreferenced_resources_countDashboard design notes
controller="mcpserver"(matchesNamed("mcpserver")in the reconciler)$mcpserver_namespaceis sourced frommcpserver_condition_infolabelsprocess_*,rest_client_*) omitnamespacelabel filters because the raw/metricsendpoint does not include Kubernetes labels — these are added by Prometheus relabeling if configuredTest plan
make build— binary builds successfullymake test— all unit tests pass (controller coverage 91.4%), includesverify-grafana-dashboardgo test ./config/grafana/...— JSON validity, required metric references, no unimplemented metricsTestMetricsEndpoint— metrics endpoint returns HTTP 200 via authenticated port-forwardTestManagerPodRunning— controller-manager pod is Runningmcpserver_condition_info,controller_runtime_reconcile_total,process_*series present after reconciling MCPServerskustomize build config/grafana/— produces ConfigMap withgrafana_dashboard: "1"labelCloses #102
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation