feat(observability): ship Grafana dashboards and Prometheus alerts

[sourcehawk]

Description

Closes #183

Operators built on the framework now get Grafana dashboards and Prometheus alert rules for the metrics the framework records (#182) and for controller-runtime's own series, including the alert the apply counters were built for: a managed resource rewritten on every reconcile. Everything lives under observability/ as templates rendered with make dashboards|alerts METRIC_NAMESPACE=<ns>, is unit tested with promtool and a Go template lint, and can be inspected on a local Prometheus + Grafana stack fed by a simulator that drives the real pkg/metrics recorder. One expectation is placed on existing operators: the controller name passed to metrics.NewRecorder must equal the controller-runtime controller name (lower-cased kind unless Named overrides it), or the dashboards cannot correlate the framework's series with controller-runtime's; the docs, GoDoc and the repository's own e2e suites now say and do so.

Changes

• Alert rules: CustomResourceNotReady, CustomResourceConditionUnknown, CustomResourceConditionStuck (per operator, rendered per metric namespace); ManagedResourceNotConverging, ManagedResourceApplyFailing, ControllerReconcileErrors, ControllerReconcilePanics, ControllerWorkqueueBacklog, ControllerReconcileLatencyHigh, OperatorLeaderMissing (shared, installed once per cluster as ocf-managed-resources and ocf-controller-runtime). Ratio thresholds with floors so legitimate churn at scale stays silent; quantile thresholds on real histogram bucket bounds; aggregation by the operator's namespace so two installs alert separately.
• Two dashboards with metric-namespace-templated uids: an operator overview (reconciliation, workqueue, managed resource applies, condition summary, API client, process, all filtered by one controller variable) and the per-owner conditions browser ported from go-crd-condition-metrics with stale-series protection (topk by (kind, id)), consistent filters, a wired resource variable that the alerts' dashboard_url deep links use, and wildcard All values so cluster-scoped owners stay visible.
• Render pipeline in the root Makefile (dashboards, alerts, test-alerts, lint-dashboards, observability-up, observability-down) with NAMESPACE_LABEL, ALERT_FORMAT=prometheusrule|rules, PROMETHEUSRULE_NAMESPACE and PROMETHEUSRULE_LABELS; output under the gitignored observability/generated/.
• Local stack: docker compose Prometheus and Grafana bound to localhost, a Go simulator playing a scripted world (healthy owners, a hot-loop resource, stuck, Unknown, reason-flipping and cluster-scoped owners, failing applies, erroring and slow reconciles, a backlogged workqueue, leader election), and a parity test that starts a real unmanaged controller-runtime controller to assert the simulator's lookalike series match names, labels and buckets.
• CI: an observability job runs the promtool tests and the template lint.
• Docs: consumer-facing docs/observability.md (site nav, index, README table, AI instructions, plugin skill reference); observability/README.md as the maintainer guide; docs/component.md and pkg/metrics GoDoc state the controller-name rule.

Related

• Sub-PRs on this branch, each reviewed and self-merged: feat(observability): add the alert render pipeline, alert templates and promtool tests #188 (pipeline and alerts), feat(observability): add a local Prometheus and Grafana stack fed by a metrics simulator #189 (dev stack and simulator), feat(observability): add the operator overview and conditions browser dashboards #190 (dashboards and template lint), docs(observability): document the dashboards, alerts and local stack #191 (docs).
• Follow-up: dashboard screenshots for docs/observability.md, captured by hand from the local stack.

Testing

make all, make test-alerts (promtool lint under both namespace label variants plus 15 unit test cases including the negative cases: churn at scale and a single edit do not fire ManagedResourceNotConverging, sporadic conflicts do not fire ManagedResourceApplyFailing, a second healthy install does not mask an erroring one, a reason change does not reset the for: clock), make lint-dashboards, go test ./observability/... (parity and scripted-world tests), make docs-build in strict mode, and make sync-plugin plus make ai-instructions leaving the tree clean, all pass on this branch. Every panel of both dashboards and every alert was exercised against live data on the local stack (make observability-up): all 42 panel targets return series, the expected nine alerts fire within minutes with OperatorLeaderMissing firing under -leader=false, the browser's filters and the alert deep links narrow to one owner, and the cluster-scoped owner is visible under All. To poke at it yourself: make observability-up, then open http://localhost:3000 and http://localhost:9090/alerts.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JPihvXVfS997iGmGabTGsd

[Copilot]

🟡 Changes recommended

Stale condition series can trigger false alerts, shared rules can merge unrelated operators, and some valid metric namespaces produce rejected Grafana dashboards.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a complete observability package for OCF operators, including dashboards, alerts, rendering, validation, local simulation, and documentation.

Changes:

• Adds Grafana dashboards and Prometheus alert rules with tests.
• Adds a local Prometheus/Grafana stack and metrics simulator.
• Documents rendering, installation, controller naming, and maintenance.

File summaries

File	Description
README.md	Links observability documentation.
plugin/skills/building-components/SKILL.md	Adds observability guidance.
plugin/skills/building-components/references/observability.md	Syncs observability documentation.
plugin/skills/building-components/references/component.md	Documents controller-name matching.
pkg/metrics/metrics.go	Updates recorder GoDoc.
observability/README.md	Adds maintainer instructions.
observability/observability_test.go	Validates templates and metrics.
observability/dev/simulator/world.go	Implements simulated scenarios.
observability/dev/simulator/world_test.go	Tests simulator output.
observability/dev/simulator/runtime.go	Defines runtime metric lookalikes.
observability/dev/simulator/runtime_test.go	Verifies metric parity.
observability/dev/simulator/main.go	Runs the simulator endpoint.
observability/dev/prometheus/prometheus.yml	Configures local Prometheus.
observability/dev/grafana/provisioning/datasources/prometheus.yaml	Provisions Prometheus in Grafana.
observability/dev/grafana/provisioning/dashboards/ocf.yaml	Provisions dashboards.
observability/dev/docker-compose.yaml	Defines the local stack.
observability/dashboards/crd_conditions_browser.tpl.json	Adds the conditions browser.
observability/dashboards/ocf_operator.tpl.json	Adds the operator overview.
observability/alerts/tests/managed_resources_test.yaml	Tests resource alerts.
observability/alerts/tests/crd_conditions_test.yaml	Tests condition alerts.
observability/alerts/tests/controller_runtime_test.yaml	Tests runtime alerts.
observability/alerts/managed_resources.yaml	Adds managed-resource alerts.
observability/alerts/crd_conditions.tpl.yaml	Adds condition alerts.
observability/alerts/controller_runtime.yaml	Adds controller-runtime alerts.
mkdocs.yml	Adds observability navigation.
Makefile	Adds rendering and local-stack targets.
go.mod	Promotes the Prometheus model dependency.
e2e/primitives/suite_test.go	Aligns recorder controller names.
e2e/component/suite_test.go	Aligns recorder controller name.
docs/observability.md	Documents the observability package.
docs/index.md	Adds an observability card.
docs/component.md	Documents controller-name correlation.
.gitignore	Ignores rendered artifacts.
.github/workflows/test.yml	Adds observability CI checks.
.github/copilot-instructions.md	Registers observability guidance.
.ai/base.md	Updates generated AI guidance.

Review details

Suppressed comments (2)

observability/alerts/crd_conditions.tpl.yaml:75

• Filtering to Unknown before max() means an Unknown series from a former leader remains eligible forever, even when the active leader now exports True or False. First choose the freshest series per controller/kind/id/condition across every status, then filter to Unknown; cover recovery while the stale pod continues to be scraped.

          max (
            {{operator_namespace}}controller_condition{status="Unknown"}
          ) by (controller, kind, name, condition, {{namespace_label}})

observability/alerts/crd_conditions.tpl.yaml:132

• This also filters out the current Ready=True series before stale-series selection. Consequently, any old non-True series still exported by a former leader can make the owner look stuck indefinitely. Select the freshest Ready series across statuses first, and only then test status!="True"; add a stale-non-True/current-True test case.

            max (
              {{operator_namespace}}controller_condition{condition="Ready", status!="True"}
            ) by (controller, kind, name, condition, {{namespace_label}})

• Files reviewed: 34/36 changed files
• Comments generated: 4
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sourcehawk]

