feat(telemetry): Add operator health dashboard and metrics - #314
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdded readiness and infrastructure-state Prometheus metrics to reconciliation. Added tests for metric values and stale-state replacement. Added a VictoriaMetrics-backed Grafana dashboard and provisioned it through a ChangesOperator health telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Reconciler
participant Prometheus
participant VictoriaMetrics
participant Grafana
Reconciler->>Prometheus: Record readiness and dependency gauges
Prometheus->>VictoriaMetrics: Expose telemetry for storage
Grafana->>VictoriaMetrics: Query operator health metrics
VictoriaMetrics-->>Grafana: Return dashboard series
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
internal/metrics/metrics_test.go (1)
35-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce the duplicate collector helper.
gatherduplicatesgatherApplicationInfoat lines 11-25. Only the collector differs. Make the older helper delegate.♻️ Proposed refactor
func gatherApplicationInfo(t *testing.T) []*dto.Metric { t.Helper() - out := make(chan prometheus.Metric, 64) - go func() { - ApplicationInfo.Collect(out) - close(out) - }() - var metrics []*dto.Metric - for m := range out { - dtoMetric := &dto.Metric{} - assert.NoError(t, m.Write(dtoMetric)) - metrics = append(metrics, dtoMetric) - } - return metrics + return gather(t, ApplicationInfo) }🤖 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 `@internal/metrics/metrics_test.go` around lines 35 - 49, Update the existing gatherApplicationInfo helper to delegate to the generic gather function, passing its application-info collector instead of duplicating collection and metric conversion logic. Preserve gather’s current behavior and remove the redundant implementation from gatherApplicationInfo.
🤖 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 `@deploy/telemetry/dashboards/wandb-operator-health.json`:
- Around line 112-119: Scope all five operator-gauge PromQL targets to the
operator job by adding the existing job=~"(.*/)?wandb-operator" selector:
wandb_weightsandbiases_ready, both wandb_application_info targets, and both
wandb_infra_state targets. Preserve each query’s existing filters and
expressions while ensuring only the W&B operator scrape series are included.
- Line 1615: Update the CPU throttling Prometheus expression in the dashboard to
derive the operator pod selector from the chart values, using the rendered
telemetry namespace and operator fullname/prefix rather than hardcoded
wandb-operator identifiers. Ensure the selector remains scoped to the configured
namespace and matches non-default release names, preserving the existing zero
fallback.
In `@internal/controller/reconciler/reconcile_v2.go`:
- Around line 1590-1607: Add deletion handling for readiness and infrastructure
metric series: in internal/controller/reconciler/reconcile_v2.go, call the new
metrics deletion helper in the finalizer branch before
controllerutil.RemoveFinalizer, and clear each component’s existing series at
the start of every status-map loop before recording current entries. In
internal/metrics/metrics.go, add DeleteWeightsAndBiasesMetrics(namespace, name
string) to delete the readiness pair and infrastructure metrics, plus a
component-scoped helper that deletes matching series by namespace, name, and
component.
- Line 1606: Guard the SetInfraState call for the Kafka component in the
reconciliation flow so it runs only when wandb.Spec.Kafka.ManagedKafka is
configured. Leave Kafka status inference unchanged, and avoid recording metrics
with an empty instance name or state for resources without managed Kafka.
---
Nitpick comments:
In `@internal/metrics/metrics_test.go`:
- Around line 35-49: Update the existing gatherApplicationInfo helper to
delegate to the generic gather function, passing its application-info collector
instead of duplicating collection and metric conversion logic. Preserve gather’s
current behavior and remove the redundant implementation from
gatherApplicationInfo.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 05547bea-8f68-491a-bb56-564c72a75359
📒 Files selected for processing (6)
deploy/telemetry/dashboards/wandb-operator-health.jsondeploy/telemetry/templates/telemetry-ui.yamlinternal/controller/reconciler/readiness.gointernal/controller/reconciler/reconcile_v2.gointernal/metrics/metrics.gointernal/metrics/metrics_test.go
Comment on lines
+112
to
+119
| "targets": [ | ||
| { | ||
| "expr": "wandb_weightsandbiases_ready", | ||
| "legendFormat": "{{namespace}}/{{name}}", | ||
| "refId": "A", | ||
| "instant": true | ||
| } | ||
| ], |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope the operator-gauge queries to the operator job.
Every controller-runtime, workqueue, rest_client, webhook, and process target in this dashboard carries job=~"(.*/)?wandb-operator". The five targets that read the operator's own gauges carry no selector:
- Line 114
wandb_weightsandbiases_ready - Line 168
wandb_infra_state{state!="Healthy"} - Line 223
wandb_application_info - Line 246
wandb_infra_state == 1 - Line 315
wandb_application_info
Any other scrape target that exposes these metric names is included. "Distinct image versions" is the clearest failure: count(count by (tag) (wandb_application_info)) counts tags across all matching series, so a second W&B install in the same VictoriaMetrics instance reports false version drift and turns the tile red.
🐛 Proposed fix for the CR Ready target
"targets": [
{
- "expr": "wandb_weightsandbiases_ready",
+ "expr": "wandb_weightsandbiases_ready{job=~\"(.*/)?wandb-operator\"}",
"legendFormat": "{{namespace}}/{{name}}",
"refId": "A",
"instant": true
}
],Apply the same selector to the four remaining targets.
📝 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.
Suggested change
| "targets": [ | |
| { | |
| "expr": "wandb_weightsandbiases_ready", | |
| "legendFormat": "{{namespace}}/{{name}}", | |
| "refId": "A", | |
| "instant": true | |
| } | |
| ], | |
| "targets": [ | |
| { | |
| "expr": "wandb_weightsandbiases_ready{job=~\"(.*/)?wandb-operator\"}", | |
| "legendFormat": "{{namespace}}/{{name}}", | |
| "refId": "A", | |
| "instant": true | |
| } | |
| ], |
🤖 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 `@deploy/telemetry/dashboards/wandb-operator-health.json` around lines 112 -
119, Scope all five operator-gauge PromQL targets to the operator job by adding
the existing job=~"(.*/)?wandb-operator" selector: wandb_weightsandbiases_ready,
both wandb_application_info targets, and both wandb_infra_state targets.
Preserve each query’s existing filters and expressions while ensuring only the
W&B operator scrape series are included.
| }, | ||
| "targets": [ | ||
| { | ||
| "expr": "sum(rate(container_cpu_cfs_throttled_periods_total{container=\"operator\", pod=~\"wandb-operator-[^-]+-[^-]+\"}[$__rate_interval])) / sum(rate(container_cpu_cfs_periods_total{container=\"operator\", pod=~\"wandb-operator-[^-]+-[^-]+\"}[$__rate_interval])) or vector(0)", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the operator Deployment name, container name, and namespace helper in the chart.
fd -e yaml -e tpl | xargs rg -nP -C6 'kind:\s*Deployment' -g '*operator*'
fd -e yaml -e tpl | xargs rg -nP -C3 'name:\s*operator\b|containers:'
rg -nP -C3 'define "telemetry.namespace"' -g '*.tpl'Repository: wandb/operator
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo "== file context around dashboard selector =="
sed -n '1560,1640p' deploy/telemetry/dashboards/wandb-operator-health.json
echo
echo "== locate wandb-operator pod/controller deployment in deploy/operator and relevant telemetry helpers =="
rg -n -C 5 'kind:\s*Deployment|app.kubernetes.io/name:\s*wandb-operator|app.kubernetes.io/name:\s*operator|control-plane:\s*controller-manager|define "telemetry\.namespace"|telemetry\.namespace|fullname|name:{{|name:' deploy/operator templates deploy/telemetry -g '*.yaml' -g '*.tpl' -g 'Chart.yaml' | sed -n '1,260p'
echo
echo "== operator deployment templates =="
fd -e yaml -e tpl -d 3 . deploy/operator | rg 'deployment|manager|values|Chart\.yaml' | while read -r f; do
echo "--- $f"
sed -n '1,220p' "$f"
done
echo
echo "== telemetry namespace/name helpers =="
rg -n -C 4 'define "telemetry\.namespace"|telemetry\.namespace|define "telemetry\.fullname"|telemetry\.fullname|define "telemetry\.name"|telemetry\.name|include ".*fullname|fullname' deploy/telemetry -g '*.tpl'Repository: wandb/operator
Length of output: 10348
🏁 Script executed:
#!/bin/bash
set -u
echo "== operator deployment templates =="
for f in $(fd -e yaml -e tpl '.*' deploy/operator | rg 'deployment|manager|values|Chart\.yaml|_helpers|namespace|common' | head -50); do
echo "--- $f"
sed -n '1,240p' "$f"
done
echo
echo "== relevant telemetry helpers =="
for f in $(fd -e tpl '.*' deploy/telemetry | head -80); do
echo "--- $f"
sed -n '1,220p' "$f"
doneRepository: wandb/operator
Length of output: 12900
🏁 Script executed:
#!/bin/bash
set -u
echo "== wandb operator fullname helper files =="
rg -n -C 8 'define "wandb-base\.fullname"|define "wandb-operator\.fullname"|_helpers\.tpl|fullname' deploy -g '*.tpl' -g '*.yaml' -g 'Chart.yaml' | sed -n '1,260p'
echo
echo "== controller deployment in wandb-operator dependency manifests =="
rg -n -C 8 '^kind:\s*Deployment$|app\.kubernetes\.io/name:\s*wandb-operator|control-plane:\s*controller-manager|name:\s*operator|namespace:\s*wandb' helm dependency . -g '*.yaml' -g '*.tpl' | sed -n '1,260p' || true
echo
echo "== direct template references to deployment name/pod labels =="
fd -e tpl -e yaml . deploy/operator | xargs rg -n -C 6 'metadata:|name:|labels:|selector:|app\.kubernetes\.io/name|wandb-operator' | sed -n '1,300p'Repository: wandb/operator
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo "== Wandb base helpers if present =="
fd -e tpl '.*' . | rg 'wandb-base|base|_helpers' | while read -r f; do
echo "--- $f"
sed -n '1,220p' "$f"
done
echo
echo "== all wandb-operator.fullname references =="
rg -n 'wandb-operator\.fullname|wandb-base\.fullname|fullName|full-name' deploy -g '*.tpl' -g '*.yaml' | sed -n '1,220p'
echo
echo "== operator container names in templates =="
rg -n -C 4 '(^|[\s-])name:\s*operator\b|name:\s*manager\b|containerName:\s*operator' deploy/operator deploy/telemetry -g '*.tpl' -g '*.yaml' | sed -n '1,200p'Repository: wandb/operator
Length of output: 8677
Harden the CPU throttling selector.
Derive this selector from the chart values instead of hardcoding wandb-operator. Use the rendered telemetry namespace and the operator fullname/prefix so non-default releases and telemetry.namespace values do not fall back to 0% or include operator pods from other namespaces.
🤖 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 `@deploy/telemetry/dashboards/wandb-operator-health.json` at line 1615, Update
the CPU throttling Prometheus expression in the dashboard to derive the operator
pod selector from the chart values, using the rendered telemetry namespace and
operator fullname/prefix rather than hardcoded wandb-operator identifiers.
Ensure the selector remains scoped to the configured namespace and matches
non-default release names, preserving the existing zero fallback.
Expose per-CR readiness and per-dependency state as gauges on the operator metrics endpoint, mirroring the existing wandb_application_info pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set wandb_weightsandbiases_ready wherever readiness is decided, and publish each dependency's state after infra status inference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A dedicated "is the operator itself healthy?" board with a description header and per-section notes explaining each panel in plain language. Covers reconcile health (errors, results, latency, workers, terminal errors, panics), workqueue backlog, Kubernetes API-client calls, admission/conversion webhooks, and Go process health, plus CR readiness, unhealthy-dependency count, per-dependency state, and image-version drift from the operator's wandb_* gauges. Queries match job=~"(.*/)?wandb-operator" so they follow the operator to any namespace and other controller-runtime operators don't bleed in. Current-state tiles use instant queries so transient startup states don't linger. Panels the operator's client-go build doesn't emit (rest-client and webhook latency histograms) are left out rather than shown empty. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend the operator-health board's Process & runtime section with up/liveness, uptime, last-restarted and restarts-in-range (from the operator's own process metrics) and container CPU-throttling (cAdvisor, scoped to the operator pod by name so it stays namespace-portable). Drops the "who it's for" line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jthakkar04
force-pushed
the
jthakkar/operator-health-telemetry
branch
from
August 5, 2026 19:22
0dafbe5 to
ce50fe2
Compare
casey-coreweave
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Answers "is the operator itself healthy?" through dashboard.
wandb_weightsandbiases_readyandwandb_infra_state— on the operator's existing metrics endpoint (no new scrape).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests