fix(crossplane-observability): derive the provider job label from the PodMonitor the chart ships

`crossplane.providers.job` defaulted to `crossplane-providers` — a plausible-looking
name that no PodMonitor can ever produce. Prometheus-operator derives a PodMonitor's
job label as `<namespace>/<name>`, so the chart's own provider scrape lands under
`crossplane-system/<release>-crossplane-observability-providers`, and the 11 rules
filtering on `job="crossplane-providers"` matched nothing. Enabling
`providerPodMonitor` therefore produced a working scrape and still-empty rules, with
nothing anywhere reporting a problem.

Fixed the way the core half already works — by deriving the coupling instead of asking
two settings to agree. `crossplane.core.job` names the Service, so core is
self-consistent by construction; `crossplane.providers.job` now defaults to empty,
meaning "the PodMonitor this chart ships", and is computed from the rendered object.
An explicit value still wins, which is the case that matters when a platform scrape
already covers the provider pods.

Guarded so it cannot regress: validate.sh step 1c walks the all-enabled render and
asserts every shipped monitor's job label is one the rules actually query — a
ServiceMonitor's being the Service name, a PodMonitor's being `<namespace>/<name>`.
Confirmed it fails on the old default before the fix. The promtool unit tests render
with an explicit job so their fixtures stay independent of the release name; they test
rule logic, step 1c tests the wiring.

This is the second half of making the addon work without per-cluster values: the
saap-catalog wrapper currently carries
`job: crossplane-system/crossplane-providers-and-functions`, which describes one
cluster's hand-applied PodMonitor and is wrong everywhere else. With this it can drop
the override entirely.

Refs SA-8539, #283

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: TehreemNisa

crossplane-observability/Chart.yaml

@@ -2,5 +2,5 @@ apiVersion: v2
 name: crossplane-observability
 description: A Helm chart for Crossplane observability (ServiceMonitor + PrometheusRule + Grafana dashboard), integrated with OpenShift user-workload monitoring.
 type: application
-version: 0.1.8
+version: 0.1.9
 appVersion: "0.0.1"

crossplane-observability/README.md

@@ -84,7 +84,7 @@ override:
 | Value | Default | Stakater Cloud / typical override |
 | --- | --- | --- |
 | `crossplane.core.job` | `crossplane` | `crossplane-metrics` |