Disposition of the two findings Copilot's review of cf3287c suppressed (no thread to reply in):

• observability/alerts/crd_conditions.tpl.yaml:75 (CustomResourceConditionUnknown filters to Unknown before max(), so a former leader's stale Unknown series stays eligible): applied in cca1e25. The rule joins on the freshest series per owner and condition across every status first.
• observability/alerts/crd_conditions.tpl.yaml:132 (CustomResourceConditionStuck filters out Ready=True before selecting the freshest series): applied in cca1e25, same join keyed on (controller, kind, name, condition, <namespace label>). TestStaleFormerLeaderSeriesDoNotFire covers the stale-non-True / current-True case for this rule at the 7h mark.

[Copilot]

🟡 Changes recommended

The renderer can retain obsolete artifacts and accepts an unvalidated namespace-label identifier.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

Makefile:249

• NAMESPACE_LABEL is substituted directly into PromQL as a label identifier, but it is never validated. A typo such as NAMESPACE_LABEL=exported-namespace therefore makes both render targets succeed while producing invalid queries. Validate it against the Prometheus label-name grammar before rendering.

sed -e 's/{{operator_namespace}}/$(2)_/g' -e 's/{{namespace_label}}/$(3)/g' $(1)

Makefile:273

• The alert output directory is not cleaned before rendering. After a rule file is removed or renamed, its old generated YAML remains and the documented kubectl apply -f observability/generated/alerts/ command continues installing obsolete alerts. Remove generated YAML files before writing the current rule set.

	@mkdir -p $(OBS_OUT)/alerts

Makefile:262

• This directory is only created, so dashboards removed or renamed by a framework upgrade remain in the gitignored output. The documented --from-file=observability/generated/dashboards/ install then republishes that stale dashboard. Clear generated JSON files before rendering the current template set.

	@mkdir -p $(OBS_OUT)/dashboards

• Files reviewed: 34/36 changed files
• Comments generated: 1
• Review effort level: Balanced

[sourcehawk]

Disposition of the three findings Copilot's review of ef05bb8 suppressed (no thread to reply in), all applied in 272c982:

• Makefile:249 (NAMESPACE_LABEL substituted into PromQL unvalidated): require_metric_namespace now rejects a label that does not match the Prometheus label name grammar ^[A-Za-z_][A-Za-z0-9_]*$, with a message naming it. Documented in the NAMESPACE_LABEL row of docs/observability.md.
• Makefile:273 (stale rule files survive in generated/alerts/): make alerts removes generated/alerts/*.yaml before rendering.
• Makefile:262 (stale dashboards survive in generated/dashboards/): make dashboards removes generated/dashboards/*.json before rendering. observability-render-dev relies on the same cleanup instead of its own rm. Documented next to the output layout in docs/observability.md.

[Copilot]

🔵 Needs a closer look

Equal LastTransitionTime values can leave dashboard reason filtering stale, and the simulator currently masks that behavior.

Review details

Suppressed comments (6)

Previously missed (2) — in code that hasn't changed since the last review.

observability/dev/simulator/world.go:242

• A reason-only update does not change a Kubernetes condition's LastTransitionTime; the framework's meta.SetStatusCondition call preserves it while the status remains False. Resetting flipSince here makes the simulator's Since value and stale-series behavior unlike a real operator, and hides equal-timestamp old/new reason series from local validation. Keep the original transition time when only the reason flips.

			flipSince = time.Now()

observability/dashboards/crd_conditions_browser.tpl.json:115

• The reason filter is not stale-safe when old and current pods export different reasons with the same LastTransitionTime (the normal case for a reason-only update). topk(1) may select the old reason on the right, causing the current reason's series on the left not to match and making this total incorrect. Use an independent per-pod freshness/leadership signal and cover equal-valued reason series in a dashboard query test.

This issue also appears in the following locations of the same file:

• line 247
• line 397
• line 468
• line 543

          "expr": "count(\n    {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", status=~\"$status\", reason=~\"$reason\", id=~\"$resource_id\", {{namespace_label}}=~\"$namespace\"} > 0\n  and\n    topk by (id, condition) (1, {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", {{namespace_label}}=~\"$namespace\"})\n)",

observability/dashboards/crd_conditions_browser.tpl.json:247

• This stale-series join cannot reliably select the current reason. The metric value is LastTransitionTime, and the framework uses meta.SetStatusCondition (pkg/component/conditions.go:161), which preserves that timestamp when only Reason changes. After a leader change, the old and current reason series can therefore tie, so topk(1) may make the table show or filter on the stale reason indefinitely. The dashboard needs an independent freshness/leadership signal, plus a regression case where the two pods have equal metric values.

          "expr": "sum(\n  {{operator_namespace}}controller_condition{\n    kind=\"$kind\",\n    condition=~\"$condition\",\n    status=~\"$status\",\n    reason=~\"$reason\",\n    id=~\"$resource_id\",\n    {{namespace_label}}=~\"$namespace\"\n  }\n  and\n  topk by (id, condition) (1, {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", {{namespace_label}}=~\"$namespace\"})\n) by (name, {{namespace_label}}, condition, status, reason)",

observability/dashboards/crd_conditions_browser.tpl.json:397

• This reason-filtered count can choose a stale pod's reason because reason-only changes retain LastTransitionTime, leaving topk(1) tied. Selecting a specific current reason can therefore show zero (or the stale reason can remain selected) after failover. Join against an independent freshness/leader signal and test equal-valued old/new reason series.

              "expr": "count(\n    {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", status=\"False\", reason=~\"$reason\", id=~\"$resource_id\", {{namespace_label}}=~\"$namespace\"} > 0\n  and\n    topk by (id, condition) (1, {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", {{namespace_label}}=~\"$namespace\"})\n)",

observability/dashboards/crd_conditions_browser.tpl.json:468

• This reason-filtered count has the same equal-timestamp failure: a reason-only update does not advance LastTransitionTime, so topk(1) can retain the former leader's reason and reject the current series. Use a separate freshness/leadership signal rather than the transition timestamp for pod deduplication.

              "expr": "count(\n    {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", status=\"Unknown\", reason=~\"$reason\", id=~\"$resource_id\", {{namespace_label}}=~\"$namespace\"} > 0\n  and\n    topk by (id, condition) (1, {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", {{namespace_label}}=~\"$namespace\"})\n)",

observability/dashboards/crd_conditions_browser.tpl.json:543

• Because reason-only changes preserve LastTransitionTime, the old and current reason series can tie here and topk(1) may select the stale one. A specific reason filter can consequently report the wrong True count after leader failover. Deduplicate using an independent freshness/leader signal and add an equal-timestamp regression.

              "expr": "count(\n    {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", status=\"True\", reason=~\"$reason\", id=~\"$resource_id\", {{namespace_label}}=~\"$namespace\"} > 0\n  and\n    topk by (id, condition) (1, {{operator_namespace}}controller_condition{kind=\"$kind\", condition=~\"$condition\", {{namespace_label}}=~\"$namespace\"})\n)",

• Files reviewed: 34/36 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[sourcehawk]

Copilot's review of 272c982 posted no comments and suppressed six, all in code unchanged since the first review. Review loop cap (three rounds) reached; these are left open for a design decision rather than addressed here.

• observability/dashboards/crd_conditions_browser.tpl.json lines 115, 247, 397, 468, 543 (one finding): the topk by (id, condition) (1, ...) stale-series join keys on LastTransitionTime, and meta.SetStatusCondition (which applyStatusCondition uses) preserves that timestamp when only reason changes. After a leader change, a former leader still being scraped can therefore export the previous reason with the same value as the current leader's series, and topk(1) picks one of them arbitrarily, so a reason-filtered panel may show or hide the stale reason. Verified as real, but narrow: it needs a former leader still scraped and a reason-only change since failover. The alert rules are not affected: a tie implies equal status, and every rule drops reason before max(). The suggested fix (dedupe on a leadership signal such as leader_election_master_status joined on the scrape target) needs a fallback for operators that run without leader election, so it is a design choice for a follow-up.
• observability/dev/simulator/world.go:242: the simulator resets flipSince on a reason-only flip, so its series do not tie the way a real operator's would. Verified; belongs with the same follow-up so the local stack can reproduce the case above.

[Copilot]

🟡 Changes recommended

Alert wording, metadata validation, and simulator shutdown error handling need correction.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

observability/alerts/managed_resources.yaml:57

• The expression fires once the updated share is merely greater than 50%, but this notification says the resource is rewritten on every reconcile. A sustained 51% ratio satisfies the rule and would receive a materially stronger diagnosis than the data supports; describe it as repeatedly/mostly rewritten and update the exact annotation fixtures accordingly.

            {{ $labels.kind }} `{{ $labels.resource }}` of component `{{ $labels.component }}` ({{ $labels.owner_kind }}) is rewritten on every reconcile
          description: |
            Over the last 15 minutes, {{ $value | humanizePercentage }} of the applies of the `{{ $labels.kind }}` resource `{{ $labels.resource }}` in component `{{ $labels.component }}` of owner kind `{{ $labels.owner_kind }}` (controller `{{ $labels.controller }}`) updated the object. In steady state a converged resource applies as `none` on every pass, so a resource whose every apply is an update is being rewritten on every reconcile even though nothing changed.

Makefile:309

• A malformed label token without = is silently turned into a different valid label: for example, PROMETHEUSRULE_LABELS=release renders release: "release" instead of failing, so the Prometheus Operator may never select the rule. Validate each documented key=value entry before emitting it.

	          for kv in $$(echo "$(PROMETHEUSRULE_LABELS)" | tr ',' ' '); do \
	            echo "    $${kv%%=*}: \"$${kv#*=}\""; \
	          done; \

• Files reviewed: 36/38 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Render validation and condition-simulation accuracy still have unresolved correctness issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 36/38 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Condition alerts and the browser merge separate operator installations, and one accepted metric namespace produces an invalid PrometheusRule name.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

observability/dashboards/crd_conditions_browser.tpl.json:590

• The browser has no job variable, so all of its variables and panels query every installation exposing this metric namespace, unlike the job-scoped operator overview. When two installs have the same kind/id, the stale-series topk can select the other install, causing alert and overview deep links to show the wrong owner state. Add a job selector to the browser, scope all queries by it, and pass var-job from both link producers.

        "definition": "label_values({{operator_namespace}}controller_condition,kind)",

Makefile:250

• METRIC_NAMESPACE=_demo passes this validation, but the default alert render turns its rule name into -demo-crd-conditions; Kubernetes rejects that metadata.name because it starts with -. Either derive a DNS-safe PrometheusRule name for leading-underscore metric namespaces or reject that input, and keep the documented grammar/tests in sync.

@echo "$(METRIC_NAMESPACE)" | grep -Eq '^[A-Za-z_][A-Za-z0-9_]{0,16}$$' || { \
	echo "Error: METRIC_NAMESPACE '$(METRIC_NAMESPACE)' is not renderable."; \
	echo "It must match ^[A-Za-z_][A-Za-z0-9_]*$$ and be at most 17 characters, so that the"; \
	echo "dashboard uid <namespace>_crd_conditions_browser fits Grafana's 40 character limit."; \
	exit 1; \

observability/alerts/crd_conditions.tpl.yaml:91

• This topk and final aggregation merge identical condition identities across jobs. A newer non-Unknown series from another installation can therefore suppress CustomResourceConditionUnknown, and simultaneous alerts lose the installation identity. Retain a stable install label such as job in both grouping clauses and cover two jobs with conflicting statuses.

            topk by (controller, kind, name, condition, {{namespace_label}}) (
              1, {{operator_namespace}}controller_condition
            )
          ) by (controller, kind, name, condition, {{namespace_label}})

observability/alerts/crd_conditions.tpl.yaml:152

• The stuck-condition selection also omits the scrape job/install identity. For the same owner identity in two installations, a newer Ready=True series can win topk and suppress the genuinely stuck series, or two stuck installs can collapse into one alert. Keep a stable install label in both aggregations and add a cross-job test.

              topk by (controller, kind, name, condition, {{namespace_label}}) (
                1, {{operator_namespace}}controller_condition{condition="Ready"}
              )
            ) by (controller, kind, name, condition, {{namespace_label}})

• Files reviewed: 36/38 changed files
• Comments generated: 1
• Review effort level: Balanced

[sourcehawk]

Disposition of the findings the reviews of 02156c5 through 0843ddb suppressed (no thread to reply in):

• observability/alerts/managed_resources.yaml:57 (summary overstates a 51% ratio): applied in 681f260, same change as the posted comment on line 55.
• Makefile:309 (label entry without = rendered as key: "key"): applied in aeedca7, same change as the posted comment.
• Makefile:250 (_demo renders an invalid PrometheusRule name): applied in aeedca7, same change as the posted comment on line 249.
• observability/alerts/crd_conditions.tpl.yaml:91 and :152 (Unknown and Stuck rules merge installs): applied in c6c1257 together with the posted comment on line 42; job is in every grouping of all three rules.
• observability/dashboards/crd_conditions_browser.tpl.json:590 (browser has no job selector): applied in c6c1257. The browser gained an Operator (scrape job) variable, All by default so existing links keep working, every query is scoped by it, and both link producers (the operator dashboard's CRs not Ready table and the condition alerts' dashboard_url) pass var-job.

[Copilot]

🟡 Changes recommended

Stale reason selection remains nondeterministic, simulator scenarios lack coverage, and shutdown failures are hidden.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

observability/dev/simulator/main.go:72

• Shutdown can return an error (notably when its timeout expires), but the simulator currently reports success and hides that failure. Preserve the existing serve-error precedence, then return a wrapped shutdown error so make observability-up fails visibly when the server did not shut down cleanly.

	shutdownErr := srv.Shutdown(shutdown)

• Files reviewed: 36/38 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Namespace-label handling can misfilter dashboard conditions and merge alerts across operator installations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 36/38 changed files
• Comments generated: 2
• Review effort level: Balanced

[sourcehawk]

The review of 3608130 (stale head) suppressed one finding, observability/dev/simulator/main.go:72 (Shutdown error discarded): already applied in 09490e5.

[Copilot]

🔵 Needs a closer look

The public Make-based render pipeline lacks automated end-to-end coverage for its validation and output branches.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

Makefile:241

• The public render pipeline is not exercised by an automated test. observability_test.go reimplements only the two sed substitutions, so regressions in the required-value/grammar checks, ALERT_FORMAT branches, PrometheusRule wrapping, or metadata options can still pass CI. Add integration coverage that invokes make dashboards and make alerts for valid and invalid inputs, both namespace-label modes, both alert formats, and optional metadata.

define require_metric_namespace

• Files reviewed: 36/38 changed files
• Comments generated: 0 new
• Review effort level: Balanced