feat: log and meter reconcile skips in QController runtime#664
Conversation
Previously qtransform swallowed `SkipReconcileTag` errors and returned nil, so the QRuntime saw a plain success and the reason behind the skip was lost. The runtime now detects the tag itself, emits a dedicated `reconcile skipped` log line carrying the original reason, and increments a new `qcontroller_skips` expvar metric — while still treating the cycle like a success for backoff purposes. Signed-off-by: Oguz Kilcan <oguz.kilcan@siderolabs.com>
|
/m |
|
@oguzkilcan merge was not performed: push: To https://github.com/cosi-project/runtime.git |
I got this as well |
There was a problem hiding this comment.
Pull request overview
This PR ensures SkipReconcileTag-based “skips” are no longer silently treated as plain successes by the queue-controller runtime by propagating the tagged error out of qtransform and handling it explicitly in qruntime with dedicated logging and a new expvar metric.
Changes:
- Add
qcontroller_skipsexpvar metric to count skipped reconcile cycles per QController. - Update
qruntimeto detectSkipReconcileTag, logreconcile skippedwith the original reason, and treat it as success for backoff. - Stop swallowing
SkipReconcileTaginqtransformso the runtime can observe and report skip reasons; centralize reconcile-outcome log level selection viaQJob.LogLevel().
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/controller/runtime/metrics/metrics.go | Adds QControllerSkips expvar map metric. |
| pkg/controller/runtime/internal/qruntime/qruntime.go | Detects skip-tagged errors, logs a dedicated “reconcile skipped” message, and increments the new skip metric. |
| pkg/controller/runtime/internal/qruntime/qitem.go | Introduces QJob.LogLevel() helper for consistent reconcile-outcome log levels. |
| pkg/controller/generic/qtransform/qtransform.go | Propagates SkipReconcileTag errors (instead of returning nil) so the runtime can log/meter skip reasons. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "github.com/cenkalti/backoff/v4" | ||
| "github.com/siderolabs/gen/xerrors" | ||
| "go.uber.org/zap" | ||
| "go.uber.org/zap/zapcore" | ||
| "golang.org/x/sync/errgroup" | ||
| "golang.org/x/time/rate" | ||
|
|
||
| "github.com/cosi-project/runtime/pkg/controller" | ||
| "github.com/cosi-project/runtime/pkg/controller/generic/qtransform" | ||
| "github.com/cosi-project/runtime/pkg/controller/runtime/internal/adapter" |
| } | ||
|
|
||
| if xerrors.TagIs[SkipReconcileTag](err) { | ||
| return nil | ||
| return err | ||
| } |
| skipped := xerrors.TagIs[qtransform.SkipReconcileTag](reconcileError) | ||
|
|
||
| if adapter.metricsEnabled { | ||
| if reconcileError != nil { | ||
| switch { | ||
| case skipped: | ||
| metrics.QControllerSkips.Add(adapter.Name, 1) | ||
| case reconcileError != nil: | ||
| metrics.QControllerCrashes.Add(adapter.Name, 1) | ||
| } else if interval != 0 { | ||
| case interval != 0: | ||
| metrics.QControllerRequeues.Add(adapter.Name, 1) | ||
| } |
Previously qtransform swallowed
SkipReconcileTagerrors and returned nil, so the QRuntime saw a plain success and the reason behind the skip was lost. The runtime now detects the tag itself, emits a dedicatedreconcile skippedlog line carrying the original reason, and increments a newqcontroller_skipsexpvar metric — while still treating the cycle like a success for backoff purposes.