-| `crossplane.providers.job` | `crossplane-providers` | `crossplane-system/crossplane-providers-and-functions` |
+| `crossplane.providers.job` | *empty* → derived from the chart's own PodMonitor | only when another scrape already covers the provider pods, e.g. `crossplane-system/crossplane-providers-and-functions` |
 | `crossplane.core.serviceMonitorSelector` | `{name:crossplane, component:metrics}` | match your metrics Service labels (only if you enable the chart's monitor) |
 | `crossplane.providers.selector` | `pkg.crossplane.io/revision: Exists` | match your provider pods (only if you enable the chart's monitor) |
 | `grafana.instanceSelector` | `{app: grafana}` | your instance's labels, e.g. `{dashboards: crossplane}` |
@@ -93,6 +93,11 @@ The `job` values are the important ones — **the alert/recording expressions fi
 so if they don't match what your Prometheus assigns, rules evaluate to empty. Find them with
 `count by (job)({__name__=~"crossplane_managed_resource_.+"})`.
 
+If you let the chart own the scrape (the `prometheus.monitors.*` toggles above), you do **not**
+need to set either job value: the core Service is named after `crossplane.core.job`, and
+`crossplane.providers.job` left empty is derived from the PodMonitor the chart renders. Both
+couplings are asserted by `./tests/validate.sh`, so they cannot drift apart silently.
+
 **Grafana gotchas (grafana-operator):** to change `grafana.instanceSelector` you must also null
 the default key, because Helm deep-merges maps — e.g. `--set grafana.instanceSelector.app=null
 --set grafana.instanceSelector.dashboards=crossplane`. And `spec.instanceSelector` is
@@ -219,7 +224,7 @@ These need cluster/Grafana context an agent can't safely guess:
 | `crossplane.core.name` | `crossplane` | Crossplane core release/fullname (core ServiceMonitor selector). |
 | `crossplane.core.job` | `crossplane` | `job` label core metrics land under (used in alert/recording expressions). |
 | `crossplane.providers.selector` | `pkg.crossplane.io/revision: Exists` | Label selector matching provider pods (PodMonitor). |
-| `crossplane.providers.job` | `crossplane-providers` | `job` label provider metrics land under. |
+| `crossplane.providers.job` | `""` | `job` label provider metrics land under. Empty means "the PodMonitor this chart ships", whose job label prometheus-operator derives as `<namespace>/<name>` — computed rather than restated, since a wrong guess renders fine and matches nothing. Set it only when another scrape already covers the provider pods (and then keep `providerPodMonitor` off). |
 | `crossplane.inventory.enabled` | `false` | Enable Claim/inventory rules (needs an exporter — see [example](docs/resource-state-metrics-example.yaml)). |
 | `crossplane.inventory.conditionMetricPattern` | `kube_customresource_crossplane_xr_.+_condition` | Regex (PromQL `__name__=~`) matching the exporter's one-hot condition metrics across **all** XR kinds (Story 4.1). |
 | `upjet.enabled` | `false` | Upjet providers present — enables Story 6.1 and the more-accurate Upjet TTR (Story 1.2). |

crossplane-observability/templates/_helpers.tpl

@@ -57,3 +57,23 @@ Crossplane runs in, so UWM can resolve the metrics endpoints.
 {{- define "crossplane-observability.namespace" -}}
 {{- .Values.global.namespace | default .Release.Namespace }}
 {{- end }}
+
+{{/*
+The `job` label provider metrics carry, used by every provider-scoped rule expression.
+
+Unset means "the PodMonitor this chart ships", whose job label prometheus-operator
+derives as `<namespace>/<podmonitor-name>` — so it is computed here rather than
+restated in values, because a hand-written guess at that string is wrong in a way
+nothing catches: the rules render fine and silently match no series.
+
+Set `crossplane.providers.job` explicitly only when some OTHER scrape already covers
+the provider pods (a platform PodMonitor, a differently-relabelled job); then this
+chart's own `providerPodMonitor` should stay disabled.
+*/}}
+{{- define "crossplane-observability.providersJob" -}}
+{{- if .Values.crossplane.providers.job -}}
+{{- .Values.crossplane.providers.job -}}
+{{- else -}}
+{{- printf "%s/%s-providers" (include "crossplane-observability.namespace" .) (include "crossplane-observability.fullname" .) -}}
+{{- end -}}
+{{- end }}

crossplane-observability/templates/prometheus/recording/slo-recording-rules.yaml

@@ -1,6 +1,6 @@
 {{- if .Values.prometheus.recordingRules.enabled }}
 {{- $core := .Values.crossplane.core.job }}
-{{- $prov := .Values.crossplane.providers.job }}
+{{- $prov := include "crossplane-observability.providersJob" . }}
 # SLO recording rules. Pre-compute the SLIs from the roadmap so error-budget burn is
 # directly queryable, alert expressions stay readable, and the headline composite SLO
 # (Story 1.3) is a single series. Recording-rule names follow the Prometheus

crossplane-observability/templates/prometheus/rules/cloud/cloud-api-throttling.yaml

@@ -28,7 +28,7 @@ spec:
           # two different API groups isn't merged into one p95.
           expr: |
             histogram_quantile(0.95, sum by (le, group, kind) (
-              rate(upjet_resource_reconcile_delay_seconds_bucket{job="{{ .Values.crossplane.providers.job }}"}[15m])
+              rate(upjet_resource_reconcile_delay_seconds_bucket{job="{{ include "crossplane-observability.providersJob" . }}"}[15m])
             )) > {{ $r.thresholdSeconds }}
           for: {{ $r.for }}
           labels:

crossplane-observability/templates/prometheus/rules/control-plane/detection-lag.yaml

@@ -20,7 +20,7 @@ spec:
         - alert: DetectionLagHigh
           expr: |
             histogram_quantile({{ $r.quantile }}, sum by (le, gvk) (
-              rate(crossplane_managed_resource_first_time_to_reconcile_seconds_bucket{job="{{ .Values.crossplane.providers.job }}"}[15m])
+              rate(crossplane_managed_resource_first_time_to_reconcile_seconds_bucket{job="{{ include "crossplane-observability.providersJob" . }}"}[15m])
             )) > {{ $r.thresholdSeconds }}
           for: {{ $r.for }}
           labels:

crossplane-observability/templates/prometheus/rules/drift/drift-detected.yaml

@@ -31,9 +31,9 @@ spec:
           # 1h window: drift observations are sparse per gvk (tens per day on some kinds),
           # so a short window leaves most kinds with no samples and the alert blind.
           expr: |
-            sum by (gvk) (rate(crossplane_managed_resource_drift_seconds_sum{job="{{ .Values.crossplane.providers.job }}"}[1h]))
+            sum by (gvk) (rate(crossplane_managed_resource_drift_seconds_sum{job="{{ include "crossplane-observability.providersJob" . }}"}[1h]))
               /
-            sum by (gvk) (rate(crossplane_managed_resource_drift_seconds_count{job="{{ .Values.crossplane.providers.job }}"}[1h]))
+            sum by (gvk) (rate(crossplane_managed_resource_drift_seconds_count{job="{{ include "crossplane-observability.providersJob" . }}"}[1h]))
               > {{ $r.thresholdSeconds }}
           for: {{ $r.for }}
           labels:

crossplane-observability/templates/prometheus/rules/resource-health/ttr-degraded.yaml

@@ -28,9 +28,9 @@ spec:
             histogram_quantile({{ $r.quantile }}, sum by (le, {{ if .Values.upjet.enabled }}group, kind{{ else }}gvk{{ end }}) (
               rate(
               {{- if .Values.upjet.enabled }}
-                upjet_resource_ttr_bucket{job="{{ .Values.crossplane.providers.job }}"}[30m]
+                upjet_resource_ttr_bucket{job="{{ include "crossplane-observability.providersJob" . }}"}[30m]
               {{- else }}
-                crossplane_managed_resource_first_time_to_readiness_seconds_bucket{job="{{ .Values.crossplane.providers.job }}"}[30m]
+                crossplane_managed_resource_first_time_to_readiness_seconds_bucket{job="{{ include "crossplane-observability.providersJob" . }}"}[30m]
               {{- end }}
               )
             )) > {{ $r.thresholdSeconds }}

crossplane-observability/tests/validate.sh

@@ -5,6 +5,8 @@
 #
 # Layers (each fails the script on error):
 #   1. helm lint + template render (Phase 1 defaults AND everything enabled)
+#   1c. scrape/job coherence    — the `job` label each shipped monitor produces is the one
+#                                 the rules actually query (silent-empty-rules guard)
 #   2. promtool check rules     — PromQL parses, rule structure valid
 #   3. promtool test rules      — rule LOGIC fires as intended on synthetic series
 #   4. metric gate              — every referenced metric is in the allowlist; every
@@ -49,7 +51,7 @@ run_kubeconform() {
 }
 
 mkdir -p "$RENDER_DIR"
-trap 'rm -f "${RENDER_DIR}/all.yaml"' EXIT
+trap 'rm -f "${RENDER_DIR}/all.yaml" "${RENDER_DIR}/all-fixedjob.yaml"' EXIT
 
 step "1. helm lint"
 helm lint "$CHART"
@@ -60,8 +62,54 @@ helm template cp "$CHART" >/dev/null && green "renders"
 step "1. helm template (everything enabled)"
 helm template cp "$CHART" "${ALL_FLAGS[@]}" > "${RENDER_DIR}/all.yaml" && green "renders"
 
+step "1c. scrape/job coherence — the monitors this chart ships match the job labels its rules query"
+# Prometheus-operator derives the `job` label from the scrape object, and the rules filter on
+# it: a ServiceMonitor's job is the SERVICE name, a PodMonitor's is `<namespace>/<name>`. Get
+# either wrong and nothing complains — the rules render, validate, parse, and match no series
+# forever. That is exactly how the shipped provider default (`crossplane-providers`, a name no
+# PodMonitor can produce) stayed broken. So assert both couplings on the rendered output.
+python3 - "${RENDER_DIR}/all.yaml" <<'PY'
+import sys, yaml, re
+svcs, pms, jobs = [], [], set()
+for d in yaml.safe_load_all(open(sys.argv[1])):
+    if not d:
+        continue
+    k = d.get("kind")
+    if k == "Service":
+        svcs.append(d["metadata"]["name"])
+    elif k == "PodMonitor":
+        pms.append(d["metadata"]["namespace"] + "/" + d["metadata"]["name"])
+    elif k == "PrometheusRule":
+        for g in d["spec"]["groups"]:
+            for r in g["rules"]:
+                jobs.update(re.findall(r'job="([^"]+)"', r.get("expr", "")))
+fail = []
+for name in svcs:
+    if name not in jobs:
+        fail.append(f'Service/{name} is scraped as job="{name}", which no rule queries')
+for job in pms:
+    if job not in jobs:
+        fail.append(f'PodMonitor job="{job}" is queried by no rule (crossplane.providers.job disagrees)')
+if not svcs or not pms:
+    fail.append(f"expected a Service and a PodMonitor in the all-enabled render, got {len(svcs)}/{len(pms)}")
+if fail:
+    print("\033[31mscrape/job MISMATCH:\033[0m")
+    for f in fail:
+        print("  -", f)
+    print("  jobs queried by rules:", ", ".join(sorted(jobs)))
+    sys.exit(1)
+print(f"\033[32mjob labels coherent: Service {svcs} + PodMonitor {pms} all queried by rules\033[0m")
+PY
+
 step "2/3. merge rendered PrometheusRules for promtool"
-python3 - "${RENDER_DIR}/all.yaml" "${RENDER_DIR}/rules.yaml" <<'PY'
+# Rendered with an EXPLICIT crossplane.providers.job, unlike all.yaml above: left at the
+# default it derives to `<namespace>/<release>-crossplane-observability-providers`, which
+# would pin the unit-test fixtures to this script's release name. The unit tests check rule
+# LOGIC, not scrape wiring — step 1c owns the wiring — so a stable job label is what they
+# want, and pinning it here exercises the override path too.
+helm template cp "$CHART" "${ALL_FLAGS[@]}" \
+  --set crossplane.providers.job=crossplane-providers > "${RENDER_DIR}/all-fixedjob.yaml"
+python3 - "${RENDER_DIR}/all-fixedjob.yaml" "${RENDER_DIR}/rules.yaml" <<'PY'
 import sys, yaml
 src, dst = sys.argv[1], sys.argv[2]
 groups = {}  # name -> rules[]  (merge same-named groups across CRs into one file)

crossplane-observability/values.yaml

@@ -48,9 +48,21 @@ crossplane:
     selector:
       matchExpressions:
         - { key: pkg.crossplane.io/revision, operator: Exists }
-    # The `job` label provider metrics land under once scraped. On Stakater Cloud this is
+    # The `job` label provider metrics land under once scraped, used by every
+    # provider-scoped rule expression.
+    #
+    # LEAVE EMPTY to use the PodMonitor this chart ships (`providerPodMonitor` above).
+    # Prometheus-operator derives a PodMonitor's job label as `<namespace>/<name>`, so the
+    # correct value is computed from the rendered object rather than restated here — a
+    # hand-written guess at that string is wrong in a way nothing catches: the rules render
+    # fine and silently match no series. (The previous default, `crossplane-providers`, was
+    # exactly that: a plausible name no PodMonitor ever produces.)
+    #
+    # Set it only when some OTHER scrape already covers the provider pods — then keep
+    # `providerPodMonitor` disabled so the two don't both scrape. On Stakater Cloud clusters
+    # carrying the platform PodMonitor that value is
     # `crossplane-system/crossplane-providers-and-functions`.
-    job: crossplane-providers
+    job: ""
   # Story 4.1 — Composite (XR) inventory & readiness. The leaf-MR metrics above are per-GVK
   # counts that do NOT cover the Composite/Claim layer the customer experiences. That comes
   # from an inventory exporter (resource-state-metrics / ksm-crossplane), which emits a