Skip to content

add note about metric recorder #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sourcehawk merged 1 commit into from
Mar 26, 2026
Merged

add note about metric recorder #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ func (r *MyReconciler) Reconcile(ctx context.Context, req reconcile.Request) (re
Client: r.Client,
Scheme: r.Scheme,
Recorder: r.Recorder,
Metrics: r.Metrics,
Owner: owner,
Comment on lines 144 to 148
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Quick Start snippet now references r.Metrics, but README doesn’t explain what this is or how to construct a component.Recorder (e.g., via go-crd-condition-metrics) or what a minimal/no-op implementation looks like for users who don’t want metrics. Adding a short note/link near this snippet would prevent copy/paste confusion.

Copilot uses AI. Check for mistakes.
}

Expand Down
5 changes: 5 additions & 0 deletions docs/component.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ recCtx := component.ReconcileContext{
Client: r.Client, // sigs.k8s.io/controller-runtime/pkg/client
Scheme: r.Scheme, // *runtime.Scheme
Recorder: r.Recorder, // record.EventRecorder
Metrics: r.Metrics, // component.Recorder (condition metrics)
Owner: owner, // the CRD that owns this component
}

Expand All @@ -268,6 +269,10 @@ err = comp.Reconcile(ctx, recCtx)

Dependencies are passed explicitly so components remain testable and decoupled from global state.

The `Metrics` field is required. The framework records Prometheus metrics for every condition state transition during
reconciliation. The recorder implementation is provided by
[go-crd-condition-metrics](https://github.com/sourcehawk/go-crd-condition-metrics).
Comment on lines +272 to +274
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text says metrics are recorded for every condition state transition, but the current implementation calls the Metrics recorder every time setStatusCondition runs (even when the condition didn’t change). Consider rewording to reflect that the recorder is invoked on each reconciliation/condition set, with transitions reflected via LastTransitionTime rather than being the only time RecordConditionFor is called.

Suggested change
The `Metrics` field is required. The framework records Prometheus metrics for every condition state transition during
reconciliation. The recorder implementation is provided by
[go-crd-condition-metrics](https://github.com/sourcehawk/go-crd-condition-metrics).
The `Metrics` field is required. The framework invokes the Prometheus metrics recorder whenever it updates a component
condition during reconciliation; condition state transitions are reflected via each condition's `LastTransitionTime`.
The recorder implementation is provided by [go-crd-condition-metrics](https://github.com/sourcehawk/go-crd-condition-metrics).

Copilot uses AI. Check for mistakes.

## Best Practices

**Keep controllers thin.** The controller's job is to fetch the owner CRD, decide which components should exist, and
Expand Down
Loading